The `dynamic` example is scary and needs a revamp
- Dominant language
- Rust
- Stars
- 48.2k
- Forks
- 4.8k
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 161
Description
The example in question:
https://github.com/bevyengine/bevy/blob/259fb6896e07bed89df2637aace547f295fc7e82/examples/ecs/dynamic.rs
Does far more than presenting how to use the dynamic query API. Notably it:
1. Creates a custom components not tied to a rust type
2. It uses the dynamic query API
3. It uses `std::alloc` and `NonNull` to copy values from a `Vec` and point a `OwningPtr` to it.
4. It parses a mini syntax for querying.
To illustrate a dynamic querying API, I think (2) and (4) are sufficient. (1) should have a separate example. (3) could illustrate separately the `bevy_ptr` crate as well.
This might seem harmless, but in reality, it causes real problems: it confuses users. Here is an example: https://github.com/bevyengine/bevy/discussions/11424
### Other problems with the example
https://github.com/bevyengine/bevy/blob/259fb6896e07bed89df2637aace547f295fc7e82/examples/ecs/dynamic.rs#L123-L128
There is direct calls to `std::alloc` functions, a lot of manual pointer fiddling (`cast`, `copy_from`, `NonNull::new`), Even a memory leak! (the `data` ptr is allocated without ever being freed) I think this kind of code has nothing to do in a bevy example, not only it isn't representative of general bevy usage, but it might scare off potential users. Especially given that supposedly people chose a **rust** game engine because they don't have to remember to free their dynamic arrays!
### Proposed changes
In actuality, the currently exposed API do still require `OwningPtr` (the `bevy_ptr` type) at least for insertion but I think the best example would show how to convert between the `Ptr` returned by dynamic query iteration into a `&dyn Reflect`. This has already been a question (@coreh) and I suspect it's going to be super useful to a lot of users.
> The way you go from the `PtrMut` to a `&mut dyn Reflect` is as follow:
> 1. Get the `ReflectFromPtr` for the component id. Here I cache it when I build the query state, so that I don't have to fetch it while iterating the query
> https://github.com/nicopap/bevy_mod_dynamic_query/blob/149b5d4a5a3611373b99b83bc9075cbd7d03b01e/src/builder/named.rs#L28-L32
>
> 2. Get the `Ptr` or `PtrMut` returned by iterating a query in the #9774 API and call `ReflectFromPtr::from_ptr` on the individual fetch items, then you have a `&dyn Reflect`:
https://github.com/nicopap/bevy_mod_dynamic_query/blob/149b5d4a5a3611373b99b83bc9075cbd7d03b01e/src/fetches.rs#L167-L169
> Here is a version of point (1) where I go from `T: Component` to `ReflectFromPtr`:
Knowing how to convert into a `&dyn Reflect` a `PtrMut` is immediately more useful than knowing how to copy from a `Vec` into a custom component. It also has more applications, especially for someone who is interested into using the dynamic query API.
Contributor guide
Assessment
This issue has not been assessed yet.