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

ReflectComponent Commands #5541

Closed

Conversation

PROMETHIA-27
Copy link
Contributor

@PROMETHIA-27 PROMETHIA-27 commented Aug 1, 2022

(Note: Primary changes are in bevy_ecs/src/reflect.rs, almost every other file just has FromReflect derives added)

Objective

Currently, ReflectComponent (and ReflectResource) contain a series of functions that require a &mut World in order to use them. This severely affects the parallelism of using these reflection tools and limits user's interest in them. It also leads to more convoluted code when having to request a lot of resources for a system that could otherwise take advantage of the SystemParam system. This PR is an attempt to improve this API by adding functions to ReflectComponent and ReflectResource that take Commands instead of &mut World.

Solution

The most significant change that needs to happen to support this API update is that instead of implementing FromType<C> for ReflectComponent/Resource for C: Component + Reflect + FromWorld, we implement for C: Component + Reflect + FromReflect. In some ways this might be considered more permissive, but in others more restrictive. Some types may be constructible with FromWorld but not FromReflect, and vice versa. I believe that overall this change is more permissive than not, especially as types that cannot be constructed from a reflected value are more likely to behave unintuitively in reflection-heavy systems. A more impactful issue with this is that now many types will need #[derive(FromReflect)] added in order to work with ReflectComponent/Resource, which may be addressed in a different PR (opt-out FromReflect?).


Changelog

Added:

  • insert_command function to ReflectComponent
  • FromReflect derives to roughly 50-100 bevy types which used #[reflect(Component)]

Changed:

  • FromWorld bound on ReflectComponent's FromType impl to FromReflect

Migration Guide

  • If a type that previously used #[reflect(Component)] or #[reflect(Resource)] does not have #[derive(FromReflect)], it needs that added. A small amount of complex types may not be capable of this, and will need to be changed.

Copy link
Member

@MrGVSV MrGVSV left a comment

Choose a reason for hiding this comment

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

Code looks fine to me. The one thing I'm hesitant with is that this removes the ability for components to initialize from the world, right? I haven't played around too much with scenes, but is this something that's used often? How much are we losing by removing that ability?

crates/bevy_ecs/src/reflect.rs Outdated Show resolved Hide resolved
@alice-i-cecile alice-i-cecile added C-Usability A simple quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types labels Aug 3, 2022
@zicklag
Copy link
Member

zicklag commented Aug 9, 2022

I haven't looked at this implementation yet, and I'm not sure if this was considered, but I've used two different strategies that worked for inserting dynamic components using commands.

Using commands.add()

This works in Bevy today, with no changes. For example:

commands.add(move |world: &mut World| {
    reflect_components.insert(world, reflect_data);
});

Using A Custom Command Implementation

This file has an example of a custom command that weighs only 38 lines, is 100% independent of the rest of Bevy code, and lets you do this:

commands.insert_dynamic(reflect_components, reflect_data);

@nicopap
Copy link
Contributor

nicopap commented Jul 31, 2023

Probably superseded by #8895 should this be closed?

@PROMETHIA-27
Copy link
Contributor Author

Sounds good to me! I forgot this even existed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Usability A simple quality-of-life change that makes Bevy easier to use
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants