Repository navigation
Add Android support for Chronicle - #188
Conversation
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b62464385
ℹ️ 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".
| return; | ||
| } | ||
| jclass cls = env->GetObjectClass(activity); | ||
| jmethodID method = cls ? env->GetMethodID(cls, "setTouchControlsOpacity", "(F)V") : nullptr; |
There was a problem hiding this comment.
Implement the Java opacity callback before looking it up
On every Android launch, Run calls this function at port/src/main.cpp:453, but a repo-wide search finds no setTouchControlsOpacity(float) implementation in ChronicleActivity (only the visibility callback exists). GetMethodID therefore raises a pending NoSuchMethodError before renderer initialization, the new opacity option never applies, and subsequent JNI calls can abort the SDL thread; add the activity method and handle lookup exceptions.
Useful? React with 👍 / 👎.
| jobject activity = static_cast<jobject>(SDL_GetAndroidActivity()); | ||
| if (env == nullptr || activity == nullptr) { | ||
| return; | ||
| } | ||
| jclass cls = env->GetObjectClass(activity); |
There was a problem hiding this comment.
Release JNI references after each status update
During first-run extraction, the progress callback calls AndroidShowStatus up to every 500 ms. SDL_GetAndroidActivity() returns a JNI local reference and GetObjectClass() creates another, but this helper never deletes either; because SDL_main is the long-lived JNI invocation, these references accumulate instead of being popped after each C++ helper return, so a slow extraction can overflow Android's local-reference table and abort setup. Delete both references on every path, and apply the same ownership rule in the hide/picker helpers.
Useful? React with 👍 / 👎.
| } | ||
| std::unique_lock lock(android_picker_mutex); | ||
| android_picker_changed.wait(lock, [] { return android_picker_answered; }); | ||
| return android_picker_path; |
There was a problem hiding this comment.
Consume the picker result before waiting for a retry
When the initial picker is canceled or the selected image fails extraction, this returns without clearing android_picker_answered (and for a non-null result it also leaves android_picker_path populated). The error branches then call WaitForAndroidRetry, whose predicate is already true, so cancellation immediately reopens the picker behind the error dialog and a failed multi-gigabyte image is automatically extracted a second time instead of waiting for the user. Consume and reset the result under the lock before returning.
Useful? React with 👍 / 👎.
| .setTitle("Disc image could not be imported") | ||
| .setMessage(message) | ||
| .setPositiveButton("Choose another image", (dialog, which) -> requestDiscImage()) | ||
| .setNegativeButton("Close", (dialog, which) -> finish()) |
There was a problem hiding this comment.
Wake the native retry wait when Close is selected
After a canceled selection or copy/extraction failure, native startup can be blocked in WaitForAndroidRetry, but the negative button only finishes the Activity and never changes or notifies that condition. The SDL startup thread therefore remains blocked during Activity teardown, so choosing the offered Close action can hang instead of exiting; add an explicit native cancellation/exit signal that wakes the waiter and terminates the first-run loop.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 455dd7e6e8
ℹ️ 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".
| file(COPY ${CMAKE_SOURCE_DIR}/tools/font/DarkCloudCompendium.ttf DESTINATION ${ANDROID_ASSETS}/lang) | ||
| file(COPY ${CMAKE_SOURCE_DIR}/tools/font/DarkCloudCompendiumThin.ttf DESTINATION ${ANDROID_ASSETS}/lang) |
There was a problem hiding this comment.
Rename Android fonts to the filenames the runtime loads
On a fresh Android install these copies preserve the basenames DarkCloudCompendium.ttf and DarkCloudCompendiumThin.ttf (which are also the names committed under assets/lang), but the runtime only searches for font.ttf in port/src/clsmes.cpp:58 and font_thin.ttf in port/src/caption_text.cpp:130. Since font_sharp defaults to true, the shipped fonts are never found and text silently falls back to the PS2 bitmaps; copy them with the same renamed filenames used by the desktop branch.
Useful? React with 👍 / 👎.
| cy[8] = cy[9] = h * .215f; cy[10] = cy[11] = h * .10f; | ||
| cx[12] = w * .43f; cx[13] = w * .57f; cy[12] = cy[13] = h * .92f; | ||
| cx[14] = (side + .025f) * w; cx[15] = (1 - side - .025f) * w; | ||
| cy[14] = cy[15] = h * .86f; |
There was a problem hiding this comment.
Keep 720p sticks clear of the button clusters
On a supported 1280×720 landscape display, these equations place the 80px left stick at (192, 619) while the 46px Down button is at (160, 522), only about 103px apart for a combined radius of 126px (the right stick and Cross mirror this). Because hit() scans the buttons before the sticks, touches in each overlapping stick cap are reported as Down/Cross instead of analog input, so part of both rendered sticks is unusable on 720p touch displays; derive the spacing or radii so these circles do not intersect.
Useful? React with 👍 / 👎.
Summary
ps2/files change; use the shared Vulkan renderer from Add opt-in frame resolution scaling to the shared Vulkan renderer Kazarov#9.profilebuild variant; leave Debug behavior unchanged.Review updates
611182d, including renderer/input review corrections, redundant Android guard cleanup, and the rotated-surface swapchain fix.Building
The submodule pins the Kazarov PR branch until its dependency merges. APKs contain no game data; players supply their own disc image.
Validation and scope
ARM64 and x86_64 builds succeeded during local validation. On an API 35 x86_64 emulator using host RTX 3080 Vulkan, a fixed Wise Owl Forest camera workload improved from about 14.5 ticks/s to 59.5, with a 60.0 repeat after the swapchain correction. These are aggregate counter measurements, not frame-time percentiles or Fold benchmarks. The measured build also included the independently proposed shadow allocation change; its isolated gain was small and inconclusive.
The ordinary APK survived emulator display-size changes, background/resume and short screen recordings. Physical Fold/Samsung recorder and desktop regression validation remain pending. The final redundant-preprocessor cleanup was checked structurally and formatted but not rebuilt. Repository formatting and whitespace checks passed. No APKs, game assets, saves or benchmark harnesses are committed.
Dependency compatibility — do not merge yet
Requires Kazarov #9 and a final compatible upstream submodule pin before merging. The current pin is 611182d on codex/chronicle-android-compatible: it merges Chronicle's upstream renderer pin 1a8f9ab into the reviewed Android revision 69c4247, retaining the audio APIs Chronicle uses while adding upstream's game-icon support. Kazarov #9's newer b08520d head still requires the corresponding Chronicle audio/asset migration; this PR does not claim to complete that migration.
Merge conflict resolution (2026-10-17)
Merged upstream master 50eaff6, preserved the Android build documentation alongside the new upstream README, and retained both the Android startup trace and upstream data-icon setup. The renderer compatibility merge resolves the submodule conflict without adopting the incompatible audio API changes.
Validation: Windows game and test executables built successfully; 85 selected icon, audio, and input tests passed (one benchmark disabled). Android ARM64 RelWithDebInfo native library built successfully. No new APK packaging or device runtime test was performed for this merge. CI must complete on the updated PR.