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

Add assert_is_exclusive_system function #4788

Closed
Nilirad opened this issue May 17, 2022 · 6 comments
Closed

Add assert_is_exclusive_system function #4788

Nilirad opened this issue May 17, 2022 · 6 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@Nilirad
Copy link
Contributor

Nilirad commented May 17, 2022

What problem does this solve or what need does it fill?

We already have assert_is_system, but it does not work with systems that take &mut World.

What solution would you like?

A function called assert_is_exclusive_system that does nothing if the argument implements IntoExclusiveSystem, and fails to compile otherwise.

What alternative(s) have you considered?

Alternatively, you can try to do this:

bevy_ecs::system::IntoExclusiveSystem::exclusive_system(my_exclusive_system);

but it's ugly, and it's not symmetric with assert_is_system.

Another alternative is to wait for #4166 to be merged, which would deprecate exclusive systems as a special type of system.

Additional context

  • The body of the function should be really empty, not like the if false crime that has been committed against assert_is_system.
  • The function should have an #[allow(UnusedVariables)] attribute, otherwise we would have to prefix underscore the parameter (ugly).
@Nilirad Nilirad added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels May 17, 2022
@DJMcNab
Copy link
Member

DJMcNab commented May 17, 2022

Surely we should just wait for #4166 (or well, an updated version addressing the concerns)?

@Nilirad
Copy link
Contributor Author

Nilirad commented May 17, 2022

I added the PR link to the alternatives.

That being said, I think this can stay open for newcomers that want to do their first PR. Or maybe not, for a change that will get reverted soon. I don't know, I'll leave it up to the maintainers.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels May 18, 2022
@alice-i-cecile
Copy link
Member

My feeling is that #4166 is preferred, but I would probably vote in favor of merging a PR that added this, since it should be easily caught during the refactor.

@Nilirad
Copy link
Contributor Author

Nilirad commented Jun 24, 2022

Is #4166 going to be merged before 0.8? Alternatively this would be a rather trivial change that can easily land on the next release.

@alice-i-cecile
Copy link
Member

I think that's unlikely to be a high enough priority; I would be happy to merge a tiny PR adding this.

@Nilirad
Copy link
Contributor Author

Nilirad commented Jun 24, 2022

Fine. I'm going to mark this as good first issue, since its implementation can be literally based on assert_is_system.

@Nilirad Nilirad added the D-Trivial Nice and easy! A great choice to get started with Bevy label Jun 24, 2022
bors bot pushed a commit that referenced this issue Jul 13, 2022
Add compile time check for if a system is an exclusive system. Resolves #4788 

Co-authored-by: Daniel Liu <mr.picklepinosaur@gmail.com>
Co-authored-by: Daniel Liu <danieliu3120@gmail.com>
@bors bors bot closed this as completed in fe59fe5 Jul 14, 2022
inodentry pushed a commit to IyesGames/bevy that referenced this issue Aug 8, 2022
Add compile time check for if a system is an exclusive system. Resolves bevyengine#4788 

Co-authored-by: Daniel Liu <mr.picklepinosaur@gmail.com>
Co-authored-by: Daniel Liu <danieliu3120@gmail.com>
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
Add compile time check for if a system is an exclusive system. Resolves bevyengine#4788 

Co-authored-by: Daniel Liu <mr.picklepinosaur@gmail.com>
Co-authored-by: Daniel Liu <danieliu3120@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
Add compile time check for if a system is an exclusive system. Resolves bevyengine#4788 

Co-authored-by: Daniel Liu <mr.picklepinosaur@gmail.com>
Co-authored-by: Daniel Liu <danieliu3120@gmail.com>
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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants