[UUM-141406] fixing action overlay behavior#672
Open
lopezt-unity wants to merge 5 commits into
Open
Conversation
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This PR introduces static event subscriptions to PreviewActionManager.delayedPreviewChanged across several menu action settings panels. However, because these anonymous subscribers are not unregistered when the panels are destroyed, they will cause memory leaks and multiple handler invocations.
Summary of Findings
- Static Event Memory Leaks (High Importance): Multiple menu action classes (
BevelEdges,ExtrudeEdges,ExtrudeFaces,OffsetElements,SubdivideEdges,WeldVertices, andGrowSelection) register event handlers to the static eventPreviewActionManager.delayedPreviewChangedwithout a mechanism to unsubscribe. These subscriptions should be cleaned up using theDetachFromPanelEventcallback on the root visual elements.
🤖 Helpful? 👍/👎
lopezt-unity
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
##Purpose of this PR
Jira: UUM-141406 (https://jira.unity3d.com/browse/UUM-141406)
This PR fixes two related issues with ProBuilder action overlays:
triggered, the overlay would not properly refresh or display. The fix adds a workaround that toggles displayed from
false to true to ensure the overlay is properly added/refreshed in the view.
fields (FloatField, IntegerField, Vector3Field) would not dynamically update their isDelayed property. Users had to
restart the action for the setting to take effect.
###Solution:
WeldVertices, GrowSelection) to subscribe to this event and update their field properties dynamically
##Release Notes
Fixed: UUM-141406 - Action overlays now display correctly when triggering a new action while an overlay is already shown in the SceneView.
##Functional Testing status
Manual Testing:
Edges, Weld Vertices, Grow Selection)
Existing Automation:
ProBuilder has existing editor tests for action functionality. The overlay display and UI field behavior changes are
UI-specific and covered by manual testing.
##Performance Testing Status
Performance Impact: Minimal positive impact
The change replaces an inefficient approach (destroying and recreating the entire overlay UI hierarchy) with an
event-based approach that only updates the isDelayed property on existing fields. This reduces allocations and UI
rebuilds when toggling the live preview setting.
No performance testing required - this is a UI behavior fix with a minor efficiency improvement.
##Overall Product Risks
Technical Risk: 1
Isolated UI behavior changes in ProBuilder action overlays. The event-based approach is straightforward, and the
overlay display workaround is localized. Low risk of regression.
Halo Effect: 1
Changes are isolated to ProBuilder's action overlay system and the seven modified geometry actions. Does not affect
other ProBuilder functionality, other packages, or Unity Editor core systems.