bevyengine / bevyengine/bevy

The `dynamic` example is scary and needs a revamp

Open
#11,459 3 comments 1 reaction 0 assignees View on GitHub
A-ECS A-Pointers C-Docs C-Examples
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.