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

Fix global wireframe behavior not being applied on new meshes #11792

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

Kanabenki
Copy link
Contributor

Objective

Solution

  • Remove the run condition for apply_global_wireframe_material, since it prevent detecting when meshes are added or the NoWireframe marker component is removed from an entity. Alternatively this could be done by using a run condition like "added Handle<Mesh> or removed NoWireframe or WireframeConfig changed" but this seems less clear to me than directly letting the queries on apply_global_wireframe_material do the filtering.

@Kanabenki Kanabenki added C-Bug An unexpected or incorrect behavior A-Gizmos Visual editor and debug gizmos labels Feb 9, 2024
@Kanabenki Kanabenki marked this pull request as draft February 9, 2024 11:13
@Kanabenki
Copy link
Contributor Author

Kanabenki commented Feb 9, 2024

Putting in draft since there is some potential edge cases with NoWireframe I'd like to fix.

Edit: should now properly handle NoWireframe removal.

@Kanabenki Kanabenki marked this pull request as ready for review February 9, 2024 13:25
@Kanabenki Kanabenki changed the title Fix global wireframe behaviour not being applied on new meshes Fix global wireframe behavior not being applied on new meshes Feb 9, 2024
Copy link
Contributor

@Shatur Shatur left a comment

Choose a reason for hiding this comment

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

Minor suggestion, otherwise looks good!

crates/bevy_pbr/src/wireframe.rs Outdated Show resolved Hide resolved
@Shatur Shatur added this to the 0.13 milestone Feb 12, 2024
@Shatur
Copy link
Contributor

Shatur commented Feb 12, 2024

@alice-i-cecile may I suggest this PR for your merge train?

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 12, 2024
@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 Feb 12, 2024
Merged via the queue into bevyengine:main with commit 2d90b20 Feb 12, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Gizmos Visual editor and debug gizmos C-Bug An unexpected or incorrect behavior 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.

WireframeConfig change affects only currently spawned meshes
3 participants