Skip to content
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 by Bors] - Fix label macro for types with generics #1498

Closed

Conversation

TheRawMeatball
Copy link
Member

Fixes #1497

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Feb 22, 2021
@aevyrie
Copy link
Member

aevyrie commented Feb 22, 2021

I switched to this PR branch, but am still seeing a type error with the following code:

#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemLabel)]
pub enum BoundingSystem<T> {
    UpdateBoundVols(PhantomData<T>),
    UpdateDebugMeshes(PhantomData<T>),
}

errors:

error[E0107]: wrong number of type arguments: expected 1, found 0
  --> src\lib.rs:17:10
   |
17 | pub enum BoundingSystem<T> {
   |          ^^^^^^^^^^^^^^ expected 1 type argument

error[E0107]: wrong number of type arguments: expected 0, found 1
  --> src\lib.rs:17:25
   |
17 | pub enum BoundingSystem<T> {
   |                         ^ unexpected type argument

error[E0277]: the trait bound `BoundingSystem<T>: bevy::prelude::SystemLabel` is not satisfied
  --> src\lib.rs:36:28
   |
36 |                     .label(BoundingSystem::UpdateBoundVols(PhantomData::<T>::default())),
   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `bevy::prelude::SystemLabel` is 
not implemented for `BoundingSystem<T>`

@aevyrie
Copy link
Member

aevyrie commented Feb 22, 2021

With the most recent change (and a lot of derives 🙂) this seems to fix the type errors.

However, from a useability standpoint, I'm finding it impossible to get this working due to all the trait bounds. My goal is to use my components as types in generic systems. However, when multiple generic systems are run that need labels, my system dependencies are between systems of the same type - my components being the type - but this adds a ton of trait bounds on my component types. Is there some way I could wrap my components in something like a Box<> that implements all the SystemLabel functionality, without my components exploding with trait bound complexity?

I think what I'm doing here is a natural extension of generic systems, and the issues I'm running into are a repercussion of using strongly typed labels.

@TheRawMeatball
Copy link
Member Author

Hmm, can you post a snippet?

@aevyrie
Copy link
Member

aevyrie commented Feb 23, 2021

Hmm, can you post a snippet?

Here's an example: https://github.com/aevyrie/bevy_mod_bounding/blob/9f5a1169fd742f58bac342c883df2001881594a0/src/lib.rs#L17-L25

I pushed the broken code to master.

@aevyrie
Copy link
Member

aevyrie commented Feb 23, 2021

My issue has been resolved here: #1497

@Ratysz
Copy link
Contributor

Ratysz commented Feb 23, 2021

Hmm, can you post a snippet?

Here's an example: https://github.com/aevyrie/bevy_mod_bounding/blob/9f5a1169fd742f58bac342c883df2001881594a0/src/lib.rs#L17-L25

I pushed the broken code to master.

Changing

PhantomData<T>: 'static + Clone + Hash + Debug + Eq + Send + Sync + BoundingVolume,

to

T: 'static + Clone + Hash + Debug + Eq + Send + Sync + BoundingVolume,

allows this to compile.

EDIT: Oh, I see the problem. If you can't make the relevant BoundingVolume implementors satisfy all those requirements, you'll have to manually implement those traits on BoundingSystem - deriving them directly won't work.

@DJMcNab DJMcNab removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 23, 2021
@cart
Copy link
Member

cart commented Mar 9, 2021

bors r+

bors bot pushed a commit that referenced this pull request Mar 9, 2021
@bors
Copy link
Contributor

bors bot commented Mar 9, 2021

@bors bors bot changed the title Fix label macro for types with generics [Merged by Bors] - Fix label macro for types with generics Mar 9, 2021
@bors bors bot closed this Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typed SystemLabels regress generic labeling
6 participants