Deactivation of pokes is not a safe operation as memory contents could have changed (intro screen vs level 1) or memory banks (128k) could have been unpaged. I'm not against this change, but could be dangerous (as the initial activation).
machine_reset() should be revisited as it is wrong deactivating pokes on a recently resetted machine/memory.
Last edit: Sergio Baldoví 2016-05-22
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I agree that undoing a trainer's poke into memory isn't guaranteed to restore the state precisely as it was before, but unless I'm missing something I do think that by already having reversible pokes that don't perform any validation we aren't creating a new issue by restting the pokes when they are otherwise cleared?
I agree that pokemem_clear() should happen earlier in machine_reset().
BogDan: what are the particular cases you wanted to cover with this patch?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I'm using it to allow the user to clear the pokes.
Basically my "Poke Manager" has an extra button "Clear" [1] which can be used by the users to clear all the pokes.
Looking at this a big more, there is also a weird thing where the active status for the pokes in the file is calculated immediately on load but the state of memory will naturally change after the file is loaded (particularly as we auto-load a pok when it is in the same directory of a tape we are loading and the game isn't even loaded at this point).
I can shuffle pokemem_clear() to happen earlier in machine_reset() to address that problem, but will also revert for 1.2 if we believe more work would be required for the change to make sense.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
A "Clear all" action initiated by the user makes sense, what shocked me was that freeing system resources was bound to poke deactivation. It would be nice to allow a 'discard' operation, but that's not a major issue after all.
Looking at this a big more, there is also a weird thing where the active status for the pokes in the file is calculated immediately on load but the state of memory will naturally change after the file is loaded (particularly as we auto-load a pok when it is in the same directory of a tape we are loading and the game isn't even loaded at this point).
The .pok file is auto-loaded but trainers are not auto-activated, hence there is no need to wait for a tape-stop event. The activation is left to the user as it is a personal decision, e.g., get infinite lives but don't be immune or don't walk through walls.
I can shuffle pokemem_clear() to happen earlier in machine_reset() to address that problem.
I think that would be enough for now.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The .pok file is auto-loaded but trainers are not auto-activated, hence there is no need to wait for a tape-stop event. The activation is left to the user as it is a personal decision, e.g., get infinite lives but don't be immune or don't walk through walls.
This is correct, but I think the acive state is only evaluated when the pokes are loaded (e.g. when loading a tape file on a newly reset Spectrum that doesn't have the game loaded), not when the dialog is opened? If so, it is an improvement for the future.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
This is correct, but I think the acive state is only evaluated when the pokes are loaded (e.g. when loading a tape file on a newly reset Spectrum that doesn't have the game loaded), not when the dialog is opened? If so, it is an improvement for the future.
You're right. IIRC that has a marginal benefit when messing with multiple files or manually creating pokes. It is very unlikely that a newly loaded game has pokes applied.
It would be nice delaying the auto-load of the pok file to the first dialog run. We save resources and the evaluation of the active state would be more accurate.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thanks, committed in [r5507] and [r5508].
Related
Commit: [r5507]
Commit: [r5508]
Deactivation of pokes is not a safe operation as memory contents could have changed (intro screen vs level 1) or memory banks (128k) could have been unpaged. I'm not against this change, but could be dangerous (as the initial activation).
machine_reset() should be revisited as it is wrong deactivating pokes on a recently resetted machine/memory.
Last edit: Sergio Baldoví 2016-05-22
I agree that undoing a trainer's poke into memory isn't guaranteed to restore the state precisely as it was before, but unless I'm missing something I do think that by already having reversible pokes that don't perform any validation we aren't creating a new issue by restting the pokes when they are otherwise cleared?
I agree that pokemem_clear() should happen earlier in machine_reset().
BogDan: what are the particular cases you wanted to cover with this patch?
I'm using it to allow the user to clear the pokes.
Basically my "Poke Manager" has an extra button "Clear" [1] which can be used by the users to clear all the pokes.
[1] https://github.com/bog-dan-ro/spectacol/blob/master/fuse/ui/qml/qml/PokeManagerPage.qml#L190
Thanks for the background info BogDan.
Looking at this a big more, there is also a weird thing where the active status for the pokes in the file is calculated immediately on load but the state of memory will naturally change after the file is loaded (particularly as we auto-load a pok when it is in the same directory of a tape we are loading and the game isn't even loaded at this point).
I can shuffle pokemem_clear() to happen earlier in machine_reset() to address that problem, but will also revert for 1.2 if we believe more work would be required for the change to make sense.
A "Clear all" action initiated by the user makes sense, what shocked me was that freeing system resources was bound to poke deactivation. It would be nice to allow a 'discard' operation, but that's not a major issue after all.
The .pok file is auto-loaded but trainers are not auto-activated, hence there is no need to wait for a tape-stop event. The activation is left to the user as it is a personal decision, e.g., get infinite lives but don't be immune or don't walk through walls.
I think that would be enough for now.
This is correct, but I think the acive state is only evaluated when the pokes are loaded (e.g. when loading a tape file on a newly reset Spectrum that doesn't have the game loaded), not when the dialog is opened? If so, it is an improvement for the future.
You're right. IIRC that has a marginal benefit when messing with multiple files or manually creating pokes. It is very unlikely that a newly loaded game has pokes applied.
It would be nice delaying the auto-load of the pok file to the first dialog run. We save resources and the evaluation of the active state would be more accurate.
Committed in [r5513].
Related
Commit: [r5513]
Last edit: Fredrick Meunier 2016-05-23