Undo Redo for Editor panes: plan - adventuregamestudio/ags GitHub Wiki

Current state on ags4

Five facts in the existing code drive most of the design choices below.

  • Property grid hook is lossy. frmMain.propertiesPanel_PropertyValueChanged forwards e.ChangedItem.Label (the display name, not the property name) and a single e.OldValue. GUIController then calls PropertyChanged on both the component and the pane. With multi-selection there is no per-object old value; for reference-type values edited in place (CustomProperties, Interactions), old and new are the same object.
  • OnPropertyChanged has side effects. In GUIEditor, changing Image resizes the selected buttons, and a clashing Name is reverted by hand. RoomSettingsEditor updates the character layer when StartingRoom changes. Setting the old value back would not undo these.
  • IDs are positional. GUI.DeleteControl renumbers ID and ZOrder; ObjectsEditorFilter.CommandDeleteObject renumbers room object IDs; view loops and frames shift by index. An ID is not an identity in history.
  • Plain reflection is not enough. GUIButton, RoomObject, RoomRegion, RoomWalkableArea, Character, MouseCursor, Sprite, Settings and others implement ICustomTypeDescriptor, so values must be read and set through TypeDescriptor / PropertyDescriptor, as the grid does.
  • Room masks are managed bitmaps. IRoomController.GetMask / SetMask exchange Bitmaps. BaseAreasEditorFilter keeps one full-size _undoBufferMask per filter (one level, covers drawing and mask import) and handles Ctrl+Z itself. Script editors already expose Undo/Redo menu commands (ScriptEditorBase), which the new system should reuse.

Proposed design

Generic operations hold the data changes; each pane supplies UI side effects through a small context interface.

  1. IEditOperation: Do(), Undo(), Description, and optional bool TryMerge(IEditOperation next) for coalescing.
  2. UndoRedoHistory: one instance per pane, discarded when the pane closes (agreed in both tickets). Undo and redo stacks with a capacity limit, a save-point marker, and a HistoryChanged event that drives menu and toolbar state. Type-agnostic.
  3. Generic operations plus an editor context (a mix of the two options in the request):
    • Self-contained generic operations: PropertyChangeOp (object references, PropertyDescriptor, old and new value per object), ListInsertOp / ListRemoveOp (IList, item instance, index), CompoundOp.
    • IUndoContext, implemented by each pane, called after data is applied: refresh canvas and grid, raise NotifyClientsGUIControlAddedOrRemoved, fix renumbered IDs, re-select affected objects.
  4. Edits recorded as transactions. The pane opens a transaction over the affected objects (snapshot of their browsable property values), the existing code runs unchanged with its side effects, and closing the transaction diffs the snapshots into one CompoundOp. This covers grid edits (multi-select and the Image resize included), mouse move and resize, align, distribute, lock, z-order and Edit Text. A rejected Name change reverts itself, so the diff is empty and nothing is recorded. For grid edits the snapshot is taken on selection and refreshed after each change, since the grid has no "changing" event.
  5. Object identity by reference, no GUIDs. Operations keep references to the actual instances. Undoing a delete re-inserts the same instance at its old index and restores the ID numbering; redoing a paste re-adds the same instance rather than deserializing the clipboard again. Every earlier operation in the history still points at a live object. GUIDs become necessary only if instances are replaced while a history is alive, or if history must outlive the pane (D3).
  6. Room masks: a mask operation stores the before and after state of the stroke's dirty rectangle (compressed), applied through GetMask / SetMask. No native changes needed; it replaces the per-filter _undoBufferMask.

Rollout

Each step is usable on its own; the GUI editor proves the core before the harder Room editor work.

  1. Core: IEditOperation, UndoRedoHistory, generic operations, transaction helper, Ctrl+Z / Ctrl+Y and Edit menu routing through EditorContentPanel. Unit tests in AGS.Editor.Tests, no UI needed.
  2. GUI editor: property grid, move and resize (one operation per mouse-down to mouse-up), align and distribute, lock, z-order, add, paste, delete, Edit Text.
  3. View editor: add, delete and paste of frames and loops, flip, reverse, quick import, frame properties, sprite assignment.
  4. Room editor: objects, characters and edges with the same patterns; mask drawing and import as mask operations; room properties.

Decisions needed

Fifteen points need a call before or during step 1; each has a recommended default.

# Question Recommendation
D1 Logic inside each operation, or a per-editor OperationManager? Mixed: generic operations are self-contained; pane side effects go through IUndoContext.
D2 Capture property edits by snapshot and diff, or build PropertyChangeOp explicitly in each handler? Snapshot and diff: the only way that catches side effects and multi-select.
D3 Object identity: references or GUIDs? References. Revisit only if history must survive a pane closing or reloading.
D4 Edits from outside the pane that break its history: sprite deleted in Sprite Manager, character deleted from the tree, room reloaded from disk, mask resolution changed or room resized (mask snapshots no longer line up). Clear the affected history on such events. Alternative: mark operations invalid and skip them.
D5 Undo-delete or redo-paste restores a script name another pane has taken meanwhile. Detect the clash on Do / Undo; then either auto-rename (as paste does now) or refuse with a message.
D6 Room editor: undoing an operation that belongs to a layer that is not active. Switch to that layer and select the affected item, then apply.
D7 Should undo and redo restore the selection? Yes, select the affected objects.
D8 Document changes versus view state: GUI Locked is saved in the project; Room design-time Visible / Locked (DesignModified) and GUI control groups are editor-only. Record Locked and z-order; do not record view state or groups.
D9 Coalesce repeated arrow-key nudges or typing into one field? Merge consecutive operations on the same objects and property within about 1 second.
D10 History limit and memory budget for masks. Count limit (around 100) plus a byte budget for mask operations; compressed dirty rectangles. Spilling to temp files can wait.
D11 Drop delete confirmations (such as the room object prompt) once delete is undoable? Drop them.
D12 Undo API in AGS.Types (plugin API) or AGS.Editor? Start in AGS.Editor, expose later.
D13 Scope exclusions. Exclude Dialogs (they mix in Scintilla's own undo) and room backgrounds at first. Grid-only panes (Character, Inventory and others) get history almost free after step 1: include or not?
D14 Ctrl+Z when a text box or the property grid has focus. The focused text control wins; the pane history handles it otherwise.
D15 Return Room.Modified to clean when undo reaches the save point? Nice to have; the save-point marker supports it.

Open doubts

These need checking in a prototype rather than a decision up front.

  • Whether PropertyGrid's OldValue is usable at all with multi-selection (moot if D2 goes with snapshots).
  • Type editors that change collections in place (CustomProperties, Interactions): deep-copy them in the snapshot, or leave them out of history at first.
  • Expandable nested properties (where ChangedItem.Parent is not the root object) and struct values such as Size and Point.
  • GUIControlGroup state after undoing the delete of a control that was in a group.
  • Event handler functions created from the GUI editor in the Script editor: assumed to stay out of history (an undone add leaves the handler in the script).