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

Add support for spawning robots in locations #123

Closed
wants to merge 3 commits into from

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented May 11, 2023

New feature implementation

Implemented feature

Spawn a robot visual in locations with a spawn_robot_name and spawn_robot_type property

Implementation description

The implementation is quite naive and currently just spawns a single static model in spawning locations, there is a TODO in place for future plans when robot behavior is added to the site editor, including making it non static and / or adding other components to distinguish models from robots.

While working on it I noticed that querying for whether a Location has a SpawnRobot tag is not terribly ergonomic since all tags are contained in a LocationTags component that is a Vec<LocationTag>, so each system dealing with location behavior will need to query for components with LocationTags, iterate over the whole vector and verify whether the location has the tag it needs (as is done here for SpawnRobot).

On the other hand, if each tag was a different component it would be possible to filter relevant entities at the query level, improving both performance and code simplicity.

Still, I kept this out of this PR since it would be quite a large refactor and didn't want to get into it in case it's not a good idea.

@luca-della-vedova luca-della-vedova marked this pull request as draft May 11, 2023 08:53
@luca-della-vedova luca-della-vedova marked this pull request as ready for review May 12, 2023 06:46
Base automatically changed from luca/workcell_editor to main May 12, 2023 12:46
Signed-off-by: Luca Della Vedova <[email protected]>
@luca-della-vedova
Copy link
Member Author

Note that there is currently a panic when removing then adding a spawn robot again. This panic is at the model.rs level and also appears when changing the AssetSource of models, I haven't figure that out yet but it is not strictly introduced in this PR.

@luca-della-vedova
Copy link
Member Author

Small followup, panic is related to #145 so it's about the AABB plugin

@luca-della-vedova
Copy link
Member Author

Closing for now. There will be much more work needed to support robots as shown in #153, while this PR only implements naive rendering

@luca-della-vedova luca-della-vedova deleted the feature/spawn_robots branch July 24, 2024 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants