CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
577 stars 85 forks source link

short-term undo for bulk edit tools. #860

Closed kianzarrin closed 4 years ago

kianzarrin commented 4 years ago

see https://github.com/CitiesSkylinesMods/TMPE/issues/568#issuecomment-617970817

Can this wait until 11.6 release? That kind of stuff should be centralised, for example have a memo class that remembers state for anything that TM:PE can change regarding a node, segment or lane. And we should have central undo stack, maybe also see if we can integrate with the Undo mod somehow so that user has global undo facility?

My intention is for user to be able to undo immediately afterwards. I do not intend to remember anything long term. so for example when user clicks CTRL+SHIFT+CLICK on a roundabout he can immediately switch back if he CTRL+SHIFT+CLICK again in a few seconds time. My objective for now is as I said :" It won't be a proper undo feature but it is good enough for my road selection panel." Yeah it can wait. I guess my work can be upgraded to central undo stack. or maybe we need a redesign if we want to go that route.

kianzarrin commented 4 years ago

I introduced IRecordabe interface with Record() and Restore(). I added several implementations of IRecordable for segments, nodes, segmentEnds, and lanes. All of this is wrapped by a big IRecordable.

Each Mass edit operation automatically creates a record of the instancIDs it touches. to Undo, record.Restore() is called.

originalfoo commented 4 years ago

I assume there will be some structs/classes (for node, segment and lane) that can be used as a 'snapshot' (or 'restore point') for those things?

So before making changes, you take a snapshot of current state (eg. send the segment id to a constructor of a 'SegmentSnapshot' class - or whatever - and it grabs all relevant settings for the segment and stores them in its properties)? And then restore would just be running through a list of those objects calling their "Restore()" method or something like that?

If so that would be hugely useful to have for some of the stuff planned in 11.6 code cleanup.

kianzarrin commented 4 years ago

I found a bug in priority tool-> shift-click : 1- shift-click on another road to get yield signs. 2- move mouse to another road without going on grass 3- shift click on the other road and you shall get stop signs. One would expect that the state machine should restart.

Screenshot (898)

originalfoo commented 4 years ago

Ah, I was typing my comment before you added your's. Sounds good! Ok, do it :)

We could potentially, at later date, use something similar for persistence in save game

originalfoo commented 4 years ago

Regarding bug (https://github.com/CitiesSkylinesMods/TMPE/issues/860#issuecomment-618106375) open new issue for that pls.

The priority signs tool is a bit of a mess in terms of interaction, it could really do with some updates. (I'm referring to the older manual edit code, the way the icons work, node selection, etc)

kianzarrin commented 4 years ago

@aubergine10 my code is here if you want to take a pick but yeah I take snapshots.

Regarding bug (#860 (comment)) open new issue for that pls.

I am going to resolve it as part of this issue. I have to modify the code to add Undo anyway I might as well fix this too :)

originalfoo commented 4 years ago

The snapshots could facilitate some sort of copy/paste features at later date. Maybe even provide an alternate way to persist data to savegame, etc.

my code is here if you want to take a pick

trav