adventuregamestudio / ags

AGS editor and engine source code
Other
708 stars 159 forks source link

Editor: proper Undo/Redo system for the Room Editor #2321

Open ivan-mogilko opened 10 months ago

ivan-mogilko commented 10 months ago

Room Editor lacks proper Undo/Redo. There's an "Undo" in the mask paint mode, but it supports only 1 level of undo, and is implemented suboptimally.

How to implement a proper Undo/Redo system?

Suggestion

Implement a "Operation" class (example, actual name may be different) that defines an operation that may be performed in a particular context (like, certain editor). Operation has an "operation type" ID (enum or string) and a set of parameters. Operation may be subclassed, because some operations require specific arguments. Alternatively, it's not Operation itself is subclassed, but some nested "OperationArgs" member. Look the C# EventArgs for example.

Implement a "UndoRedo" manager class, as a generic manager for some editor. This manager does following:

  1. Stores Undo list, which is a stack (or dequeue) of Operations of certain capacity limit, and Redo list of same kind.
  2. Has "UndoOperation" and "RedoOperation" events, which someone can subscribe to. These receive Operation object as a parameter.
  3. Has methods like Record(), CanUndo(), CanRedo(), Undo() and Redo().
    • Record method pushes Operation to the Undo queue, and clears Redo queue.
    • Undo takes out the last Operation from the back of a Undo queue, pushes to Redo queue front, and fires "UndoOperation" event.
    • Redo takes out the first Operation from the front of a Redo queue, pushes to Undo queue back, and fires "RedoOperation" event.
    • Capacity limit works like this: for Undo queue, any extra items are removed from the front, for Redo queue any extra items are removed from the back. But since these queues should have same limit, then removal from Redo should normally not happen at all (except when it's cleared by doing a completely new operation).

For the Room Editor and its Filter classes (Character, Object, etc):

  1. Implement all necessary Operation subclasses (or OperationArgs subclasses).
  2. Implement Undo and Redo methods that accept Operation, and choose another method to run based on Operation's type ID. What is important: Undone operations must be performed backwards.

How to organize "backwards operations" for Undo. This may be done in a straightforward way, by converting each operation on event. On another hand, this may be done in another way, by supplying a conversion delegate for the UndoRedo Manager class. This may work like this: the operations are recorded in "forward" way, but right before calling Undo, Operation is cloned and clone is converted "backwards" using this delegate. If this is done so, then maybe there's no need for two events (Undo/Redo) and only 1 "DoOperation" event, and 1 "DoOperation" method in the Editor class. But this method will receive either a forward operation for Redoing, or reverted operation for Undoing.

If this will work for the Room Editor, perhaps similar thing may be implemented for some others afterwards, such as View editor, for example.

ivan-mogilko commented 10 months ago

The area drawing will be a distinct problem, because drawing something may overwrite pixels from several areas. So either this operation would need to remember which areas it affected, and be undone in multiple steps, or previous mask state remembered. I might speculate about possible variants here, but perhaps it's better to look into open source art software and see how it's done there.

On a side note, if this mechanism requires extra memory to save the operation info, we might also save portions of Undo/Redo history in temp files.

ericoporto commented 8 months ago

Just for reference when I was making my own Editor/engine, I created like this

https://github.com/ericoporto/fgmk/blob/master/fgmk/cmd.py

The actions that can be done specify both a redo/undo operation and they are executed by going to the stack and executing the redo action.

There is a problem of grouping operations - like when you click down and draw something and release the button, you probably want to group all of these into a single undo call.

If you can put a reasonable amount (all?) commands that affect the state of the room in the undo stack you can probably work fine with holding only the difference of the change but if only a few of the operations are using this method then it's easier to save the entire before state - of course this has the problem of affecting the memory. One approach may be storing the current state in a compressed way.

Also when a room is closed it would lose the undo state.

I think an easier test could be done by adding undo/redo to the GUI Editor and then using what is learned from there in the Room Editor.

ivan-mogilko commented 8 months ago

It's strange how I did not mention an alternative of having operation as a class that does everything itself, I guess I was writing this ticket having existing Editor code in mind, where operations are performed by the "editor" class. But indeed, if editing operations are moved completely to their own classes, then undo/redo should be methods in that class as well.

ericoporto commented 2 months ago

Just a note that the GUI undo/redo is much easier to implement since there are no masks, each interaction can be abstracted in a command without considerable worries about size because there aren't as much information as per pixel data.

Other detail is multiple GUIs may be opened at the same time and presumably each panel will have it's own undo/redo stack, and when they are closed such undo/redo stack may be discarded.

I don't have great imagination so I would make it similar to my python code in a way that a "command" would implement and undo/redo interface and the implementation could hold some internal data necessary for these methods. (Redo is the execute() in command pattern).