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 bad bounds for NonSend SystemParams #2325

Closed
wants to merge 2 commits into from

Conversation

MinerSebas
Copy link
Contributor

@MinerSebas MinerSebas commented Jun 9, 2021

Objective

Currently, you can't call is_added or is_changed on a NonSend SystemParam, unless the Resource is a Component (implements Send and Sync).
This defeats the purpose of providing change detection for NonSend Resources.
While fixing this, I also noticed that NonSend does not have a bound at all on its struct.

Solution

Change the bounds of T to always be 'static.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jun 9, 2021
@TheRawMeatball TheRawMeatball 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 and removed S-Needs-Triage This issue needs to be labelled labels Jun 9, 2021
@MinerSebas
Copy link
Contributor Author

As #2326 also fixes the wrong bound of NonSendMut, I reverted the fix here to prevent conflicts.
The fixes for NonSend are still valid.

@MinerSebas MinerSebas changed the title Fix bad bounds for NonSend and NonSendMut SystemParams Fix bad bounds for NonSend SystemParams Jun 9, 2021
@cart
Copy link
Member

cart commented Jun 9, 2021

Nice catch!

@cart
Copy link
Member

cart commented Jun 9, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 9, 2021
# Objective

Currently, you can't call `is_added` or `is_changed` on a `NonSend` SystemParam, unless the Resource is a Component (implements `Send` and `Sync`). 
This defeats the purpose of providing change detection for NonSend Resources.
While fixing this, I also noticed that `NonSend` does not have a bound at all on its struct.

## Solution

Change the bounds of `T` to always be `'static`.
@bors bors bot changed the title Fix bad bounds for NonSend SystemParams [Merged by Bors] - Fix bad bounds for NonSend SystemParams Jun 9, 2021
@bors bors bot closed this Jun 9, 2021
@MinerSebas MinerSebas deleted the NonSend_bound branch June 10, 2021 14:52
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
# Objective

Currently, you can't call `is_added` or `is_changed` on a `NonSend` SystemParam, unless the Resource is a Component (implements `Send` and `Sync`). 
This defeats the purpose of providing change detection for NonSend Resources.
While fixing this, I also noticed that `NonSend` does not have a bound at all on its struct.

## Solution

Change the bounds of `T` to always be `'static`.
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 S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants