MSEP-one / msep.one

MSEP.one main application repository
MIT License
22 stars 3 forks source link

Ensure 'Apply simulation state' is always stored in its own snapshot, separate from other user actions #2

Closed HungryProton closed 2 days ago

HungryProton commented 4 days ago

Fixes the following bug:

This PR applies the simulation and the user action in two separate snapshots instead.

HungryProton commented 3 days ago

Applying that previous snapshot seems a bit redundant to me, why we cannot just create snapshot twice? Once just before user is performing not whitelisted operation and second time directly after that operation? That way we also would not need to add new api to History

The issue is when calling snapshot_moment, the action was already done, so the only way to take a snapshot before the user performs an action is to go back everywhere where this method is called an call apply_simulation_if_running() before doing anything.

It's possible, and removes the need for a simulation whitelist, but it's also more intrusive and error prone (I think it will be easy to forget applying the simulation in unrelated parts of the code). The way its done here is just trying to mimic what we had before with NanoUndoRedo, but we can change that if necessary.