Early support for undo in ChangePlugin

docs/developer_guide/building.md

@@ -0,0 +1,7 @@
+# Windows
+
+# Mac-OS
+
+# Linux (Ubuntu)
+
+# Web

docs/developer_guide/index.md

@@ -0,0 +1,9 @@
+## Developer Guide
+
+This guide is currently work in progress. The goal is to document the internals of site editor. In particular, how various parts work.
+For starters we have documented design decisions around `Undo` and `Deletion`. The hope is that as we add more features, people keep adding to this guide so that new contributors find it easy to work with the codebase.
+
+### Topics
+
+* [Building from source](building.md)
+* [Implementing Undo in your plugin](undo.md)

docs/developer_guide/undo.md

@@ -0,0 +1,32 @@
+# Undo Functionality
+
+The undo functionality is handled by the `RevisionTracker` resource.
+
+## Implementing Undo for your own plugin
+
+Bevy is designed to be extended using `Plugin`s. The `RevisionTracker` builds on this
+ideal.
+
+## Design Considerations
+
+There are several possible ways to implement undo functionality. The old Traffic Editor used
+to build on Qt's QAction functionality. This relies heavily on C++'s object oriented nature.
+While it is possible to use `dyn` and `Arc` in rust to try to replicate this method, it does not
+play well with Bevy's event driven ECS nature. Rather we focus on creating a resource which generates unique ids.
+Each unique ID corresponds to an action. It is up to individual plugin authors to handle the
+buffer which stores the changes in state. It is recommended that one maintains a hashmap with the action id being the key and a custom struct that represents your change.
+
+## Implementing your own Undo-able action
+
+If your plugin simply changes a component, it is recommended that you use the `Change` event and associated tools.
+The change component itself does a lot of the heavy lifting for the components and will automatically provide you with feedback. If you are doing more than just changing
+a component then you probably need to read the rest of this section.
+
+### Manually storing actions
+
+The best reference for how to implement undo in your action is perhaps by reading the change plugin's source code. However,
+for the sake of good design documentation, this section will try to explain how you can implement undo for your plugin.
+
+When making a change to the world, the first thing you need to do is request a new revision id from the `RevisionTracker` resource. This revision ID is the unique
+ID for the specific action. Your plugin should store it along with the required information to undo the change. When a user wants
+to undo something the `UndoEvent` event is emitted. Your plugin should implement a listener for this event. The event itself will tell you which action is being undone.

rmf_site_editor/src/interaction/gizmo.rs

@@ -15,7 +15,12 @@
  *
 */
 
-use crate::interaction::*;
+use std::collections::HashMap;
+
+use crate::{
+    interaction::*,
+    site::{RevisionTracker, UndoEvent},
+};
 use bevy::{math::Affine3A, prelude::*, window::PrimaryWindow};
 use bevy_mod_raycast::{deferred::RaycastMesh, deferred::RaycastSource, primitives::rays::Ray3d};
 use rmf_site_format::Pose;
@@ -224,10 +229,26 @@ impl DragPlaneBundle {
     }
 }
 
+/// This handles the
+#[derive(Debug, Clone, Copy)]
+pub struct DraggingInfo {
+    entity: Entity,
+    start_pos: Transform,
+}
+
+// A hack
+impl PartialEq for DraggingInfo {
+    fn eq(&self, other: &Self) -> bool {
+        self.entity == other.entity
+    }
+}
+
+impl Eq for DraggingInfo {}
+
 /// Used as a resource to keep track of which draggable is currently hovered
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Resource)]
 pub enum GizmoState {
-    Dragging(Entity),
+    Dragging(DraggingInfo),
     Hovering(Entity),
     None,
 }
@@ -346,7 +367,15 @@ pub fn update_gizmo_click_start(
                         if let Some(drag_materials) = &gizmo.materials {
                             *material = drag_materials.drag.clone();
                         }
-                        *gizmo_state = GizmoState::Dragging(e);
+
+                        let Ok((_, transform)) = transforms.get(e) else {
+                            error!("Could not get transform for entity {:?}", e);
+                            return;
+                        };
+                        *gizmo_state = GizmoState::Dragging(DraggingInfo {
+                            entity: e,
+                            start_pos: (*transform).into(),
+                        });
                     } else {
                         *gizmo_state = GizmoState::None;
                     }
@@ -360,19 +389,63 @@ pub fn update_gizmo_click_start(
     }
 }
 
