-
Notifications
You must be signed in to change notification settings - Fork 67
Require #[derive(Component)]
for component types
#27
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
# Feature Name: `derive_component` | ||
|
||
## Summary | ||
|
||
Remove blanket impl from `Component` trait, and require it to be manually implemented or derived using `#[derive(Component)]`. | ||
Extend the `Component` trait to declare more information statically, instead of requiring runtime registration. | ||
|
||
## Motivation | ||
|
||
`Component` trait is being used inside Bundles and Queries. It's usage in Queries is enforced by the type system. | ||
Unfortunately, because it's automatically derived, almost every type can be treated as a component, including Bundles. | ||
That means `Component` APIs don't really have any way to prevent misuse, like `commands.insert(bundle)`. Right now | ||
it's very easy to end up with that code because of a typo, as the `Bundle` api methods are very similarly named. | ||
|
||
There are also other pitfalls, like primitive types being automatically `Component`s, but without clear meaning what they represent. | ||
This is an especially sever issue for function pointer types. It's easy to use an enum variant or newtype constructor as | ||
an component, like `.insert(MyEnum::Variant)`, without realising it. This leads to to very hard to spot bugs without any hint | ||
of either a compiler or runtime that something went wrong. This is easily preventable by not implementing `Component` trait for those types. | ||
Frizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
We also already have `#[derive(Bundle)]` and others. Adding that to `Component` keeps the syntax similar across the board. | ||
|
||
Apart from requiring explicit opt-in, `derive` gives us a natural place to define more metadata about specific component type. | ||
One way to use that is to declare component's Storage type statically, instead of requiring manual registration of that metadata | ||
into the world. | ||
|
||
```rust | ||
#[derive(Component)] | ||
#[component(storage = "SparseSet")] | ||
struct MyComponent(..); | ||
``` | ||
|
||
This enables compiler to perform much more optimizations in the query iteration code, as a lot of the code paths can be statically eliminated. | ||
Frizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## User-facing explanation | ||
|
||
In order to define your own component type, you have to implement a `Component` trait. The easiest way to do that is using a `derive` macro. | ||
Frizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```rust | ||
#[derive(Component)] | ||
struct HitPoints(u32); | ||
``` | ||
|
||
If you forget to properly annotate your component type, the compiler will remind you | ||
of that as soon as you try to insert that component into the world or query for it. Apart from type safety, this also provides a visual indication that given type | ||
Frizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
is intended to be directly associated with entities in the world. | ||
|
||
By default, your component will be stored using Dense storage strategy. | ||
In order to declare your component's storage strategy manually, you have to provide an additional attribute | ||
to specify the component settings. | ||
|
||
```rust | ||
#[derive(Component)] | ||
#[component(storage = "SparseSet")] | ||
struct Selected; | ||
``` | ||
|
||
### Migration strategy | ||
|
||
All your component types must be annotated with a derive. | ||
|
||
```diff | ||
+ #[derive(Component)] | ||
struct MyComponent { .. } | ||
``` | ||
|
||
You can no longer use primitive types (e.g. `u32`, `bool`) as components. | ||
Wrap them in a custom struct, and mark that as a component. | ||
|
||
```diff | ||
+ #[derive(Component)] | ||
+ struct HitPoints(u32); | ||
|
||
- commands.entity(id).insert(100); | ||
+ commands.entity(id).insert(HitPoints(100)); | ||
``` | ||
|
||
Remove all registration of components. Make sure to correctly annotate components that were previously registered with `SparseSet` storage type. | ||
|
||
```diff | ||
- world.register_component( | ||
- ComponentDescriptor::new::<MyComponent>(StorageType::SparseSet) | ||
- ).unwrap(); | ||
|
||
+ #[derive(Component)] | ||
+ #[component(storage = "SparseSet")] | ||
struct MyComponent { .. } | ||
``` | ||
|
||
Explain the proposal as if it was already included in the engine and you were teaching it to another Bevy user. That generally means: | ||
Frizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- Introducing new named concepts. | ||
- Explaining the feature, ideally through simple examples of solutions to concrete problems. | ||
- Explaining how Bevy users should *think* about the feature, and how it should impact the way they use Bevy. It should explain the impact as concretely as possible. | ||
- If applicable, provide sample error messages, deprecation warnings, or migration guidance. | ||
- If applicable, explain how this feature compares to similar existing features, and in what situations the user would use each one. | ||
|
||
## Implementation strategy | ||
|
||
- remove the existing blanket impl | ||
- implement basic derive macro that just implements a marker trait | ||
- add associated `Storage` type to the `Component` trait | ||
- provide a way to specify the `Storage` type using an macro attribute. | ||
- use the associated `Storage` type inside query iterators to statically determine | ||
if the query is sparse or dense. | ||
|
||
## Drawbacks | ||
|
||
Requiring extra annotation on every component type adds some boilerplate. | ||
|
||
## Rationale and alternatives | ||
|
||
- instead of `derive`, use an attribute macro. | ||
- Do nothing and keep registering metadata at runtime. Live with the branching inside query iterators. | ||
- Require manual implementation of `Component` trait without providing derive macro. | ||
|
||
Frizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Unresolved questions | ||
|
||
- How to support dynamic components for future scripting layer? | ||
|
||
## Future possibilities | ||
|
||
The Component derive could later lead to automatic derives for reflection, or in general serve | ||
as a way to reduce boilerplate from new features of the engine added in the future. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.