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

Handle wgsl errors in the game of life example #13624

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

kornelski
Copy link
Contributor

Currently copypasting the example into a new project without also copying "shaders/game_of_life.wgsl" gives an unhelpful blank screen.
This change makes it panic instead. I think nicer error handling is outside scope of the example, and this is good enough to point out that the shader code is missing.

@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples D-Shaders This code uses GPU shader languages labels Jun 1, 2024
@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged D-Trivial Nice and easy! A great choice to get started with Bevy X-Uncontroversial This work is generally agreed upon labels Jun 1, 2024
@kornelski
Copy link
Contributor Author

This affects other examples, so if this approach is okay, I can repeat it for other examples too.

@IceSentry
Copy link
Contributor

This affects other examples

IIRC game_of_life is the only example that does that kind of thing, I'm not sure which other example you are referring to.

@kornelski
Copy link
Contributor Author

I mean there's a pattern of examples/foo.rs using assets/shaders/foo.rs

https://github.com/bevyengine/bevy/blob/main/examples/shader/array_texture.rs

"shaders/array_texture.wgsl".into()

https://github.com/bevyengine/bevy/blob/main/assets/shaders/array_texture.wgsl

and the example code run outside of bevy's directory structure fails to load the files it depends on, and displays a blank screen:

ERROR bevy_asset::server: Path not found: /tmp/assets/shaders/array_texture.wgsl

I'm proposing to move these paths to a const near the top of the file, so that users will more likely notice the dependency.

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 2, 2024
@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 2, 2024
@alice-i-cecile
Copy link
Member

I'd be happy to see this done more broadly in some form. The current PR is an improvement though so I'll merge it on Monday unless you tell me not to.

@rparrett
Copy link
Contributor

rparrett commented Jun 2, 2024

I think moving the shader path into a constant is a great idea.

The actual error produced here feels confusing to me, which is unfortunate.

thread '<unnamed>' panicked at examples/shader/compute_shader_game_of_life.rs:233:25:
Initializing assets/shaders/game_of_life.wgsl:
Pipeline could not be compiled because the following shader is not loaded yet: AssetId<bevy_render::render_resource::shader::Shader>{ index: 0, generation: 0}
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_render::renderer::render_system`!

Specifically, the not loaded *yet* part. I wonder if that could be improved (not in this PR), or if there are scenarios where that language actually makes sense?

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 3, 2024
Merged via the queue into bevyengine:main with commit 8d666c8 Jun 3, 2024
29 checks passed
kornelski added a commit to kornelski/bevy-temp that referenced this pull request Jun 12, 2024
@kornelski kornelski deleted the example-err branch June 12, 2024 11:12
github-merge-queue bot pushed a commit that referenced this pull request Jun 12, 2024
The examples won't work when copy-pasted to another project, without
also copying their shader files. This change adds constants at the top
of the files to bring attention to the dependencies.

Follow up to
[#13624](#13624 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Examples An addition or correction to our examples D-Shaders This code uses GPU shader languages 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.

5 participants