Skip to content

feat(activity): finish the activity when the game quits (FP#979) - #35

Merged
apotema merged 2 commits into
mainfrom
feat/979-finish-activity
Sep 30, 2026
Merged

apotema merged 2 commits into
mainfrom
feat/979-finish-activity

Conversation

@apotema

@apotema apotema commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds labelle_android.activity.finish(activity), the Android half of making game.quit() close the app.

Today, on Android, the menu Exit button and FP #979's boot-failure Exit button do nothing. game.quit() only flips the engine's running flag, and a NativeActivity is not closed by its native loop ending. Something has to ask the framework to finish the activity.

finish(activity), new in src/activity.zig and src/jni/activity_finish.c:

  1. Calls ANativeActivity_finish(activity), which posts to the UI thread. Calling contract: finish must be called from the glue app thread while the activity holds its window. src/activity.zig explains why that orders the onDestroy hook write before the UI thread reads it. The normal lifecycle follows: PAUSE, TERM_WINDOW, STOP, DESTROY. The backend therefore tears down the same way it does when the user backs out.
  2. Chains the activity's onDestroy. The glue's own onDestroy runs first; it blocks until the backend's android_main has returned. Then the process ends with _exit(0).
    • Why end the process: the engine is a process global that outlives activity instances (labelle-bgfx#143). After a quit, running stays false, so a relaunch into the cached process would restore the quit game and finish again at once. Ending the process makes the next launch a cold start, which matches desktop, where quitting ends the process. sokol's Android backend does the same (exit(0) at the end of its onDestroy).
    • Why _exit rather than exit: the engine's asset and audio worker threads are still live, and exit would run C++ static destructors under them. Logging goes straight to logcat, so nothing needs flushing.
  3. Only the first call in a process does anything; the once-guard is request in Zig, with host tests. It is a no-op off Android.

The caller is labelle-toolkit/labelle-bgfx#186, which adds one call site in its Android shell, next to the existing launch_intent.apply call.

Part of moca-tecnologia/flying-platform-labelle#979 / labelle-toolkit/labelle-bgfx#172.

Tests

  • zig build test → exit 0 (host; includes the activity.test.* tests for the no-op and the once-guard)
  • zig build test -Dtarget=aarch64-linux-android → exit 0 (compiles activity_finish.c against the NDK)

Runtime: Mac emulator (API 36, arm64)

Setup: FP #979 (fix/977-loading-terminal-failure @ a8d2eae) with uncommitted local: overrides to this branch and to labelle-bgfx feat/979-android-quit. Built with labelle build --platform=android --optimize=ReleaseFast (exit 0). libgame.so defines labelle_android_finish_activity and imports ANativeActivity_finish and _exit.

Case Result
Main menu Exit (Vulkan) android: quit requested; finishing the activity → surface_lost → bgfx: shutdownPrograms + bgfx.shutdown → android: activity finished after a quit request; ending the process → Zygote: Process … exited cleanly (0). pidof is empty and dumpsys activity activities shows no FP record. No FATAL or signal lines.
#979 failure panel Exit: four ASTC magic bytes zeroed in the embedded sky atlas of a copied, re-signed APK (Codex's method) The panel shows menu / sky__background / LoadFailed. After HOME and resume, the same PID keeps the panel. Exit gives the same finish → teardown → exited cleanly (0) sequence.
#979 failure panel Exit on GLES (the emulator refuses ASTC 8x8, so the panel appears naturally) Same sequence.
Relaunch after a quit New PID, bgfx: first surface (cold init); not a restore.
HOME/resume and normal play Same PID, re-init against new surface (restore). New Game plays at 60 FPS. No quit lines.

Not done

  • Not verified on a physical device (SM-T505); tested only on the emulator.
  • Two activity instances alive at once (#143) at the moment of quit: the process exit also ends the second instance. This case was not exercised.

https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ

`activity.finish(activity)` calls ANativeActivity_finish (thread-safe) and
chains the activity's onDestroy so that, after the glue's own onDestroy has
waited for the backend's android_main to return, the process ends with
_exit(0). The engine is a process global that outlives activity instances
(labelle-bgfx#143), so without ending the process a relaunch would restore
the quit game and finish again at once. First call per process only; a
no-op off Android.

Part of moca-tecnologia/flying-platform-labelle#979 / labelle-toolkit/labelle-bgfx#172.

Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T14:32:34.992304Z 177ab5f New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: b44ed051-764e-480f-a7f5-6e94cd414f22

📥 Commits

Reviewing files that changed from the base of the PR and between 0fac554 and e15a35f.

📒 Files selected for processing (5)
  • README.md
  • build.zig
  • src/activity.zig
  • src/jni/activity_finish.c
  • src/root.zig

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

The PR adds a public activity.finish API. On Android, it requests activity teardown and exits the process after the matching activity’s destroy callback. The root module exports the API, and the Android build compiles its native helper.

Changes

Activity finish service

Layer / File(s) Summary
Public API and module integration
src/activity.zig, src/root.zig, README.md
Adds finish(activity), which returns false off Android and delegates to the native bridge on Android. Exports the activity module, references it in the root test block, and documents the service.
Android finish and teardown
src/jni/activity_finish.c, build.zig
Adds the Android helper. It rejects null activities and repeated requests, preserves the glue onDestroy callback, requests activity finish, then exits after the matching activity is destroyed. The Android build includes the helper.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ZigActivity as activity.finish
  participant FinishBridge as labelle_android_finish_activity
  participant AndroidActivity as ANativeActivity
  participant DestroyCallback as wrapped onDestroy
  ZigActivity->>FinishBridge: Request activity finish
  FinishBridge->>AndroidActivity: Install callback and request finish
  AndroidActivity->>DestroyCallback: Invoke onDestroy
  DestroyCallback->>DestroyCallback: Call glue callback
  DestroyCallback->>DestroyCallback: Exit process if activity matches
Loading

Merge Risk: ⚪ Minimal · up to e15a3

This adds an API that closes the Android app and exits the process on quit. Emulator testing was reported as successful, and no concrete merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (4 skipped: 4 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description clearly explains the Android activity finish behavior, lifecycle handling, process-exit rationale, tests, and known limitations. It directly matches the changeset.
Title check ✅ Passed The title clearly and concisely summarizes the main change: finishing the Android activity when the game quits.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e15a35f013

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/jni/activity_finish.c
@apotema

apotema commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Codex, OpenAI LLM. VERDICT: CHANGES REQUESTED exact e15a35f01334443371a13f4a8064d93e3b2ca7f0. At src/jni/activity_finish.c42-48, the advertised any-thread finish API writes the UI-owned onDestroy callback and plain finishing/glue_on_destroy state without synchronization against independently concurrent UI destruction. The atomic_flag serializes callers but not UI lifecycle callback reads. Posting ANativeActivity_finish afterward orders its own requested destruction, not one already in progress. Install the hook on the UI thread during lifecycle initialization before worker access, or narrow/enforce the safe caller contract. Existing resumed-frame/glue synchronization may protect the currently tested bgfx call site; this finding is about the exported any-thread contract, not a claim that menu Exit failed. Root+single read-only helper source review; no redundant heavy job. _exit after normal teardown is accepted for the documented cold-start policy.

Codex (PR #35): finish() was advertised as callable from any thread, but it
writes callbacks->onDestroy, which the UI thread reads on destruction; an
any-thread caller has no ordering against a concurrent destroy.

There is no UI-thread init point in this package (ANativeActivity_onCreate
belongs to the backend's glue), so the contract is narrowed and documented:
call from the glue app thread while the activity holds its window.
NativeActivity.onDestroy destroys the surface before the native onDestroy,
and the glue's onNativeWindowDestroyed blocks until the app thread has
processed TERM_WINDOW, so the hook write happens-before the read.

The once-guard moves to Zig (`request`) with host tests: null never takes
it, repeats never reach the JNI half, and a finish that did nothing
releases it.

Part of moca-tecnologia/flying-platform-labelle#979 / labelle-toolkit/labelle-bgfx#172.

Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ
@apotema

apotema commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Replying to Codex's review of e15a35f (issuecomment-5912790140). The fix is in 177ab5ffb00cd8965d1f2461ae801d131354d0fb.

Change: I narrowed the contract rather than installing the hook on the UI thread. This package has no UI-thread initialization point. ANativeActivity_onCreate belongs to the backend's native_app_glue: bgfx compiles it, and sokol has its own. Hooking onCreate from here would mean changing the manifest's android.app.func_name and wrapping the glue for both backends, which is well outside #979. So:

  • The contract, documented in src/activity.zig and the README: call finish from the glue app thread while the activity holds its window (after INIT_WINDOW, before the caller has processed TERM_WINDOW). The bgfx caller (labelle-bgfx#186) only calls it after a frame, while bgfx_ready, which holds only between INIT_WINDOW and TERM_WINDOW on that thread.

  • Why the write to callbacks->onDestroy then happens-before the UI thread reads it:

    1. NativeActivity.onDestroy (UI thread) calls onSurfaceDestroyedNative before unloadNativeCode → native onDestroy.
    2. The glue's onNativeWindowDestroyed → android_app_set_window(NULL) posts TERM_WINDOW and waits on the glue mutex/condvar until the app thread's android_app_post_exec_cmd sets window = NULL and broadcasts.
    3. The app thread only processes TERM_WINDOW after finish has returned (program order).

    So the hook writes → unlock → the UI thread leaves onNativeWindowDestroyed → the UI thread reads the slot. No destroy can be reading the slot concurrently while the caller holds the window.

  • The once-guard moved to Zig (request), with host tests:

    • A null activity never takes the guard.
    • A repeat call never reaches the JNI half.
    • A finish that did nothing (no callbacks table) releases the guard.
  • _exit(0) after the glue's normal destroy is unchanged.

Tests:

  • zig build test → exit 0.
  • zig build test -Dtarget=aarch64-linux-android → exit 0.
  • The three activity.test.* tests run on the host.

Emulator (API 36, arm64; SM-T505 not touched). FP #979 @ a8d2eae with uncommitted local: overrides to 177ab5f and bgfx #186 (on the #182 Vulkan base). ReleaseFast build exit 0; deps matched the branches.

  • Menu Exit: the log shows, in order:

    • quit requested; finishing the activity
    • surface_lost
    • bgfx.shutdown
    • activity finished after a quit request; ending the process
    • Zygote: Process 14071 exited cleanly (0)

    Afterwards pidof is empty, dumpsys shows no activity record, and there are 0 FATAL or signal lines.

  • Relaunch: new PID 14178, bgfx: first surface (cold init).

https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ

@apotema

apotema commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Codex, OpenAI LLM.

VERDICT: APPROVED at exact 177ab5ffb00cd8965d1f2461ae801d131354d0fb for this provider activity-finish change. Root and the one read-only helper found no remaining race under the documented app/glue-thread, live-window contract: TERM_WINDOW acknowledgement orders hook installation before UI destruction continues. Host tests: 275 pass, 6 skip; aarch64 Android compile check exits0; all eight current CI jobs green. The earlier any-thread review finding is addressed by the narrowed contract and its live/resumed backend caller.

Independent Windows runtime integration uses exact35 with bgfx186 88d93619073712fd5168880970a189a0b153eda2, current approved frame-counter/renderer changes and the local zbgfx batch. CLI4 ReleaseFast build through -j2 wrapper exits0. API36 rootable Google APIs emulator37.1.11, host GPU, -cores2; x86_64 ELF relocated in a locally re-signed APK; no local overrides committed.

  • Vulkan main menu Exit: quit request → bgfx.shutdown → activity destruction → process exited cleanly0; PID gone. Cold relaunch used new PID3483.
  • Gameplay HOME/resume: same PID3483, renderer restored, 660 new frames over12s, startup markers cleared.
  • Deliberately corrupted sky atlas: LoadFailed panel remains after Vulkan HOME/resume with same PID3680, 660 new frames over12s. Panel Exit shuts down and exits cleanly0. GLES panel also renders780frames over12s and Exit cleanly0.
  • Non-debuggable manifest verified without DEBUGGABLE: GLES intent override ignored, actualVulkan/source setting,780frames; menu Exit cleanly0. Root/global Khronos layers still loaded; this proves renderer-extra gating only, not global-layer gating.
  • Early force-stop at2s leaves .labelle_vulkan_start; next launch selects actualOpenGLES/source crash-guard and renders600frames over12s. Fresh healthy Vulkan retains the marker at120frames before10s, clears it at600frames after10s. Earlier healthy launch and resume also cleared markers. The final healthy-run tap did not trigger Exit (intro was still active); it is not counted as an additional Exit pass.

No captured fatal/abort in these logs. Known #178 object-lifetime VUIDs remain on this batch-without178 runtime and are separate from this provider verdict; no validation-clean or #178 approval claim. Local evidence: D:/prj/labelle-toolkit/zbgfx-172-exit35-evidence, build/host/cross-compile logs alongside it. Emulator now off and no local heavy job.

This does not approve bgfx186's stale released-provider pin: release35 first, then re-pin186 to that release, fresh CI and exact-head peer review. You remain the merge author; release and coordination gates remain in effect.

@apotema
apotema merged commit a561e1f into main Sep 30, 2026
9 checks passed
@apotema
apotema deleted the feat/979-finish-activity branch September 30, 2026 15:34
apotema added a commit that referenced this pull request Sep 30, 2026
…ard, activity finish (#37)

Minor release for labelle-bgfx#172 (Android Vulkan) and FP#979 (Android Exit):
- "renderer" provider setting + labelle.renderer meta-data + optional Vulkan feature (#26/#31)
- launch-time renderer resolution → LABELLE_BGFX_RENDERER (#27/#32)
- Vulkan crash guard: frames+time stable rule, no_backup markers (#28/#33)
- docs: emulator testing + choosing a renderer (#29/#34)
- activity.finish(): game.quit() closes the app on Android (#35)
docs/renderer.md now names 0.4.0.

Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ
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.

1 participant