Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor optional model properties #263

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4f8f1ba
Remove MobileRobotMarker from OptionalModelProperties
xiyuoh Jan 2, 2025
2923bf6
Use Mobility struct for saving
xiyuoh Jan 2, 2025
5b35ec3
Update Tasks for saving
xiyuoh Jan 2, 2025
f65bbda
Change MobileRobotMarker to RobotMarker
xiyuoh Jan 3, 2025
e33e360
Merge branch 'main' into xiyu/optional_model_properties
xiyuoh Jan 9, 2025
4de0f3a
Add Mobility to ModelDescriptionBundle
xiyuoh Jan 9, 2025
b3111a2
Use ComponentId for ModelProperty and update Mobility
xiyuoh Jan 10, 2025
a312a10
Refactor Tasks
xiyuoh Jan 13, 2025
e332401
Interactoins with locations for GoToPlace tasks
xiyuoh Jan 14, 2025
88d0ed1
Cleanup
xiyuoh Jan 14, 2025
19a8274
Insert RobotMarker
xiyuoh Jan 14, 2025
3274b60
Remove task-based interactions and GoToPlace NameInSite
xiyuoh Jan 17, 2025
edd54cc
Combine RobotMarker and OptionalModelData into Robot
xiyuoh Jan 17, 2025
dcdf8b3
Add draft collision
xiyuoh Jan 17, 2025
3fae5da
Make the inner bundle of Parented transparent
mxgrey Jan 20, 2025
a8ff8d7
Use prerelease of ron
mxgrey Jan 20, 2025
c69b6ce
Fix style
mxgrey Jan 20, 2025
6c09af4
Set up RobotPropertyData to store property kinds and move CollisionKinds
xiyuoh Jan 21, 2025
98f3dba
Move MobilityKinds into RobotPropertyData
xiyuoh Jan 21, 2025
5652be2
Fix vec2
xiyuoh Jan 21, 2025
538ebd8
Add InspectRobotPropertyPlugin for common use
xiyuoh Jan 21, 2025
7985a80
Enable visualize circle collision
xiyuoh Jan 21, 2025
d93a4db
Move Tasks into Site and remove OptionalModelData
xiyuoh Jan 23, 2025
a173474
Merge remote-tracking branch 'origin/transparent_parented' into xiyu/…
xiyuoh Jan 23, 2025
bc769fa
Cleanup
xiyuoh Jan 23, 2025
3b1e85b
Address comments
xiyuoh Jan 28, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 23 additions & 39 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion rmf_site_editor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
33 changes: 9 additions & 24 deletions rmf_site_editor/src/interaction/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,19 @@ 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
{
let mut is_description_selected = false;
if let Some(description_entity) = affiliation.0 {
if let Ok((_, description_selected, description_hovered)) =
model_descriptions.get(description_entity)
Expand All @@ -52,7 +54,6 @@ pub fn update_model_instance_visual_cues(
instance_selected
.support_selected
.insert(description_entity);
is_description_selected = true;
} else {
instance_selected
.support_selected
Expand All @@ -72,34 +73,18 @@ pub fn update_model_instance_visual_cues(
if let Some(tasks) = tasks {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this isn't hurting anything, but I don't believe any of this block is needed any longer.

// 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)
{
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);
}
}
}
// TODO(@xiyuoh) support task-based visual cues
}
}

// When instances are removed, prevent any location from supporting them
for removed in removed_components.read() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed any longer either.

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);
}
Expand Down
38 changes: 17 additions & 21 deletions rmf_site_editor/src/site/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,18 +348,21 @@ 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,
};
}
}

for (robot_id, robot_data) in &site_data.robots {
// Robot IDs are pointing to model description entities
if let Some(model_description_entity) = id_to_entity
.get(robot_id)
.filter(|e| model_description_to_source.contains_key(*e))
{
commands
.entity(*model_description_entity)
.insert(ModelProperty(robot_data.clone()));
} else {
// TODO(@xiyuoh) consider creating an entity if not found
error!("Robot is pointing to a non-existent model description!");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's include the robot_id and robot_data values in this message.

};
}

for (model_instance_id, parented_model_instance) in &site_data.model_instances {
Expand Down Expand Up @@ -396,15 +399,8 @@ 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())
}
_ => continue,
};
if let Some(tasks) = site_data.tasks.get(model_instance_id) {
commands.entity(model_instance_entity).insert(tasks.clone());
}
}

Expand Down
Loading
Loading