+#[derive(Debug)]
+pub struct GizmoMoveChange {
+    pub entity: Entity,
+    pub prev_pos: Transform,
+    pub dest_pos: Transform,
+}
+
+#[derive(Resource, Default)]
+pub struct GizmoMoveUndoBuffer {
+    pub revisions: HashMap<usize, GizmoMoveChange>,
+}
+
+pub(crate) fn undo_gizmo_change(
+    change_history: ResMut<GizmoMoveUndoBuffer>,
+    mut undo_cmds: EventReader<UndoEvent>,
+    mut move_to: EventWriter<MoveTo>,
+) {
+    for undo in undo_cmds.read() {
+        let Some(change) = change_history.revisions.get(&undo.action_id) else {
+            continue;
+        };
+        move_to.send(MoveTo {
+            entity: change.entity,
+            transform: change.prev_pos,
+        })
+    }
+}
+
+#[derive(Resource, Default)]
+pub struct LastDraggedPos {
+    transform: Transform,
+}
+
 pub fn update_gizmo_release(
     mut draggables: Query<(&Gizmo, &mut Draggable, &mut Handle<StandardMaterial>)>,
     mut selection_blockers: ResMut<SelectionBlockers>,
     gizmo_blockers: Res<GizmoBlockers>,
     mut gizmo_state: ResMut<GizmoState>,
     mouse_button_input: Res<Input<MouseButton>>,
     mut picked: ResMut<Picked>,
+    mut version_tracker: ResMut<RevisionTracker>,
+    mut change_history: ResMut<GizmoMoveUndoBuffer>,
+    last_dragged: Res<LastDraggedPos>,
 ) {
     let mouse_released = mouse_button_input.just_released(MouseButton::Left);
     let gizmos_blocked = gizmo_blockers.blocking();
     if mouse_released || gizmos_blocked {
-        if let GizmoState::Dragging(e) = *gizmo_state {
-            if let Ok((gizmo, mut draggable, mut material)) = draggables.get_mut(e) {
+        if let GizmoState::Dragging(info) = *gizmo_state {
+            if let Ok((gizmo, mut draggable, mut material)) = draggables.get_mut(info.entity) {
+                change_history.revisions.insert(
+                    version_tracker.get_next_revision(),
+                    GizmoMoveChange {
+                        entity: draggable.for_entity,
+                        prev_pos: info.start_pos,
+                        dest_pos: last_dragged.transform,
+                    },
+                );
                 draggable.drag = None;
                 if let Some(gizmo_materials) = &gizmo.materials {
                     *material = gizmo_materials.passive.clone();
@@ -398,6 +471,7 @@ pub fn update_drag_motions(
     cameras: Query<&Camera>,
     camera_controls: Res<CameraControls>,
     drag_state: Res<GizmoState>,
+    mut last_dragged: ResMut<LastDraggedPos>,
     mut cursor_motion: EventReader<CursorMoved>,
     mut move_to: EventWriter<MoveTo>,
     primary_window: Query<&Window, With<PrimaryWindow>>,
@@ -432,7 +506,7 @@ pub fn update_drag_motions(
             return;
         };
 
-        if let Ok((axis, draggable, drag_tf)) = drag_axis.get(dragging) {
+        if let Ok((axis, draggable, drag_tf)) = drag_axis.get(dragging.entity) {
             if let Some(initial) = &draggable.drag {
                 let n = if axis.frame.is_local() {
                     drag_tf
@@ -468,7 +542,7 @@ pub fn update_drag_motions(
             }
         }
 
-        if let Ok((plane, draggable, drag_tf)) = drag_plane.get(dragging) {
+        if let Ok((plane, draggable, drag_tf)) = drag_plane.get(dragging.entity) {
             if let Some(initial) = &draggable.drag {
                 let n_p = if plane.frame.is_local() {
                     drag_tf
@@ -492,6 +566,9 @@ pub fn update_drag_motions(
                 let tf_goal = initial
                     .tf_for_entity_global
                     .with_translation(initial.tf_for_entity_global.translation + delta);
+                last_dragged.transform = Transform::from_matrix(
+                    (initial.tf_for_entity_parent_inv * tf_goal.compute_affine()).into(),
+                );
                 move_to.send(MoveTo {
                     entity: draggable.for_entity,
                     transform: Transform::from_matrix(

rmf_site_editor/src/interaction/mod.rs

@@ -155,6 +155,8 @@ impl Plugin for InteractionPlugin {
             .init_resource::<GizmoState>()
             .init_resource::<CurrentEditDrawing>()
             .init_resource::<CurrentLevel>()
+            .init_resource::<GizmoMoveUndoBuffer>()
+            .init_resource::<LastDraggedPos>()
             .insert_resource(HighlightAnchors(false))
             .add_event::<ChangePick>()
             .add_event::<MoveTo>()
@@ -209,7 +211,8 @@ impl Plugin for InteractionPlugin {
                         update_highlight_visualization.after(SelectionServiceStages::Select),
                         update_cursor_hover_visualization.after(SelectionServiceStages::Select),
                         update_gizmo_click_start.after(SelectionServiceStages::Select),
-                        update_gizmo_release,
+                        update_gizmo_release.after(update_gizmo_click_start),
+                        undo_gizmo_change,
                         update_drag_motions
                             .after(update_gizmo_click_start)
                             .after(update_gizmo_release),

rmf_site_editor/src/site/change_plugin.rs

@@ -17,7 +17,9 @@
 
 use crate::site::SiteUpdateSet;
 use bevy::prelude::*;
-use std::fmt::Debug;
+use std::{fmt::Debug, sync::Arc};
+
+use super::{RevisionTracker, UndoEvent};
 
 /// The Change component is used as an event to indicate that the value of a
 /// component should change for some entity. Using these events instead of
@@ -58,28 +60,96 @@ impl<T: Component + Clone + Debug> Default for ChangePlugin<T> {
     }
 }
 
+/// This is a changelog used for the undo/redo system
+/// within the change plugin.
+struct ChangeLog<T: Component + Clone + Debug> {
+    entity: Entity,
+    from: Option<T>,
+    to: T,
+}
+
+/// This buffer stores the history of changes
+#[derive(Resource)]
+struct ChangeHistory<T: Component + Clone + Debug> {
+    pub(crate) revisions: std::collections::HashMap<usize, ChangeLog<T>>,
+}
+
+impl<T: Component + Clone + Debug> Default for ChangeHistory<T> {
+    fn default() -> Self {
+        Self {
+            revisions: Default::default(),
+        }
+    }
+}
+
 impl<T: Component + Clone + Debug> Plugin for ChangePlugin<T> {
     fn build(&self, app: &mut App) {
-        app.add_event::<Change<T>>().add_systems(
-            PreUpdate,
-            update_changed_values::<T>.in_set(SiteUpdateSet::ProcessChanges),
-        );
+        app.add_event::<Change<T>>()
+            .init_resource::<ChangeHistory<T>>()
+            .add_systems(
+                PreUpdate,
+                (
+                    update_changed_values::<T>.in_set(SiteUpdateSet::ProcessChanges),
+                    undo_change::<T>.in_set(SiteUpdateSet::ProcessChanges),
+                ), // TODO do this on another stage
+            );
+    }
+}
+
+fn undo_change<T: Component + Clone + Debug>(
+    mut commands: Commands,
+    mut values: Query<&mut T>,
+    change_history: ResMut<ChangeHistory<T>>,
+    mut undo_cmds: EventReader<UndoEvent>,
+) {
+    for undo in undo_cmds.read() {
+        let Some(change) = change_history.revisions.get(&undo.action_id) else {
+            continue;
+        };
+
+        if let Ok(mut component_to_change) = values.get_mut(change.entity) {
+            if let Some(old_value) = &change.from {
+                *component_to_change = old_value.clone();
+            } else {
+                commands.entity(change.entity).remove::<T>();
+            }
+        } else {
+            error!("Undo history corrupted.");
+        }
     }
 }
 
 fn update_changed_values<T: Component + Clone + Debug>(
     mut commands: Commands,
     mut values: Query<&mut T>,
     mut changes: EventReader<Change<T>>,
+    mut undo_buffer: ResMut<RevisionTracker>,
+    mut change_history: ResMut<ChangeHistory<T>>,
 ) {
     for change in changes.read() {
-        if let Ok(mut new_value) = values.get_mut(change.for_element) {
-            *new_value = change.to_value.clone();
+        if let Ok(mut component_to_change) = values.get_mut(change.for_element) {
+            change_history.revisions.insert(
+                undo_buffer.get_next_revision(),
+                ChangeLog {
+                    entity: change.for_element,
+                    to: change.to_value.clone(),
+                    from: Some(component_to_change.clone()),
+                },
+            );
+            *component_to_change = change.to_value.clone();
         } else {
             if change.allow_insert {
                 commands
                     .entity(change.for_element)
                     .insert(change.to_value.clone());
+                change_history.revisions.insert(
+                    undo_buffer.get_next_revision(),
+                    ChangeLog {
+                        entity: change.for_element,
+                        to: change.to_value.clone(),
+                        from: None,
+                    },
+                );
             } else {
                 error!(
                     "Unable to change {} data to {:?} for entity {:?} \

rmf_site_editor/src/site/mod.rs

@@ -114,6 +114,9 @@ pub use site_visualizer::*;
 pub mod texture;
 pub use texture::*;
 
+pub mod undo_plugin;
+pub use undo_plugin::*;
+
 pub mod util;
 pub use util::*;
 
@@ -220,6 +223,7 @@ impl Plugin for SitePlugin {
             ChangePlugin::<Scale>::default(),
             ChangePlugin::<Distance>::default(),
             ChangePlugin::<Texture>::default(),
+            UndoPlugin::default(),
         ))
         .add_plugins((
             ChangePlugin::<DoorType>::default(),