diff --git a/docs/developer_guide/building.md b/docs/developer_guide/building.md new file mode 100644 index 00000000..d982598a --- /dev/null +++ b/docs/developer_guide/building.md @@ -0,0 +1,7 @@ +# Windows + +# Mac-OS + +# Linux (Ubuntu) + +# Web \ No newline at end of file diff --git a/docs/developer_guide/index.md b/docs/developer_guide/index.md new file mode 100644 index 00000000..1833a1ec --- /dev/null +++ b/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) diff --git a/docs/developer_guide/undo.md b/docs/developer_guide/undo.md new file mode 100644 index 00000000..caefeb16 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/rmf_site_editor/src/interaction/gizmo.rs b/rmf_site_editor/src/interaction/gizmo.rs index dc537d7f..04cfab1f 100644 --- a/rmf_site_editor/src/interaction/gizmo.rs +++ b/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,6 +389,39 @@ 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, +} + +pub(crate) fn undo_gizmo_change( + change_history: ResMut, + mut undo_cmds: EventReader, + mut move_to: EventWriter, +) { + 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)>, mut selection_blockers: ResMut, @@ -367,12 +429,23 @@ pub fn update_gizmo_release( mut gizmo_state: ResMut, mouse_button_input: Res>, mut picked: ResMut, + mut version_tracker: ResMut, + mut change_history: ResMut, + last_dragged: Res, ) { 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, drag_state: Res, + mut last_dragged: ResMut, mut cursor_motion: EventReader, mut move_to: EventWriter, primary_window: Query<&Window, With>, @@ -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( diff --git a/rmf_site_editor/src/interaction/mod.rs b/rmf_site_editor/src/interaction/mod.rs index 9fdb1bff..3407f79d 100644 --- a/rmf_site_editor/src/interaction/mod.rs +++ b/rmf_site_editor/src/interaction/mod.rs @@ -155,6 +155,8 @@ impl Plugin for InteractionPlugin { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() + .init_resource::() .insert_resource(HighlightAnchors(false)) .add_event::() .add_event::() @@ -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), diff --git a/rmf_site_editor/src/site/change_plugin.rs b/rmf_site_editor/src/site/change_plugin.rs index cfc6c826..08051fa3 100644 --- a/rmf_site_editor/src/site/change_plugin.rs +++ b/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,12 +60,62 @@ impl Default for ChangePlugin { } } +/// This is a changelog used for the undo/redo system +/// within the change plugin. +struct ChangeLog { + entity: Entity, + from: Option, + to: T, +} + +/// This buffer stores the history of changes +#[derive(Resource)] +struct ChangeHistory { + pub(crate) revisions: std::collections::HashMap>, +} + +impl Default for ChangeHistory { + fn default() -> Self { + Self { + revisions: Default::default(), + } + } +} + impl Plugin for ChangePlugin { fn build(&self, app: &mut App) { - app.add_event::>().add_systems( - PreUpdate, - update_changed_values::.in_set(SiteUpdateSet::ProcessChanges), - ); + app.add_event::>() + .init_resource::>() + .add_systems( + PreUpdate, + ( + update_changed_values::.in_set(SiteUpdateSet::ProcessChanges), + undo_change::.in_set(SiteUpdateSet::ProcessChanges), + ), // TODO do this on another stage + ); + } +} + +fn undo_change( + mut commands: Commands, + mut values: Query<&mut T>, + change_history: ResMut>, + mut undo_cmds: EventReader, +) { + 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::(); + } + } else { + error!("Undo history corrupted."); + } } } @@ -71,15 +123,33 @@ fn update_changed_values( mut commands: Commands, mut values: Query<&mut T>, mut changes: EventReader>, + mut undo_buffer: ResMut, + mut change_history: ResMut>, ) { 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 {:?} \ diff --git a/rmf_site_editor/src/site/mod.rs b/rmf_site_editor/src/site/mod.rs index 10f3be71..4ced7335 100644 --- a/rmf_site_editor/src/site/mod.rs +++ b/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::::default(), ChangePlugin::::default(), ChangePlugin::::default(), + UndoPlugin::default(), )) .add_plugins(( ChangePlugin::::default(), diff --git a/rmf_site_editor/src/site/undo_plugin.rs b/rmf_site_editor/src/site/undo_plugin.rs new file mode 100644 index 00000000..6ab35b6c --- /dev/null +++ b/rmf_site_editor/src/site/undo_plugin.rs @@ -0,0 +1,88 @@ +use bevy::{ecs::event, prelude::*}; +use bevy_impulse::event_streaming_service; +use std::fmt::Debug; + +use crate::{EditMenu, MenuEvent, MenuItem}; + +/// This represents an undo event. If you want to implement some form of undo-able +/// action, your plugin should watch for this event. Specifically, we should be +#[derive(Event, Debug, Clone, Copy)] +pub struct UndoEvent { + /// The action id that should be reverted + pub action_id: usize, +} + +/// This struct represents the undo menu item. +#[derive(Resource)] +struct UndoMenu { + menu_entity: Entity, +} + +/// This is responsible for drawing the undo GUI menu +///TODO(arjo): Decouple +impl FromWorld for UndoMenu { + fn from_world(world: &mut World) -> Self { + let undo_label = world.spawn(MenuItem::Text("Undo".into())).id(); + let edit_menu = world.resource::().get(); + world.entity_mut(edit_menu).push_children(&[undo_label]); + Self { + menu_entity: undo_label, + } + } +} + +/// This item watches the GUI menu to detect if an undo action was clicked. +fn watch_undo_click( + mut reader: EventReader, + menu_handle: Res, + mut undo_buffer: ResMut, + mut event_writer: EventWriter, +) { + for menu_click in reader.read() { + if menu_click.clicked() && menu_click.source() == menu_handle.menu_entity { + let Some(undo_item) = undo_buffer.undo_last() else { + continue; + }; + event_writer.send(UndoEvent { + action_id: undo_item, + }); + } + } +} + +/// The RevisionTracker resource is used to manage the undo +/// stack. When a plugin wishes to commit an action it should request anew revision +/// from the RevisionTracker. It is up to the plugin to maintain its +/// own history of what state changes ocurred. +#[derive(Resource, Default)] +pub struct RevisionTracker { + pub(crate) prev_value: usize, + pub(crate) undo_stack: Vec, +} + +impl RevisionTracker { + /// Request a new revision id. This is associated with the action you sent + pub fn get_next_revision(&mut self) -> usize { + self.prev_value += 1; + self.undo_stack.push(self.prev_value); + self.prev_value + } + + /// Get the last item that was supposed to be undone. + pub(crate) fn undo_last(&mut self) -> Option { + self.undo_stack.pop() + } +} + +#[derive(Default)] +pub struct UndoPlugin; + +impl Plugin for UndoPlugin { + fn build(&self, app: &mut App) { + app.init_resource::() + .init_resource::() + .init_resource::() + .add_event::() + .add_systems(Update, (watch_undo_click)); + } +} diff --git a/rmf_site_editor/src/widgets/inspector/inspect_anchor.rs b/rmf_site_editor/src/widgets/inspector/inspect_anchor.rs index af51cdcf..327717af 100644 --- a/rmf_site_editor/src/widgets/inspector/inspect_anchor.rs +++ b/rmf_site_editor/src/widgets/inspector/inspect_anchor.rs @@ -150,7 +150,6 @@ fn impl_inspect_anchor( ui.add(DragValue::new(&mut y).speed(0.01)); if x != tf.translation.x || y != tf.translation.y { - {} params.move_to.send(MoveTo { entity: id, transform: Transform::from_translation([x, y, 0.0].into()), diff --git a/rmf_site_editor/src/widgets/menu_bar.rs b/rmf_site_editor/src/widgets/menu_bar.rs index 193d9475..8e1e2e9d 100644 --- a/rmf_site_editor/src/widgets/menu_bar.rs +++ b/rmf_site_editor/src/widgets/menu_bar.rs @@ -183,6 +183,29 @@ impl FromWorld for ViewMenu { } } +#[derive(Resource)] +pub struct EditMenu { + /// Map of menu items + menu_item: Entity, +} + +impl EditMenu { + pub fn get(&self) -> Entity { + return self.menu_item; + } +} + +impl FromWorld for EditMenu { + fn from_world(world: &mut World) -> Self { + let menu_item = world + .spawn(Menu { + text: "Edit".to_string(), + }) + .id(); + Self { menu_item } + } +} + #[non_exhaustive] #[derive(Event)] pub enum MenuEvent { @@ -304,6 +327,7 @@ fn top_menu_bar( true, ); }); + ui.menu_button("View", |ui| { render_sub_menu( ui,