Dynamic Component Id type #12794

ecoskey · 2024-03-30T05:50:05Z

Objective

Currently working with dynamic component ids requires a lot of manual pointer work and unsafe code, which seems unnecessary when dynamic components are registered from known rust code.

Solution

Add a wrapper around ComponentId with a type parameter T to act as a witness that that id corresponds to a component with type T. This allows registering multiple components with the same underlying type, that dynamic queries can access separately in a safe way.

Changelog

added TypedComponentId
added world_id() to all entity cells
added get_dynamic(), get_dynamic_mut(), into_dynamic_mut(), insert_dynamic() as appropriate to entity cells
added init_dynamic_component() to World
added new_dynamic() to ComponentDescriptor

note: name needs work - added TypedComponentId - added world_id() to all entity cells - added get_dynamic(), get_dynamic_mut(), into_dynamic_mut(), insert_dynamic() as appropriate to entity cells - added init_dynamic_component() to World - added new_dynamic() to ComponentDescriptor

thank you CI tests for telling me I am dum

jnhyatt · 2024-03-30T06:25:26Z

Worth considering, in my prototype I restrict T: Component which lets me choose the storage type from the component without having to specify it manually when creating a new dynamic component. It's such an often overlooked detail it feels weird to me to make people specify it. On the other hand there's definitely value to being able to use any type for the layout, but on the other hand you can always newtype. Restricting T: Component also has the benefit of being more explicit imo.

ecoskey · 2024-03-30T07:01:24Z

At least for me one of the key benefits of dynamic components is that they don't require newtyping and new variants can be made on the fly, hence the lack of a Component constraint.

SkiFire13

The approach looks good to me. Some things that I would also like to see:

some tests, both for the happy path and for when the component_id refers to a different world or a component that doesn't exist;
some examples on how to use this together with dynamic queries (it would be very nice if we could remove the unsafe from the ecs/dynamic example);
an API that works with untyped ComponentIds too (right now you have to carry T everywhere, which might not be wanted in some cases):
- for example this could be done with a method that upgrades a ComponentId to a TypedComponentId<T> by checking if the registered TypeId is T's.

crates/bevy_ecs/src/world/entity_ref.rs

SkiFire13 · 2024-03-30T09:15:20Z

crates/bevy_ecs/src/world/mod.rs

+    pub fn init_dynamic_component<T: Send + Sync + 'static>(
+        &mut self,
+        storage_type: StorageType,
+    ) -> TypedComponentId<T> {


Maybe you could have a init_dynamic_component that takes a T: Component and a init_dynamic_component_with_storage that only takes T: Send + Sync + 'static but requires specifying the storage?

the first just sounds like init_component. Should that just return the typed wrapper then, since the inner id is still accessible?

Changing init_component to return TypedComponent<T> might be controversial, although consistent with this change.

SkiFire13 · 2024-03-30T09:20:58Z

crates/bevy_ecs/src/world/entity_ref.rs

+        if self.world_id() != component_id.world_id {
+            panic!("Attempted to insert a dynamic component into the wrong World");
+        }


Just to be sure I would put a debug_assert! after all the world id checks that verifies that the type id of T is the same as the one in the ComponentDescriptor associated to component_id in this World.

debug_assert_eq!(self.components().get_info(component_id.id).and_then(|d| d.type_id()), Some(TypeId::of::<T>()));

SkiFire13 · 2024-03-30T09:26:54Z

crates/bevy_ecs/src/component.rs

+pub struct TypedComponentId<T: Send + Sync + 'static> {
+    pub(crate) id: ComponentId, //todo: pub(crate) fields and have the World construct this, or private fields and construct from an &mut World reference?
+    pub(crate) world_id: WorldId,
+    pub(crate) data: PhantomData<T>,
+}


This should document that a safety invariant of this type is that the world with id self.world_id contains a component with id self.id of type T.

SkiFire13 · 2024-03-30T09:38:07Z

At least for me one of the key benefits of dynamic components is that they don't require newtyping and new variants can be made on the fly, hence the lack of a Component constraint.

I wonder how much more ergonomic that is though, since you have to carry the TypedComponentId<T> everywhere to use them.

Also note that you won't need a newtype for each dynamic component. With a single newtype around e.g. a String you can create as many dynamic components that contain String as you want (as opposed to right now where you need a newtype per-component).

crates/bevy_ecs/src/system/commands/mod.rs

ecoskey · 2024-03-30T16:23:16Z

Also note that you won't need a newtype for each dynamic component. With a single newtype around e.g. a String you can create as many dynamic components that contain String as you want (as opposed to right now where you need a newtype per-component).

Taking that to the extreme, you would probably just end up with TableStorageComponent<> and SparseStorageComponent<> newtypes, which should probably just end up as parameters anyway. Unless I'm misunderstanding you (good chance of that tbh). In my experience dynamic components do require the id being passed around, but I don't see a good way to avoid that.

Edit: just saw your "upgrading" idea. Seems useful, but I don't think it's a full replacement since you'd still have to pass the id around, or use ComponentIdOf, which will ignore any dynamic components.

…t_ids

fix examples/dynamic.rs?

ecoskey · 2024-03-31T03:38:42Z

todo:

tests
fix examples/dynamic.rs
link to dynamic.rs from try_insert_dynamic and insert_dynamic doc comment

james7132 · 2024-03-31T11:32:08Z

What is the expected use case here? Untyped components were originally introduced as a way to enable scripting interfaces, but here you have a Rust type. Can you provide a end-user or library use case that requires what are effectively runtime-generated newtype components?

SkiFire13 · 2024-03-31T13:16:55Z

Untyped components were originally introduced as a way to enable scripting interfaces, but here you have a Rust type.

