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

Add required features to manifest for shader_material_glsl example #8657

Conversation

rparrett
Copy link
Contributor

Objective

Fixed #8505

Solution

Add required-features to the cargo manifest for that example.

There's precedent for this with the tonemapping example which requires non-default features ktx2 and zstd.

Now when running the example, you see a friendly error message:

 cargo run --example shader_material_glsl
error: target `shader_material_glsl` in package `bevy` requires the features: `shader_format_glsl`
Consider enabling them by passing, e.g., `--features="shader_format_glsl"`

@rparrett rparrett added C-Examples An addition or correction to our examples C-Bug An unexpected or incorrect behavior labels May 23, 2023
@alice-i-cecile alice-i-cecile added the A-Build-System Related to build systems or continuous integration label May 23, 2023
@alice-i-cecile alice-i-cecile requested a review from mockersf May 23, 2023 19:32
@mockersf
Copy link
Member

There's precedent for this with the tonemapping example which requires non-default features ktx2 and zstd.

Those two features are actually part of the default set and should be removed from the required-features of that example.

I would prefer the example to check that the feature is enabled before calling the code requiring the features than adding that

@rparrett
Copy link
Contributor Author

rparrett commented May 23, 2023

I would prefer the example to check that the feature is enabled before calling the code requiring the features than adding that

Could you explain that preference a bit more? Is required-features only meant to be used when a feature is required for compilation? Is that distinction useful in this case?

It's handy to have that metadata available for prototype_bevy_example_runner.

@mockersf
Copy link
Member

mockersf commented Jun 1, 2023

Could you explain that preference a bit more? Is required-features only meant to be used when a feature is required for compilation? Is that distinction useful in this case?

Thinking about this more, it doesn't really fly...

What do you think of either:

  • removing that example. This means we consider glsl support something that we don't want to promote that much
  • adding feature shader_format_glsl to the default features of Bevy. This was the case before make glsl and spirv support optional #8491

It's handy to have that metadata available for prototype_bevy_example_runner.

In my experience building all the examples, it's best (for total build time) to use the same set of feature for all and not change per example.

@rparrett
Copy link
Contributor Author

Closing in favor of #8969

@rparrett rparrett closed this Jun 27, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jun 27, 2023
…8970)

# Objective

These features are now included by default, so this metadata is no
longer required.

See #8657 (comment)

## Solution

Remove the metadata
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

example shader_material_glsl panics due to missing feature
3 participants