Repository navigation
feat(activity): finish the activity when the game quits (FP#979) - #35
Conversation
`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
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
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. 📝 WalkthroughWalkthroughThe PR adds a public ChangesActivity finish service
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
Merge Risk: ⚪ Minimal · up to 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)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 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".
|
Codex, OpenAI LLM. VERDICT: CHANGES REQUESTED exact |
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
|
Replying to Codex's review of Change: I narrowed the contract rather than installing the hook on the UI thread. This package has no UI-thread initialization point.
Tests:
Emulator (API 36, arm64; SM-T505 not touched). FP #979 @ a8d2eae with uncommitted
|
|
Codex, OpenAI LLM. VERDICT: APPROVED at exact Independent Windows runtime integration uses exact35 with bgfx186
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: 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. |
…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
Summary
Adds
labelle_android.activity.finish(activity), the Android half of makinggame.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'srunningflag, and a NativeActivity is not closed by its native loop ending. Something has to ask the framework to finish the activity.finish(activity), new insrc/activity.zigandsrc/jni/activity_finish.c:ANativeActivity_finish(activity), which posts to the UI thread. Calling contract:finishmust be called from the glue app thread while the activity holds its window.src/activity.zigexplains why that orders theonDestroyhook 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.onDestroy. The glue's ownonDestroyruns first; it blocks until the backend'sandroid_mainhas returned. Then the process ends with_exit(0).runningstays 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 itsonDestroy)._exitrather thanexit: the engine's asset and audio worker threads are still live, andexitwould run C++ static destructors under them. Logging goes straight to logcat, so nothing needs flushing.requestin 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.applycall.Part of moca-tecnologia/flying-platform-labelle#979 / labelle-toolkit/labelle-bgfx#172.
Tests
zig build test→ exit 0 (host; includes theactivity.test.*tests for the no-op and the once-guard)zig build test -Dtarget=aarch64-linux-android→ exit 0 (compilesactivity_finish.cagainst the NDK)Runtime: Mac emulator (API 36, arm64)
Setup: FP #979 (
fix/977-loading-terminal-failure@ a8d2eae) with uncommittedlocal:overrides to this branch and to labelle-bgfxfeat/979-android-quit. Built withlabelle build --platform=android --optimize=ReleaseFast(exit 0).libgame.sodefineslabelle_android_finish_activityand importsANativeActivity_finishand_exit.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).pidofis empty anddumpsys activity activitiesshows no FP record. No FATAL or signal lines.menu / sky__background / LoadFailed. After HOME and resume, the same PID keeps the panel. Exit gives the same finish → teardown →exited cleanly (0)sequence.bgfx: first surface (cold init); not a restore.re-init against new surface (restore). New Game plays at 60 FPS. No quit lines.Not done
https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