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] - Add FromWorld bound to T in Local<T> #5481

Closed
wants to merge 1 commit into from

Conversation

PROMETHIA-27
Copy link
Contributor

Objective

Currently, actually using a Local on a system requires that it be T: FromWorld, but that requirement is only expressed on the SystemParam machinery, which leads to the confusing error message for when the user attempts to add an invalid system. By adding these bounds to Local directly, it improves clarity on usage and semantics.

Solution

  • Add T: FromWorld bound to Local's definition

Migration Guide

  • It might be possible for references to Locals without T: FromWorld to exist, but these should be exceedingly rare and probably dead code. In the event that one of these is encountered, the easiest solutions are to delete the code or wrap the inner T in an Option to allow it to be default constructed to None.

@mockersf mockersf added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jul 29, 2022
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jul 29, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 1, 2022
# Objective

Currently, actually using a `Local` on a system requires that it be `T: FromWorld`, but that requirement is only expressed on the `SystemParam` machinery, which leads to the confusing error message for when the user attempts to add an invalid system. By adding these bounds to `Local` directly, it improves clarity on usage and semantics. 

## Solution

- Add `T: FromWorld` bound to `Local`'s definition

## Migration Guide

- It might be possible for references to `Local`s without `T: FromWorld` to exist, but these should be exceedingly rare and probably dead code. In the event that one of these is encountered, the easiest solutions are to delete the code or wrap the inner `T` in an `Option` to allow it to be default constructed to `None`.
@bors bors bot changed the title Add FromWorld bound to T in Local<T> [Merged by Bors] - Add FromWorld bound to T in Local<T> Aug 1, 2022
@bors bors bot closed this Aug 1, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

Currently, actually using a `Local` on a system requires that it be `T: FromWorld`, but that requirement is only expressed on the `SystemParam` machinery, which leads to the confusing error message for when the user attempts to add an invalid system. By adding these bounds to `Local` directly, it improves clarity on usage and semantics. 

## Solution

- Add `T: FromWorld` bound to `Local`'s definition

## Migration Guide

- It might be possible for references to `Local`s without `T: FromWorld` to exist, but these should be exceedingly rare and probably dead code. In the event that one of these is encountered, the easiest solutions are to delete the code or wrap the inner `T` in an `Option` to allow it to be default constructed to `None`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Currently, actually using a `Local` on a system requires that it be `T: FromWorld`, but that requirement is only expressed on the `SystemParam` machinery, which leads to the confusing error message for when the user attempts to add an invalid system. By adding these bounds to `Local` directly, it improves clarity on usage and semantics. 

## Solution

- Add `T: FromWorld` bound to `Local`'s definition

## Migration Guide

- It might be possible for references to `Local`s without `T: FromWorld` to exist, but these should be exceedingly rare and probably dead code. In the event that one of these is encountered, the easiest solutions are to delete the code or wrap the inner `T` in an `Option` to allow it to be default constructed to `None`.
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 M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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.

4 participants