For example if you're implementing scripting using an interpreted language this would let you store multiple components that hold a your_scripting_api::Value

james-j-obrien · 2024-04-01T04:25:53Z

Can you provide a end-user or library use case that requires what are effectively runtime-generated newtype components?

I feel in many use cases for dynamic components we actually do have a known type we want to treat the component as when we access it. As mentioned above many scripting use cases will be defining many of it's components as a shared Value type, but also if you had a simple "Counter" system in a card game where there was a runtime defined set of counters you could put on a certain card entity and you want to be able to query for cards with a given counter on them it could be done entirely safely with an API like this. I think removing the unsafe barrier in as many cases is possible makes it more likely people will actually make use of those features.

james-j-obrien · 2024-04-01T07:24:30Z

My question for this implementation is if we have to check the world id anyway to ensure safety would it be worth it to drop the type wrapper all together and just check whether the type id matches the one you registered it with? That way you can just use component ids everywhere.

urben1680 · 2024-04-06T21:23:36Z

What is the expected use case here? Untyped components were originally introduced as a way to enable scripting interfaces, but here you have a Rust type. Can you provide a end-user or library use case that requires what are effectively runtime-generated newtype components?

The author linked this to me when I was asking on the Discord for having dynamic marker types. And I am sold on this as a solution for what I want to do.

My library offers a few SystemParams with non-pub Queries in them. Part of the logic requires marker components. But it is impossible for such a component to be unique to every usage of my SystemParam. So I want to use this feature here to newtype () (or maybe non-ZST types) and use it as that unique marker type.

It is still not easy for the SystemParam to learn what the ComponentId of the marker type ended up being. But when my SystemParam initializes it's params, my marker type will have put it's ComponentId in a resource, discoverable by the following logic of the SystemParams init. Once the param knows the ComponentId, it can create Commands that adds and removes them.

Still a bit hacky, probably not what the most obvious use case could be here, but a use case only possible with this.

jnhyatt · 2024-04-20T05:05:07Z

FYI, I ended up publishing my MVP as bevy_dyn_component. The proposed API is a bit different from this proposal, but has many similarities. The examples highlight one or two potential use cases for this as well, but I think most of them would ultimately be better as relations.

ecoskey · 2024-08-28T04:05:06Z

closing, was intended to solve a very niche issue that doesn't exist anymore (at least for me)

ecoskey added 4 commits March 29, 2024 22:09

add dynamic component commands

fb358eb

Merge branch 'bevyengine:main' into component_ids

6fe00ad

fix typo

dee8683

thank you CI tests for telling me I am dum

SkiFire13 reviewed Mar 30, 2024

View reviewed changes

crates/bevy_ecs/src/system/commands/mod.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/system/commands/mod.rs Outdated Show resolved Hide resolved

mnmaita added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events labels Mar 30, 2024

Merge branch 'main' into component_ids

29e65a3

ecoskey added 5 commits March 30, 2024 13:08

Merge branch 'main' into component_ids

b34093c

fix unsoundness

dff0147

Merge branch 'component_ids' of github.com:ecoskey/bevy into componen…

5a0bf5b

…t_ids

fix docs

9d9dbb4

fix examples/dynamic.rs?

Merge branch 'main' into component_ids

2f051f4

Merge branch 'main' into component_ids

638f6b1

ecoskey closed this Aug 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Component Id type #12794

Dynamic Component Id type #12794

ecoskey commented Mar 30, 2024

jnhyatt commented Mar 30, 2024

ecoskey commented Mar 30, 2024

SkiFire13 left a comment

SkiFire13 Mar 30, 2024

ecoskey Mar 30, 2024

SkiFire13 Mar 30, 2024

SkiFire13 Mar 30, 2024

ecoskey Mar 30, 2024

SkiFire13 Mar 30, 2024

SkiFire13 commented Mar 30, 2024

ecoskey commented Mar 30, 2024 •

edited

Loading

ecoskey commented Mar 31, 2024

james7132 commented Mar 31, 2024

SkiFire13 commented Mar 31, 2024

james-j-obrien commented Apr 1, 2024 •

edited

Loading

james-j-obrien commented Apr 1, 2024

urben1680 commented Apr 6, 2024 •

edited

Loading

jnhyatt commented Apr 20, 2024

ecoskey commented Aug 28, 2024

Dynamic Component Id type #12794

Dynamic Component Id type #12794

Conversation

ecoskey commented Mar 30, 2024

Objective

Solution

Changelog

jnhyatt commented Mar 30, 2024

ecoskey commented Mar 30, 2024

SkiFire13 left a comment

Choose a reason for hiding this comment

SkiFire13 Mar 30, 2024

Choose a reason for hiding this comment

ecoskey Mar 30, 2024

Choose a reason for hiding this comment

SkiFire13 Mar 30, 2024

Choose a reason for hiding this comment

SkiFire13 Mar 30, 2024

Choose a reason for hiding this comment

ecoskey Mar 30, 2024

Choose a reason for hiding this comment

SkiFire13 Mar 30, 2024

Choose a reason for hiding this comment

SkiFire13 commented Mar 30, 2024

ecoskey commented Mar 30, 2024 • edited Loading

ecoskey commented Mar 31, 2024

james7132 commented Mar 31, 2024

SkiFire13 commented Mar 31, 2024

james-j-obrien commented Apr 1, 2024 • edited Loading

james-j-obrien commented Apr 1, 2024

urben1680 commented Apr 6, 2024 • edited Loading

jnhyatt commented Apr 20, 2024

ecoskey commented Aug 28, 2024

ecoskey commented Mar 30, 2024 •

edited

Loading

james-j-obrien commented Apr 1, 2024 •

edited

Loading

urben1680 commented Apr 6, 2024 •

edited

Loading