bevy_editor_pls icon indicating copy to clipboard operation
bevy_editor_pls copied to clipboard

Feature request: Add/remove component buttons

Open PROMETHIA-27 opened this issue 3 years ago • 11 comments

I think that the editor being able to manipulate the components of entities- and display all available ones- is a vital feature which is missing currently. I'd be happy to make a PR for this, I just wanted to make an issue first in case you were working on this already. The UI for it should be easy, the biggest problem I encountered was being able to iterate all the component types available. I've found out that any type that has #[reflect(Component)] on it and has been registered (both necessary for being serialized as a scene) can be accessed like this:

let reg = app.world.get_resource::<bevy::reflect::TypeRegistry>().unwrap();
for ty in reg.read().iter() {
    if let Some(_) = ty.data::<ReflectComponent>() {
        println!("{}", ty.name());
    }
}

I'd be working on new-main rather than main, since I assume main is not going to be further developed until new-main is merged into it.

Should I go ahead and make a PR for this?

PROMETHIA-27 avatar Jan 20 '22 17:01 PROMETHIA-27

I think the reflection API isn't quite enough yet, because it only allows calling add_component with a &dyn Reflect, and there is no way to just insert the default value.

jakobhellermann avatar Jan 20 '22 17:01 jakobhellermann

I think I've solved it. To insert default value just do:

// reflect is a ReflectComponent
reflect.add_component(world, entity, &bevy::reflect::DynamicStruct::default());

I was able to add a component with an i32 to an entity with almost this exact code, and the i32 defaulted to 0. This also works for empty struct components, but crashes for tuplestruct components. DynamicTupleStruct also does not work for tuplestruct components, which is really confusing. I think there may be a bug somewhere but I don't understand how. Still looking into that.

EDIT: Nvm, it works perfectly fine on tuple struct components with DynamicTupleStruct. The problem is that loading a scene file with a tuple struct component in it crashes, as it invariably uses DynamicStruct.

EDIT 2: Nvm again, I'm just bad at writing the .scn.ron files. If you use "tuple_struct" instead of "struct" it works fine. Feel like that should be a more obvious error than a panic with 'Attempted to apply non-TupleStruct type to TupleStruct type.' though.

EDIT 3: It's also extremely easy to not do default values:

reflect.insert("value", 12i32);

and now the i32 on the struct is 12. I think should work very well, as you only need a type id to access everything necessary to do this. Would be easy enough to store all components' names and ids in a resource I think.

PROMETHIA-27 avatar Jan 20 '22 19:01 PROMETHIA-27

The problem with that trick is that there is no way to find out if DynamicStruct or DynamicTupleStruct (or DynamicEnum in the future) are the correct ones to use just from a type id.

Another solution could be to add a #[reflect(Default)] to bevy which will store a way to get a Box<dyn Reflect> for the type. Then you could pass that to add_component.

Would be easy enough to store all components' names and ids in a resource I think.

Both names and type ids are already in the TypeRegistryArc resource so that should be enough.

jakobhellermann avatar Jan 20 '22 21:01 jakobhellermann

I opened a PR for #[reflect(Default)]: https://github.com/bevyengine/bevy/pull/3733

jakobhellermann avatar Jan 20 '22 22:01 jakobhellermann

I don't like that solution much as it's even more things to annotate types with, especially when reflect already has a trait bound for default. I think it would be better to just expose an enum of Struct | TupleStruct | Enum on the TypeRegistration, or maybe on the ReflectComponent itself. Plus that would increase compatibility with any crates that forget to add Default to their reflect tag, on top of deriving reflect + reflecting component + registering the type.

PROMETHIA-27 avatar Jan 20 '22 23:01 PROMETHIA-27

I don't know if we can rely on the FromWorld bound always being on a component, I believe it was only added as a necessity as there is no other way of creating a component from a &dyn Reflect.

Someone on discord said

I strongly believe Default should not be required for component (and I'd love to get rid of the FromWorld requirement)

so having an explicit #[reflect(Default)] - while boilerplatey - is probably the way to go for now IMO.

jakobhellermann avatar Jan 21 '22 15:01 jakobhellermann

Also I finished up and pushed the code for an Add menu that lets you spawn new bundles into the world: https://github.com/jakobhellermann/bevy_editor_pls/commit/b35bcc2efa76b8e6f5e488721192d37e7eeabf0b

It doesn't use reflection and will only work for manually registered bundles but I'm linking this here since there's some overlap.

jakobhellermann avatar Jan 21 '22 15:01 jakobhellermann

Ah, that's a good point, fair enough. I suppose anything that doesn't fulfill everything on its own could probably be newtyped. I'll continue with an add component button branch- I'll just default to DynamicStruct and update it to the proper reflection code when that's available. Could also work on transform gizmos after, those just need that one crate and link its selection to inspector selection, both use the same picking crate underneath I believe.

PROMETHIA-27 avatar Jan 21 '22 15:01 PROMETHIA-27

Yeah I think the DynamicStruct workaround is definitely worth merging and a good solution until bevy supports something else

jakobhellermann avatar Jan 21 '22 16:01 jakobhellermann

The gizmo crate uses bevy_mod_picking while this crate uses bevy_mod_raycast, which the picking crate is built on.

I don't know how flexible bevy_mod_picking and the gizmo crate is and I used the raycast one since I'm doing my own selection logic anyways, so that's something to investigate.

jakobhellermann avatar Jan 21 '22 16:01 jakobhellermann

could an add/remove Component button now be added that the changes have been merged ?

asdfsdfsdfzhjzu avatar Oct 21 '23 00:10 asdfsdfsdfzhjzu