Extract recursive mouse filter into NodeExtensions; fix popup event leak - #990
Closed
Billy-Lasso wants to merge 1 commit into
Closed
Extract recursive mouse filter into NodeExtensions; fix popup event leak#990Billy-Lasso wants to merge 1 commit into
Billy-Lasso wants to merge 1 commit into
Conversation
…eak (#452) Extract PopupOverlay's recursive SetMouseFilter into a public NodeExtensions.SetMouseFilterRecursive so any subtree can have its mouse filter set generically (modal input blocking). Also fix a signal-handler leak: TileInfoPopup subscribes to the overlay's Click event to dismiss on outside click, but never unsubscribes, so a stale handler from a closed tile-info popup could close later modals (e.g. the Escape quit menu) when clicking outside. Unsubscribe in _ExitTree.
Author
|
Closing - opened from the wrong GitHub account by mistake. Will reopen from the correct account. |
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.
Closes #452.
Summary
1. Make modal mouse-filter blocking reusable (#452)
PopupOverlayhad a private staticSetMouseFilterthat recursed through a subtree setting the GodotMouseFilter. This moves it to a public extension method,NodeExtensions.SetMouseFilterRecursive(this Node, Control.MouseFilterEnum), so any code can generically block/restore input to a subtree.PopupOverlay.Isolate()/Reconnect()now call it; the private helper is removed (along with a redundantcontrol.MouseFilterassignment that the recursion already handled).2. Fix a popup signal-handler leak (found while testing #452)
TileInfoPopupsubscribes toPopupOverlay.Clickto dismiss on an outside click (overlay.Click += Close) but never unsubscribes. Because the closed popup is only removed from the tree (not freed) and the delegate keeps it alive, the stale handler persists. Concretely: after opening any tile-info popup once, clicking outside a later modal (e.g. the Escape quit menu) would invoke the stale handler, which callsHideTileInfo()→OnHidePopup(), closing the modal that should have stayed open. This unsubscribes in_ExitTree(guarded byIsInstanceValidfor app teardown), so the tile-info dismiss behavior remains but can no longer affect other popups.Testing
dotnet build C7/C7.sln: 0 errors.dotnet test C7/C7.slnwithCIV3_HOMEset: 67 passed, 0 failed, 0 skipped.