Hidden menu panels stop catching pointers and rendering #19

Open
hiccup444 wants to merge 1 commit from hiccup444/da-game-hiccup:menu-panels-inactive-when-hidden into main
First-time contributor

Summary

A MenuPanel is hidden with visible = false, which neither takes its collider out of physics nor stops its viewport. Both panels keep a StaticBody3D on layer 21 (the layer the XR Tools pointers raycast against) and a 1024x768 SubViewport set to UPDATE_ALWAYS. So two viewports render every frame for the whole session, including gameplay, where neither is visible. handle_pointer_event() doesn't check visibility either.

Where it bites: the pause panel lives under Game, so pausing never disables it. After closing pause, its invisible collider stays 1.3 m in front of where you paused. Back in the main menu, if you paused between the menu anchor and the menu panel, it sits in the laser's path:

  • the laser stops on nothing;
  • trigger presses go to the hidden pause menu;
  • hidden Options controls can change settings, which are saved to user://settings.cfg.

Level.apply_mode() has the same pattern: hidden content groups keep processing and colliding. That is harmless today (they hold only meshes and labels). Once sandbox items or story props get physics, they would be invisible obstacles in the wrong mode.

Changes

  • src/ui/menu_panel.gd: the panel follows its visibility in the tree.
    • On NOTIFICATION_VISIBILITY_CHANGED, it switches its Body/CollisionShape3D off or on (deferred, see below) and its SubViewport between UPDATE_ALWAYS and UPDATE_DISABLED. It also does this once in _ready, because entering the tree sends no visibility change, so a panel created under a hidden parent has to check for itself.
    • The notification also arrives when an ancestor is hidden, so the main menu panel goes inactive along with MenuContent.
    • While hidden, handle_pointer_event() ignores events.
    • A press the panel was holding when it hid is let go without clicking a button or moving a slider. The panel hides and reshows its menu control, which makes the viewport drop the press, and forgets its own pressed state.
  • src/core/game.gd: _on_menu_action() acts only in MENU and not paused; _on_pause_action() acts only while paused.
  • src/levels/level.gd: apply_mode() goes through a small _set_group() that sets each group's process_mode along with its visibility (hidden PROCESS_MODE_DISABLED, shown PROCESS_MODE_INHERIT).
    • Under the default disable_mode, this also takes a hidden group's collision objects out of the physics space.
    • A shown group inherits the level's process mode, so open_pause() and _unpause_world() still freeze and unfreeze it.
    • A hidden group stays disabled through a pause and resume.

