Skip to content

Conversation

@aevyrie
Copy link
Member

@aevyrie aevyrie commented Mar 4, 2022

Objective

  • Eliminate all worldcell.get_resource().unwrap() cases.
  • Provide helpful messages on panic.

Solution

  • Adds infallible resource getters to WorldCell, mirroring World.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 4, 2022
@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 S-Needs-Triage This issue needs to be labelled labels Mar 4, 2022
@alice-i-cecile
Copy link
Member

😆 I intentionally didn't add these, because I think we should pursue #3939 instead. However, this is a perfectly good change in the meantime.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks perfect; I have no complaints.

@alice-i-cecile alice-i-cecile added 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 Mar 4, 2022
@aevyrie
Copy link
Member Author

aevyrie commented Mar 4, 2022

😆 I intentionally didn't add these, because I think we should pursue #3939 instead. However, this is a perfectly good change in the meantime.

Ah, that's helpful context. Yeah, in the meantime this reduces noise and more importantly provides useful panic messages. The bevy_winit event loop is the biggest offender and could definitely use another pass to see if the WorldCells are really needed. I kept the changes in #3974 minimal in this respect.

@alice-i-cecile
Copy link
Member

The bevy_winit event loop is the biggest offender and could definitely use another pass to see if the WorldCells are really needed

Yeah, they're not :) I successfully replaced it with a SystemState in #3939. Ultimately, I think I would prefer to see #4090 used instead for it (for more legibility and easy caching), but that can come later.

@alice-i-cecile
Copy link
Member

@aevyrie I'm able to merge this now, and this is certainly trivial. Can you rebase?

@aevyrie aevyrie force-pushed the worldcell-resources branch from 3e3eb79 to 32ababd Compare April 25, 2022 20:55
@aevyrie
Copy link
Member Author

aevyrie commented Apr 25, 2022

@alice-i-cecile done, pending CI.

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Apr 25, 2022
# Objective

- Eliminate all `worldcell.get_resource().unwrap()` cases.
- Provide helpful messages on panic.

## Solution

- Adds infallible resource getters to `WorldCell`, mirroring `World`.
@bors bors bot changed the title Add infallible resource getters for WorldCell [Merged by Bors] - Add infallible resource getters for WorldCell Apr 25, 2022
@bors bors bot closed this Apr 25, 2022
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

- Eliminate all `worldcell.get_resource().unwrap()` cases.
- Provide helpful messages on panic.

## Solution

- Adds infallible resource getters to `WorldCell`, mirroring `World`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Eliminate all `worldcell.get_resource().unwrap()` cases.
- Provide helpful messages on panic.

## Solution

- Adds infallible resource getters to `WorldCell`, mirroring `World`.
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 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.

2 participants