Refactor optional model properties

rmf_site_editor/Cargo.toml

@@ -49,7 +49,6 @@ pathdiff = "*"
 ehttp = { version = "0.4", features = ["native-async"] }
 nalgebra = "0.32.5"
 anyhow = "*"
-strum = "*"
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 clap = { version = "4.0.10", features = ["color", "derive", "help", "usage", "suggestions"] }

rmf_site_editor/src/interaction/model.rs

@@ -33,12 +33,15 @@ pub fn update_model_instance_visual_cues(
             &mut Selected,
             &mut Hovered,
             &mut Affiliation<Entity>,
-            Option<Ref<Tasks<Entity>>>,
+            Option<Ref<Tasks>>,
         ),
         (With<ModelMarker>, Without<Group>),
     >,
-    mut locations: Query<(&mut Selected, &mut Hovered), (With<LocationTags>, Without<ModelMarker>)>,
-    mut removed_components: RemovedComponents<Tasks<Entity>>,
+    mut locations: Query<
+        (&NameInSite, &mut Selected, &mut Hovered),
+        (With<LocationTags>, Without<ModelMarker>),
+    >,
+    mut removed_components: RemovedComponents<Tasks>,
 ) {
     for (instance_entity, mut instance_selected, mut instance_hovered, affiliation, tasks) in
         &mut model_instances
@@ -72,25 +75,33 @@ pub fn update_model_instance_visual_cues(
         if let Some(tasks) = tasks {
             // When tasks for an instance have changed, reset all locations from supporting this instance
             if tasks.is_changed() {
-                for (mut location_selected, mut location_hovered) in locations.iter_mut() {
+                for (_, mut location_selected, mut location_hovered) in locations.iter_mut() {
                     location_selected.support_selected.remove(&instance_entity);
                     location_hovered.support_hovering.remove(&instance_entity);
                 }
             }
 
-            if let Some(task_location) = tasks.0.first().and_then(|t| t.location()) {
-                if let Ok((mut location_selected, mut location_hovered)) =
-                    locations.get_mut(task_location.0)
+            // Additional interaction for default tasks
+            if let Some(go_to_place) = tasks
+                .0
+                .first()
+                .and_then(|t| serde_json::from_value::<GoToPlace>(t.config.clone()).ok())
+            {
+                for (location_name, mut location_selected, mut location_hovered) in
+                    locations.iter_mut()
                 {
-                    if instance_selected.cue() && !is_description_selected {
-                        location_selected.support_selected.insert(instance_entity);
-                    } else {
-                        location_selected.support_selected.remove(&instance_entity);
-                    }
-                    if instance_hovered.cue() {
-                        location_hovered.support_hovering.insert(instance_entity);
-                    } else {
-                        location_hovered.support_hovering.remove(&instance_entity);
+                    if location_name == &go_to_place.location {
+                        if instance_selected.cue() && !is_description_selected {
+                            location_selected.support_selected.insert(instance_entity);
+                        } else {
+                            location_selected.support_selected.remove(&instance_entity);
+                        }
+                        if instance_hovered.cue() {
+                            location_hovered.support_hovering.insert(instance_entity);
+                        } else {
+                            location_hovered.support_hovering.remove(&instance_entity);
+                        }
+                        break;
                     }
                 }
             }
@@ -99,7 +110,7 @@ pub fn update_model_instance_visual_cues(
 
     // When instances are removed, prevent any location from supporting them
     for removed in removed_components.read() {
-        for (mut location_selected, mut location_hovered) in locations.iter_mut() {
+        for (_, mut location_selected, mut location_hovered) in locations.iter_mut() {
             location_selected.support_selected.remove(&removed);
             location_hovered.support_hovering.remove(&removed);
         }

rmf_site_editor/src/site/load.rs

@@ -348,17 +348,17 @@ fn generate_site_entities(
         model_description_dependents.insert(model_description_entity, HashSet::new());
         model_description_to_source
             .insert(model_description_entity, model_description.source.0.clone());
-        // Insert optional model properties
-        for optional_property in &model_description.optional_properties.0 {
-            match optional_property {
-                OptionalModelProperty::DifferentialDrive(diff_drive) => commands
-                    .entity(model_description_entity)
-                    .insert(ModelProperty(diff_drive.clone())),
-                OptionalModelProperty::MobileRobotMarker(robot_marker) => commands
-                    .entity(model_description_entity)
-                    .insert(ModelProperty(robot_marker.clone())),
-                _ => continue,
-            };
+        // Insert optional model data
+        if let serde_json::Value::Object(map) = &model_description.optional_data.0 {
+            for (k, v) in map.iter() {
+                if *k == Mobility::label() {
+                    if let Ok(mobility) = serde_json::from_value::<Mobility>(v.clone()) {
+                        commands
+                            .entity(model_description_entity)
+                            .insert(ModelProperty(mobility));
+                    }
+                }
+            }
         }
     }
 
@@ -396,15 +396,20 @@ fn generate_site_entities(
                 model_instance.name.0,
             );
         }
-
-        // Insert optional model properties
-        for optional_property in &model_instance.optional_properties.0 {
-            match optional_property {
-                OptionalModelProperty::Tasks(tasks) => {
-                    commands.entity(model_instance_entity).insert(tasks.clone())
+        // Insert optional model data
+        if let serde_json::Value::Object(map) = &model_instance.optional_data.0 {
+            for (k, v) in map.iter() {
+                if *k == Tasks::label() {
+                    if let Ok(tasks) = serde_json::from_value::<Tasks>(v.clone()) {
+                        commands.entity(model_instance_entity).insert(tasks);
+                        if let Some(description_entity) = model_instance.description.0 {
+                            commands
+                                .entity(description_entity)
+                                .insert(ModelProperty(RobotMarker));
+                        }
+                    }
                 }
-                _ => continue,
-            };
+            }
         }
     }
 

rmf_site_editor/src/site/save.rs

@@ -1200,7 +1200,7 @@ fn migrate_relative_paths(
 fn generate_model_descriptions(
     site: Entity,
     world: &mut World,
-) -> Result<BTreeMap<u32, ModelDescriptionBundle<u32>>, SiteGenerationError> {
+) -> Result<BTreeMap<u32, ModelDescriptionBundle>, SiteGenerationError> {
     let mut state: SystemState<(
         Query<
             (
@@ -1213,13 +1213,14 @@ fn generate_model_descriptions(
             (With<ModelMarker>, With<Group>, Without<Pending>),
         >,
         Query<&Children>,
-        // Optional model properties
-        Query<&ModelProperty<DifferentialDrive>>,
-        Query<&ModelProperty<MobileRobotMarker>>,
+        // Optional model data
+        // TODO(@xiyuoh) store property data in optional_data instead of querying
+        // on save or query ModelProperty<T> instead but exclude required
+        Query<&ModelProperty<Mobility>>,
     )> = SystemState::new(world);
-    let (model_descriptions, children, differential_drive, robot_marker) = state.get(world);
+    let (model_descriptions, children, mobility) = state.get(world);
 
-    let mut res = BTreeMap::<u32, ModelDescriptionBundle<u32>>::new();
+    let mut res = BTreeMap::<u32, ModelDescriptionBundle>::new();
     if let Ok(children) = children.get(site) {
         for child in children.iter() {
             if let Ok((site_id, name, source, is_static, scale)) = model_descriptions.get(*child) {
@@ -1230,16 +1231,14 @@ fn generate_model_descriptions(
                     scale: scale.clone(),
                     ..Default::default()
                 };
-                if let Ok(diff_drive) = differential_drive.get(*child) {
-                    desc_bundle.optional_properties.0.push(
-                        OptionalModelProperty::DifferentialDrive(diff_drive.0.clone()),
-                    );
-                };
-                if let Ok(mobile_robot) = robot_marker.get(*child) {
-                    desc_bundle.optional_properties.0.push(
-                        OptionalModelProperty::MobileRobotMarker(mobile_robot.0.clone()),
-                    );
-                };
+                let mut optional_data = serde_json::Map::new();
+                if let Ok(mobility_property) = mobility.get(*child) {
+                    if let Ok(mobility_data) = serde_json::to_value(mobility_property.0.clone()) {
+                        optional_data.insert(Mobility::label(), mobility_data.into());
+                    }
+                }
+                desc_bundle.optional_data = OptionalModelData(optional_data.into());
+
                 res.insert(site_id.0, desc_bundle);
             }
         }
@@ -1258,13 +1257,11 @@ fn generate_model_instances(
             (With<ModelMarker>, Without<Group>, Without<Pending>),
         >,
         Query<(Entity, &SiteID), With<LevelElevation>>,
-        Query<(&Point<Entity>, &SiteID), (With<LocationTags>, Without<Pending>)>,
         Query<&Children>,
         Query<&Parent>,
-        Query<&Tasks<Entity>, (With<MobileRobotMarker>, Without<Group>)>,
+        Query<&Tasks, (With<RobotMarker>, Without<Group>)>,
     )> = SystemState::new(world);
-    let (model_descriptions, model_instances, levels, locations, _, parents, tasks) =
-        state.get(world);
+    let (model_descriptions, model_instances, levels, _, parents, tasks) = state.get(world);
 
     let mut site_levels_ids = std::collections::HashMap::<Entity, u32>::new();
     for (level_entity, site_id) in levels.iter() {
@@ -1296,30 +1293,14 @@ fn generate_model_instances(
             ),
             ..Default::default()
         };
+        let mut optional_data = serde_json::Map::new();
         if let Ok(robot_tasks) = tasks.get(instance_entity) {
-            let tasks: Vec<Task<u32>> = robot_tasks
-                .0
-                .clone()
-                .iter()
-                .map(|task| match task {
-                    Task::GoToPlace(go_to_place) => locations
-                        .get(go_to_place.location.unwrap().0)
-                        .map(|(_, location_id)| {
-                            Task::GoToPlace(GoToPlace {
-                                location: Some(Point(location_id.0)),
-                            })
-                        })
-                        .unwrap(),
-                    Task::WaitFor(wait_for) => Task::WaitFor(WaitFor {
-                        duration: wait_for.duration.clone(),
-                    }),
-                })
-                .collect::<Vec<Task<u32>>>();
-            model_instance
-                .optional_properties
-                .0
-                .push(OptionalModelProperty::Tasks(Tasks(tasks.clone())));
+            if let Ok(tasks_data) = serde_json::to_value(robot_tasks.clone()) {
+                optional_data.insert("Tasks".to_string(), tasks_data.into());
+            }
         }
+        model_instance.optional_data = OptionalModelData(optional_data.into());
+
         res.insert(
             instance_id.0,
             Parented {

rmf_site_editor/src/widgets/creation.rs

@@ -248,15 +248,14 @@ impl<'w, 's> Creation<'w, 's> {
                                 };
                                 if ui.button(add_icon).clicked() {
                                     if let Some(site_entity) = self.current_workspace.root {
-                                        let model_description_bundle: ModelDescriptionBundle<
-                                            Entity,
-                                        > = ModelDescriptionBundle {
-                                            name: NameInSite(pending_model.name.clone()),
-                                            source: ModelProperty(pending_model.source.clone()),
-                                            is_static: ModelProperty(IsStatic::default()),
-                                            scale: ModelProperty(pending_model.scale.clone()),
-                                            ..Default::default()
-                                        };
+                                        let model_description_bundle: ModelDescriptionBundle =
+                                            ModelDescriptionBundle {
+                                                name: NameInSite(pending_model.name.clone()),
+                                                source: ModelProperty(pending_model.source.clone()),
+                                                is_static: ModelProperty(IsStatic::default()),
+                                                scale: ModelProperty(pending_model.scale.clone()),
+                                                ..Default::default()
+                                            };
                                         let description_entity = self
                                             .commands
                                             .spawn(model_description_bundle)

rmf_site_editor/src/widgets/fuel_asset_browser.rs

@@ -271,7 +271,7 @@ impl<'w, 's> FuelAssetBrowser<'w, 's> {
                 if let Some(selected) = &gallery_status.selected {
                     if ui.button("Load as Description").clicked() {
                         if let Some(site_entity) = self.current_workspace.root {
-                            let model_description: ModelDescriptionBundle<Entity> =
+                            let model_description: ModelDescriptionBundle =
                                 ModelDescriptionBundle {
                                     name: NameInSite(selected.owner.clone() + "/" + &selected.name),
                                     source: ModelProperty(AssetSource::Remote(

rmf_site_editor/src/widgets/inspector/inspect_asset_source.rs

@@ -137,6 +137,8 @@ impl<'a> InspectAssetSourceComponent<'a> {
                 ui.text_edit_singleline(path);
             }
         }
+        ui.add_space(10.0);
+
         if &new_source != self.source {
             Some(new_source)
         } else {