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

Provide more information when a filewatcher failure is hit. #13715

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Provide more information when a filewatcher failure is hit. #13715

merged 4 commits into from
Jun 6, 2024

Conversation

brandon-reinhart
Copy link
Contributor

@brandon-reinhart brandon-reinhart commented Jun 6, 2024

A naked unwrap led to an opaque error that can be hit when using the embedded filewatcher.

I've changed this an unwrap_or_else panic! with the error message providing more details about the failed operation.

A better solution would be to print an error! and not panic...

This was tested with the asset_processing example.

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.

This is an improvement at least. Definitely agree that it's weird that this panics.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-Assets Load files from disk to use for things like images, models, and sounds C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Uncontroversial This work is generally agreed upon S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 6, 2024
@brandon-reinhart
Copy link
Contributor Author

The panic may be related to what people are running into here: #10575

@brandon-reinhart
Copy link
Contributor Author

Also #7008

@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 and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 6, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 6, 2024
Merged via the queue into bevyengine:main with commit 3122c87 Jun 6, 2024
28 checks passed
@brandon-reinhart brandon-reinhart deleted the improve-asset-path-error branch June 6, 2024 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds 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 S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants