Skip to content

fix: preserve VN portrait topmost state on macOS - #262

Merged
Lucas1479 merged 3 commits into
Code-Amadeus:mainfrom
FDU-ZJN:fix/macos-vn-portrait-topmost
Nov 9, 2026
Merged

Lucas1479 merged 3 commits into
Code-Amadeus:mainfrom
FDU-ZJN:fix/macos-vn-portrait-topmost

Conversation

@FDU-ZJN

@FDU-ZJN FDU-ZJN commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What and why

On macOS, the VN portrait can lose its always-on-top setting when Aqua Tk creates or redisplays its borderless native window. A normal game window can then cover the portrait when activated.

Realize the native window before applying -topmost, and restore that attribute after deiconify() through one shared show() method. Both visibility changes and Lite reaction presentation use this method. This fixes the window lifecycle at the Tk shell without polling or forcing keyboard focus.

The failure was reproduced with unmodified upstream render/vn_overlay_window.py at 9547f39a: after withdraw() → deiconify() → update(), -topmost changes from 1 to 0.

Change class

  • Routine bug fix

Owning layer: native VN portrait window lifecycle.

User-visible effect: preserve portrait placement above ordinary application windows during display and redisplay.

Compatibility or migration impact: none; existing dependencies and configuration are unchanged.

Evidence

Tested on Darwin arm64, uv-managed CPython 3.12.14, Pillow 13.0.0, Tcl/Tk 9.0.4.

  • python -m pytest tests/test_vn_overlay_window_contract.py tests/test_vn_overlay_continuity.py tests/test_vn_overlay_auth.py tests/test_vn_portrait_environment.py tests/test_vn_portrait_shared.py -q — 38 passed on this branch.
  • ruff check render/vn_overlay_window.py tools/vn_portrait_overlay_lite.py tests/test_vn_overlay_window_contract.py tests/test_vn_overlay_continuity.py — passed.
  • git diff --check — passed before committing.
  • Native regression test creates a real portrait without character assets and checks initial display plus two hide/show cycles. It runs on macOS; other platforms skip this Aqua-specific check.
  • Native inspection of the repaired portrait showed NSWindow.level = 19, visible = true, and hidesOnDeactivate = false.
  • Installed Lite assets retained topmost through initial display, hide/show, synthetic speech start and completion. Real-game focus switching has also passed; see acceptance results below.

Review follow-up and real-machine acceptance

Commit 8b23fec adds tests/test_vn_overlay_window_contract.py to the platform-safe test list in .github/workflows/python-macos.yml, so the Aqua native regression test now runs in macOS CI. Local rerun of this file plus tests/test_vn_portrait_environment.py: 20 passed.

Real-machine acceptance, confirmed by the tester:

  • Initial display: portrait remains above an ordinary game window after activating the game — passed.
  • Hide/show: portrait remains above the game after hiding and redisplaying it — passed.
  • Keyboard focus: after clicking the game, keyboard input remains with the game — passed.

Environment: macOS 26.6.2 arm64, uv-managed CPython 3.12.14, Pillow 13.0.0, Tcl/Tk 9.0.4. Acceptance used the local OCR checkout with portrait changes identical to e0c46cf; the 8b23fec follow-up changes only the CI test list.

Final check

  • One coherent problem without unrelated cleanup
  • Existing lifecycle callers share the fix
  • No secrets, local state, logs, or character assets included
  • Third-party notices and provenance preserved

@FDU-ZJN
FDU-ZJN requested a review from Lucas1479 as a code owner October 25, 2026 15:40

@Lucas1479 Lucas1479 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed at e0c46cf. The fix is appropriately scoped to the native Tk window lifecycle: realize the window before setting -topmost, and restore it through the shared show() path. I found no functional regression in the reviewed changes.

Before merging, please complete two validation items:

  1. Add tests/test_vn_overlay_window_contract.py to the test list in .github/workflows/python-macos.yml. The new native regression test is skipped outside macOS, but neither macOS workflow currently includes this file, so the existing CI configuration never executes it. This is a regression-coverage gap rather than evidence that the implementation is incorrect.
  2. Complete the real-game focus-switching check noted in the PR description. Please confirm that the portrait remains above an ordinary game window on initial display and after hide/show, and that keyboard input stays with the game. Record the result and relevant macOS/Tk environment in the PR.

Independent Windows validation on this commit: all seven VN overlay/portrait test files passed (45 passed, 1 skipped); Ruff passed for the changed Python files; the native initial-display and two hide/show-cycle check also passed when invoked directly on Windows. The skipped pytest case is the Aqua-specific test, so these results do not independently validate macOS behavior.

The Windows checks-and-smoke failure is in dependency auditing for the existing filelock and setuptools versions. This PR changes neither dependencies nor the lockfile; we will handle that separately on the maintenance side. No dependency changes are requested in this PR.

@FDU-ZJN

FDU-ZJN commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Added the missing macOS coverage in 8b23fec: tests/test_vn_overlay_window_contract.py is now included in .github/workflows/python-macos.yml. The local rerun of this file and the portrait environment tests passed: 20 passed.

The real-game acceptance check also passed on macOS 26.6.2 arm64, uv-managed CPython 3.12.14, Pillow 13.0.0, Tcl/Tk 9.0.4:

  • The portrait stays above an ordinary game window on initial display after clicking the game.
  • It stays above the game after hide/show.
  • Keyboard input stays with the game after clicking it.

@Lucas1479
Lucas1479 merged commit be9b97a into Code-Amadeus:main Nov 9, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants