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

Enable dynamic mutable access to component data #1284

Merged
merged 2 commits into from
Jan 22, 2021

Conversation

willcrichton
Copy link
Contributor

I'm working on developing a world visualizer/editor for Bevy. See a demo: https://discord.com/channels/691052431525675048/692648638823923732/801608361463513089

To work, the tool needs to get mutable access to every component in the world, as well as find a method for displaying the component in a UI. Right now, I'm using this strategy:

  1. Maintain a mapping from TypeId to pre-registered types that implement the Inspectable trait in bevy-inspector-egui (see here). For these types, I use archetype.get_dynamic to get the component data as *mut u8 then transmute it to the appropriate type.
  2. If Inspectable is not implemented, then try to cast the component to &mut dyn Reflect using ReflectComponent, then use bevy-inspector-egui to generate a generic UI for components implementing Reflect.

This PR adds two changes to make this possible. First, it publicly exposes Archetype::get_dynamic. Second, it adds a new method ReflectComponent::reflect_component_mut that returns &mut dyn Reflect instead of just &dyn Reflect.

Note: obviously the proliferation of unsafe code and transmute is concerning here. If there's a different approach that works without these changes, I'd be happy to hear.

@willcrichton willcrichton changed the title Enable dynamic component access Enable dynamic mutable access to component data Jan 22, 2021
@bjorn3
Copy link
Contributor

bjorn3 commented Jan 22, 2021

You should probably squash.

@@ -38,6 +39,17 @@ impl ReflectComponent {
(self.reflect_component)(archetype, entity_index)
}

/// # Safety
/// This does not do bound checks on entity_index. You must make sure entity_index is within bounds before calling.
/// To avoid having multiple mutable pointers to the same data, this method can only be called in a thread-local system.
Copy link
Member

@cart cart Jan 22, 2021

Choose a reason for hiding this comment

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

Its good to call out the "scheduler" side of things in the "Safety" section, but we should also call out generically that this could violate rust's mutability rules. Callers should verify that they are not violating the rules.

@cart cart merged commit 7166a28 into bevyengine:master Jan 22, 2021
rparrett pushed a commit to rparrett/bevy that referenced this pull request Jan 27, 2021
* Enable dynamic mutable access to component data

* Add clippy allowance, more documentation
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.

3 participants