komadori / bevy_mod_outline

Apache License 2.0
123 stars 10 forks source link

Always compute outline depth when SetOutlineDepth is changed #10

Closed mxgrey closed 1 year ago

mxgrey commented 1 year ago

I came across an edge case with computing the outline depth settings. If the SetOutlineDepth component is changed but GlobalTransform is not, then the outline depth settings will not get updated.

This PR just fixes the logic for detecting the changes.

komadori commented 1 year ago

Hi @mxgrey, I've just got back from holiday. I've merged this PR and can release 0.3.3. However, can you explain to me why you needed to add SetOutlineDepth in your project's commit here: https://github.com/open-rmf/rmf_site/pull/111/commits/672868b786f2bde548a903af71a158c3ee5d6611 ? SetOutlineDepth::Flat with an origin of (0,0,0) should already be the default behaviour, so while you've clearly found a legitimate bug in bevy_mod_outline I don't understand why it is affecting the behaviour of the outliner for you.

mxgrey commented 1 year ago

Since ComputedOutlineDepth derives Default and the default value of bool is false, the default behavior will have flat: false and therefore it will be as if SetOutlineDepth is set to Real.

If the default behavior was meant to stay the same as 0.2 then there would need to be a custom implementation of Default for ComputedOutlineDepth that sets flat to true.

komadori commented 1 year ago

@mxgrey Ah, I see, thanks! This bug was introduced in 0.3.1 along with SetOutlineDepth::Real and is not intentional behaviour. I was relying on compute_outline_depth() to initialise this flag correctly, which it does only if your entity is moving or if the OutlineBundle is added at the same time as the TransformBundle. I will fix this too.

komadori commented 1 year ago

@mxgrey FYI, 0.3.3 is released.