Verified by experiment on 4.6.3:

  • NOTIFICATION_VISIBILITY_CHANGED reaches a child Node3D when its parent or grandparent Node3D is hidden or shown. It is not sent on entering the tree, hence the call in _ready.
  • The collider switch is deferred because setting CollisionShape3D.disabled inside a physics callback fails with "Can't change this state while flushing queries" (Godot Physics, the project's default engine). Changing process_mode there works, so _set_group() stays immediate.
  • Across all four combinations of level and group process modes, a group's bodies are in the space only when both are enabled.
  • No made-up mouse event ends every held press safely. A release clicks a held button even when canceled or placed off the viewport, because the button keeps "pressing inside" from the last motion. Moving off the viewport first stops the click, but drags a held slider to its minimum. Hiding the menu control ends the press with neither: the button gets button_up but no pressed, the slider grab ends at its current value, and the next click lands. An OptionButton popup opened by the held press closes too.

Tests

tests/pause_check.gd gains a staged run, one stage per frame because the collider switch is deferred. Each stage clears _started when it finishes, so an aborted stage is still reported.

  • The hidden pause panel is inactive from boot (shape disabled, viewport UPDATE_DISABLED).
  • While paused it is live (shape enabled, UPDATE_ALWAYS). After close_pause() it is inactive again.
  • Pause Save and Return to Menu do nothing while not paused.
  • A press held on Return to Menu when the panel hides is let go: the button no longer reports it held.
  • Pointer events on the hidden panel emit no action, including the release of that held press. The test sends real XRToolsPointerEvents through handle_pointer_event().
  • The first pointer click on Resume after reopening lands.
  • A slider drag on the Options page while paused saves the setting. The test then unpauses with the pause input while the trigger is still held. On the next frame, the slider, the setting and the saved file all still hold the dragged value.

tests/smoke_menu.gd:

  • MENU: the menu panel is live and only MenuContent processes. A panel created under an already hidden parent starts inactive.
  • GAME: MenuContent is PROCESS_MODE_DISABLED, StoryContent processes, the menu panel is inactive, and a menu action changes nothing.
  • Back in MENU: MenuContent processes and the panel is live again. Hiding only MenuContent (the panel's own visible stays true) makes the panel inactive, and showing it again makes it live.
  • SANDBOX: only SandboxContent processes and the menu panel is inactive.

The full suite list from the CI workflow passes: smoke_menu, arms_check, addon_check, save_check, rig_motion_check, pause_check, input_check. The local run was on Windows with --xr-mode off (no headset attached); CI covers Linux.

pause_check prints a few more of the known first-pass harness lines:

  • menu_panel.gd now shows up in the "Failed to compile depended scripts" chain, because the test calls MenuPanel methods;
  • each extra set_anchored() call in the new stages prints the known Nil enabled line.

Corrections to the review

  • The proposed handle_pointer_event() guard on its own leaves a press stuck. XR Tools reports the release to the target it locked on press, even after its ray stops hitting it. If the panel hides while the trigger is held (pause toggled off by the pause input), that release is now dropped. The panel still counts the press as held, and the first click after reopening is swallowed. Reproduced headless. Letting go of the press on hide, as described above, fixes it.
  • The suggested check, emitting a pause action_selected(&"options") while not paused, can't fail. PauseMenu handles Options itself and never emits options, and Game has no branch for it. The tests use Save and Return to Menu instead.
  • The game.gd guards don't cover the Options controls, since settings_changed is a separate signal. The hidden Options page is protected by the panel-side changes: the collider is off, input is ignored, and a held press is dropped without moving a slider.

Left for in-headset checking

  • Pause just in front of the menu area, Resume, then Return to Menu. The laser should reach the main menu panel and clicks should land there.
  • Open pause, hold the trigger on a pause button, unpause with the pause input, then let go. Reopen pause: Resume should work on the first click.
  • In Options, drag the Snap angle slider and unpause with the pause input while still holding the trigger. Reopen: the slider and the turning should keep the dragged value.
  • Reopen pause after visiting Options. It should show the main page right away, with no frame of the old page (the viewport resumes rendering when shown).

Overlap with open PRs

Trial merges with #7, #11 and #14 have no conflicts. #11 edits open_pause(), _unpause_world() and the Load branch of _on_pause_action(), and appends to level.gd. This PR's hunks sit next to those but don't overlap.

Two interactions with #11:

  • Its traversal_check calls _on_pause_action(&"save") without pausing, to show that the region blocks saving. With this PR that check still passes, but because the game isn't paused rather than because of save_blocked. It should call open_pause() first to keep testing the gate.
  • It calls apply_mode() from the traversal exit. The deferred collider switch keeps that safe if it runs inside a physics callback.

Sibling PRs from the same review: #15, #16, #17 and #18. Their source changes merge cleanly with each other. Several of them add checks at the same spot in tests/pause_check.gd (and two in tests/save_check.gd), so whichever merges second may need a small rebase that keeps both blocks.

## Summary A `MenuPanel` is hidden with `visible = false`, which neither takes its collider out of physics nor stops its viewport. Both panels keep a `StaticBody3D` on layer 21 (the layer the XR Tools pointers raycast against) and a 1024x768 SubViewport set to `UPDATE_ALWAYS`. So two viewports render every frame for the whole session, including gameplay, where neither is visible. `handle_pointer_event()` doesn't check visibility either. Where it bites: the pause panel lives under `Game`, so pausing never disables it. After closing pause, its invisible collider stays 1.3 m in front of where you paused. Back in the main menu, if you paused between the menu anchor and the menu panel, it sits in the laser's path: - the laser stops on nothing; - trigger presses go to the hidden pause menu; - hidden Options controls can change settings, which are saved to `user://settings.cfg`. `Level.apply_mode()` has the same pattern: hidden content groups keep processing and colliding. That is harmless today (they hold only meshes and labels). Once sandbox items or story props get physics, they would be invisible obstacles in the wrong mode. ## Changes - `src/ui/menu_panel.gd`: the panel follows its visibility in the tree. - On `NOTIFICATION_VISIBILITY_CHANGED`, it switches its `Body/CollisionShape3D` off or on (deferred, see below) and its SubViewport between `UPDATE_ALWAYS` and `UPDATE_DISABLED`. It also does this once in `_ready`, because entering the tree sends no visibility change, so a panel created under a hidden parent has to check for itself. - The notification also arrives when an ancestor is hidden, so the main menu panel goes inactive along with `MenuContent`. - While hidden, `handle_pointer_event()` ignores events. - A press the panel was holding when it hid is let go without clicking a button or moving a slider. The panel hides and reshows its menu control, which makes the viewport drop the press, and forgets its own pressed state. - `src/core/game.gd`: `_on_menu_action()` acts only in MENU and not paused; `_on_pause_action()` acts only while paused. - `src/levels/level.gd`: `apply_mode()` goes through a small `_set_group()` that sets each group's `process_mode` along with its visibility (hidden `PROCESS_MODE_DISABLED`, shown `PROCESS_MODE_INHERIT`). - Under the default `disable_mode`, this also takes a hidden group's collision objects out of the physics space. - A shown group inherits the level's process mode, so `open_pause()` and `_unpause_world()` still freeze and unfreeze it. - A hidden group stays disabled through a pause and resume. Verified by experiment on 4.6.3: - `NOTIFICATION_VISIBILITY_CHANGED` reaches a child Node3D when its parent or grandparent Node3D is hidden or shown. It is not sent on entering the tree, hence the call in `_ready`. - The collider switch is deferred because setting `CollisionShape3D.disabled` inside a physics callback fails with "Can't change this state while flushing queries" (Godot Physics, the project's default engine). Changing `process_mode` there works, so `_set_group()` stays immediate. - Across all four combinations of level and group process modes, a group's bodies are in the space only when both are enabled. - No made-up mouse event ends every held press safely. A release clicks a held button even when canceled or placed off the viewport, because the button keeps "pressing inside" from the last motion. Moving off the viewport first stops the click, but drags a held slider to its minimum. Hiding the menu control ends the press with neither: the button gets `button_up` but no `pressed`, the slider grab ends at its current value, and the next click lands. An OptionButton popup opened by the held press closes too. ## Tests `tests/pause_check.gd` gains a staged run, one stage per frame because the collider switch is deferred. Each stage clears `_started` when it finishes, so an aborted stage is still reported. - The hidden pause panel is inactive from boot (shape disabled, viewport `UPDATE_DISABLED`). - While paused it is live (shape enabled, `UPDATE_ALWAYS`). After `close_pause()` it is inactive again. - Pause Save and Return to Menu do nothing while not paused. - A press held on Return to Menu when the panel hides is let go: the button no longer reports it held. - Pointer events on the hidden panel emit no action, including the release of that held press. The test sends real `XRToolsPointerEvent`s through `handle_pointer_event()`. - The first pointer click on Resume after reopening lands. - A slider drag on the Options page while paused saves the setting. The test then unpauses with the pause input while the trigger is still held. On the next frame, the slider, the setting and the saved file all still hold the dragged value. `tests/smoke_menu.gd`: - MENU: the menu panel is live and only `MenuContent` processes. A panel created under an already hidden parent starts inactive. - GAME: `MenuContent` is `PROCESS_MODE_DISABLED`, `StoryContent` processes, the menu panel is inactive, and a menu action changes nothing. - Back in MENU: `MenuContent` processes and the panel is live again. Hiding only `MenuContent` (the panel's own `visible` stays true) makes the panel inactive, and showing it again makes it live. - SANDBOX: only `SandboxContent` processes and the menu panel is inactive. The full suite list from the CI workflow passes: smoke_menu, arms_check, addon_check, save_check, rig_motion_check, pause_check, input_check. The local run was on Windows with `--xr-mode off` (no headset attached); CI covers Linux. pause_check prints a few more of the known first-pass harness lines: - `menu_panel.gd` now shows up in the "Failed to compile depended scripts" chain, because the test calls MenuPanel methods; - each extra `set_anchored()` call in the new stages prints the known Nil `enabled` line. ## Corrections to the review - The proposed `handle_pointer_event()` guard on its own leaves a press stuck. XR Tools reports the release to the target it locked on press, even after its ray stops hitting it. If the panel hides while the trigger is held (pause toggled off by the pause input), that release is now dropped. The panel still counts the press as held, and the first click after reopening is swallowed. Reproduced headless. Letting go of the press on hide, as described above, fixes it. - The suggested check, emitting a pause `action_selected(&"options")` while not paused, can't fail. PauseMenu handles Options itself and never emits `options`, and Game has no branch for it. The tests use Save and Return to Menu instead. - The `game.gd` guards don't cover the Options controls, since `settings_changed` is a separate signal. The hidden Options page is protected by the panel-side changes: the collider is off, input is ignored, and a held press is dropped without moving a slider. ## Left for in-headset checking - Pause just in front of the menu area, Resume, then Return to Menu. The laser should reach the main menu panel and clicks should land there. - Open pause, hold the trigger on a pause button, unpause with the pause input, then let go. Reopen pause: Resume should work on the first click. - In Options, drag the Snap angle slider and unpause with the pause input while still holding the trigger. Reopen: the slider and the turning should keep the dragged value. - Reopen pause after visiting Options. It should show the main page right away, with no frame of the old page (the viewport resumes rendering when shown). ## Overlap with open PRs Trial merges with #7, #11 and #14 have no conflicts. #11 edits `open_pause()`, `_unpause_world()` and the Load branch of `_on_pause_action()`, and appends to `level.gd`. This PR's hunks sit next to those but don't overlap. Two interactions with #11: - Its `traversal_check` calls `_on_pause_action(&"save")` without pausing, to show that the region blocks saving. With this PR that check still passes, but because the game isn't paused rather than because of `save_blocked`. It should call `open_pause()` first to keep testing the gate. - It calls `apply_mode()` from the traversal exit. The deferred collider switch keeps that safe if it runs inside a physics callback. Sibling PRs from the same review: #15, #16, #17 and #18. Their source changes merge cleanly with each other. Several of them add checks at the same spot in `tests/pause_check.gd` (and two in `tests/save_check.gd`), so whichever merges second may need a small rebase that keeps both blocks.
ui: hidden menu panels stop catching pointers and rendering
Some checks failed
tests / test (pull_request) Failing after 56s
1dc765d8ee
A hidden MenuPanel kept its collider on the pointer layer and its
SubViewport rendered every frame. After closing pause, the invisible
pause panel stayed where it was opened, so back in the menu it could
block the laser and take clicks meant for the main menu, including
Options changes that get saved.

The panel now follows its visibility in the tree: while hidden its
collider is off, its viewport stops updating and pointer events are
ignored. A press it was holding is let go by hiding and reshowing its
menu control, which ends the press without clicking a button or
moving a slider. Game ignores main menu actions outside MENU and pause
actions while not paused. Level.apply_mode also disables processing
for the hidden content groups, so physics objects and scripts added
there later go quiet with them.
Some checks failed
tests / test (pull_request) Failing after 56s
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u menu-panels-inactive-when-hidden:hiccup444-menu-panels-inactive-when-hidden
git switch hiccup444-menu-panels-inactive-when-hidden

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff hiccup444-menu-panels-inactive-when-hidden
git switch hiccup444-menu-panels-inactive-when-hidden
git rebase main
git switch main
git merge --ff-only hiccup444-menu-panels-inactive-when-hidden
git switch hiccup444-menu-panels-inactive-when-hidden
git rebase main
git switch main
git merge --no-ff hiccup444-menu-panels-inactive-when-hidden
git switch main
git merge --squash hiccup444-menu-panels-inactive-when-hidden
git switch main
git merge --ff-only hiccup444-menu-panels-inactive-when-hidden
git switch main
git merge hiccup444-menu-panels-inactive-when-hidden
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
dead-air/engine!19
No description provided.