Allow overriding global wireframe setting.

[Wcubed]

Objective

Allow the user to choose between "Add wireframes to these specific entities" or "Add wireframes to everything except these specific entities".
Fixes #7309

Solution

Make the Wireframe component act like an override to the global configuration.
Having global set to false, and adding a Wireframe with enable: true acts exactly as before.
But now the opposite is also possible: Set global to true and add a Wireframe with enable: false will draw wireframes for everything except that entity.

Updated the example to show how overriding the global config works.

[tigregalis]

Setting wireframe.enable = true to disable displaying wireframes feels wrong to me.

I'd suggest changing the local per-entity Wireframe "policy" to be more explicit (bikeshed the names of the enum variants):

/// Local per-entity wireframe config.
// ... attributes ...
pub enum Wireframe { // or `WireframePolicy` or `LocalWireframeConfig` as a field of `struct Wireframe`
    /// Follow the global config, for example if `WireframeConfig { global: true }`, this entity will show a wireframe, but if `WireframeConfig { global: false }`, this entity won't show a wireframe
    UseGlobalConfig,
    /// Use the opposite of the global config, for example if `WireframeConfig { global: true }`, this entity won't show a wireframe, but if `WireframeConfig { global: false }`, this entity will show a wireframe
    InvertGlobalConfig,
    /// Ignore the global config, and show a wireframe
    Always,
    /// Ignore the global config, and don't show the wireframe
    Never,
}

These first two *GlobalConfig variants correspond to your Wireframe { enable: bool }.

These additional variants Always and Never means that for specific entities, you can know for certain that a specific entity shows or does not show a wireframe, without querying the WireframeConfig.

[Wcubed]

@tigregalis I think you misunderstood. It was wireframe.enable = false that would make it never show the wireframe, regardless of global config. Which did show to me that my implementation was probably not as clear as it could be 😃.

I changed the wireframe component to WireframeOverride::AlwaysRender and WireframeOverride::NeverRender, to make it clear that it will make the mesh not care about the global config.

[tigregalis]

I've had a play with this and the new API makes sense to me. I've extended the example to show what toggling the global config does for entities with different local wireframe configs (red = never, yellow = no component, green = always); is this updated example something we want?

Bevy.App.2023-01-30.18-26-35.mp4

[Wcubed]

I like it. I hadn't even thought of doing that. Makes it very clear what effects each option has.

[github-actions]

Example alien_cake_addict failed to run, please try running it locally and check the result.

[Shatur]

I'm not sure if I like the new name WireframeOverride.
Maybe keep the original Wireframe name? It shorter and still makes sense since it works regardless of global config.

[Wcubed]

I'm not sure if I like the new name WireframeOverride. Maybe keep the original Wireframe name? It shorter and still makes sense since it works regardless of global config.

Good point. The AlwaysRender and NeverRender names already indicate it is an override, so the additional Override in the name is redundant 👍

[Wcubed]

Brought up to date with 0.11, and fixed typo pointed out by shatur

[alice-i-cecile]

This is a better strategy, and the docs and code quality seem good.

[Shatur]

@Wcubed could you resolve conflicts?

[IceSentry]

So, I wasn't aware of this PR and I think the merged api should have been different. I'd like to suggest it here first to see what people think and see if there's agreement.

I believe the api should have used 2 separate components Wireframe and NoWireframe (or maybe NoGlobalWireframe, NeverRenderWireframe, naming isn't really the point)

I think this is better for a few reasons:

• Easier to migrate since it doesn't change the old behaviour. Essentially nothing to migrate. Right now this PR is a breaking change but I don't think it has to be.
• It's similar to other "per mesh" rendering features like NotShadowCaster/NotShadowReceiver
• It doesn't force new users to also think about global vs not global if all they want is to render a wireframe
• This would also let you filter at the query definition level instead of filtering when running the query

[IceSentry]

If we decide to keep this api, please consider adding a migration guide section to the PR description.

[github-actions]

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

[mockersf]

it seems this PR broke rendering wireframes on windows / DX12

[IceSentry]

I don't think this was caused by this PR. It's also broken when using DX12 on the commit before this PR. The issue seems to be be with the DX12 backend.

Here's a screenshot from before this PR

[mockersf]

@IceSentry two PRs that were merged at the same time, this one and #9895. I assumed wireframe being broken was due to the PR changing wireframes. It worked before those two, could you check the commit before (375af64)?

[IceSentry]

I went back to #9796 and it's still broken when using DX12 for me.

Back to f69e923 and it still doesn't work correctly.

[IceSentry]

I created this PR with my proposed changes to make the API non breaking while keeping the override feature #10023