Add in-game screenshots#1627
Open
VinceBT wants to merge 1 commit into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds screenshot capture, gallery viewing, controller binding, library and settings entry points, and localized strings across the app. ChangesScreenshots feature rollout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 46 files
Note: This PR contains a large number of files. cubic only reviews up to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/component/QuickMenu.kt (1)
716-733: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNo initial focus when the Screenshots tab is the active tab.
QuickMenuTab.SCREENSHOTSisn't handled here, so it falls intoelse -> controllerItemFocusRequester.requestFocus(). That requester is only attached in the controller content branch (Line 694), which isn't composed whileScreenshotsQuickMenuTabis shown. The call throwsIllegalStateException(FocusRequester not initialized), is swallowed by thecatch, retried 3×, then gives up — leaving the screenshots tab with no initial focus for D-pad/controller navigation. SinceselectedTabis persisted viaPrefManager.quickMenuLastTab, reopening the menu on this tab reproduces it.Consider adding a dedicated focus requester wired into
ScreenshotsQuickMenuTaband a branch here, e.g.:when (selectedTab) { QuickMenuTab.HUD -> hudItemFocusRequester.requestFocus() QuickMenuTab.LSFG -> lsfgItemFocusRequester.requestFocus() QuickMenuTab.EFFECTS -> effectsItemFocusRequester.requestFocus() QuickMenuTab.TOOLS -> toolsItemFocusRequester.requestFocus() + QuickMenuTab.SCREENSHOTS -> screenshotsItemFocusRequester.requestFocus() else -> controllerItemFocusRequester.requestFocus() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt` around lines 716 - 733, The initial focus logic in LaunchedEffect(isVisible) does not handle QuickMenuTab.SCREENSHOTS, so it incorrectly falls back to controllerItemFocusRequester and fails when ScreenshotsQuickMenuTab is composed. Add a dedicated focus requester for the screenshots tab, attach it to the ScreenshotsQuickMenuTab branch, and extend the selectedTab when block to request that focus explicitly instead of using the controller fallback.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt (1)
84-91: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
pressedKeyCodescan leak stale held-keys across focus changes.If an
ACTION_UPis never delivered (e.g., the window loses focus while a combo button is held), that keycode stays inpressedKeyCodes, which can cause the combo to fire spuriously or fail to re-arm later. Consider clearingpressedKeyCodes(and resettingscreenshotComboFired) incleanup()and/orsetProfile()so the combo state can't wedge.Also applies to: 98-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt` around lines 84 - 91, The cleanup path in PhysicalControllerHandler is leaving combo key state behind, which can wedge later input handling. Update cleanup() and/or setProfile() so pressedKeyCodes is cleared and screenshotComboFired is reset whenever focus/profile changes, alongside the existing releaseActiveAxes(), mouseMoveTimer, and clearScrollRepeats() resets. Make sure the combo state is fully re-armed after loss of focus so stale held keys cannot survive across sessions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/utils/ScreenshotManager.kt`:
- Around line 142-157: The Q+ branch in ScreenshotManager’s MediaStore save flow
leaves a pending Downloads row behind if resolver.openOutputStream or
item.file.inputStream().copyTo fails after resolver.insert succeeds. Update the
logic around the insert/update block in ScreenshotManager to clean up the
inserted uri on failure by catching exceptions or using a finally path that
deletes the row when the copy does not complete, while still preserving the
existing IS_PENDING=0 update on success.
- Around line 158-172: The pre-Q Downloads export path in ScreenshotManager is
writing directly via FileOutputStream without first ensuring storage permission
is granted, so add a permission request/check before entering this branch for
API 26–28. Update the export flow around the Downloads handling in
ScreenshotManager so it requests WRITE_EXTERNAL_STORAGE before creating the
destination file, and only proceeds with the copy once permission is approved.
Keep the existing de-duplication and file creation logic intact after the
permission gate.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt`:
- Around line 716-733: The initial focus logic in LaunchedEffect(isVisible) does
not handle QuickMenuTab.SCREENSHOTS, so it incorrectly falls back to
controllerItemFocusRequester and fails when ScreenshotsQuickMenuTab is composed.
Add a dedicated focus requester for the screenshots tab, attach it to the
ScreenshotsQuickMenuTab branch, and extend the selectedTab when block to request
that focus explicitly instead of using the controller fallback.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt`:
- Around line 84-91: The cleanup path in PhysicalControllerHandler is leaving
combo key state behind, which can wedge later input handling. Update cleanup()
and/or setProfile() so pressedKeyCodes is cleared and screenshotComboFired is
reset whenever focus/profile changes, alongside the existing
releaseActiveAxes(), mouseMoveTimer, and clearScrollRepeats() resets. Make sure
the combo state is fully re-armed after loss of focus so stale held keys cannot
survive across sessions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10a0443a-9cdd-42c3-a3b9-d6473cafd469
📒 Files selected for processing (46)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/component/AccentActionRow.ktapp/src/main/java/app/gamenative/ui/component/FocusRing.ktapp/src/main/java/app/gamenative/ui/component/GamepadInputView.ktapp/src/main/java/app/gamenative/ui/component/InfoCard.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/component/RememberScreenshots.ktapp/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.ktapp/src/main/java/app/gamenative/ui/component/ScreenshotToastOverlay.ktapp/src/main/java/app/gamenative/ui/component/ScreenshotsPreviewStrip.ktapp/src/main/java/app/gamenative/ui/component/ScreenshotsQuickMenuTab.ktapp/src/main/java/app/gamenative/ui/component/dialog/ScreenshotControllerBindDialog.ktapp/src/main/java/app/gamenative/ui/enums/AppOptionMenuType.ktapp/src/main/java/app/gamenative/ui/screen/PluviaScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryDetailPane.ktapp/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotGalleryScreen.ktapp/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotViewer.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupScreenshots.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsScreen.ktapp/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/ui/util/ScreenshotNotificationManager.ktapp/src/main/java/app/gamenative/utils/ScreenshotManager.ktapp/src/main/java/app/gamenative/utils/ScreenshotStore.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/xml/file_provider_paths.xmlapp/src/test/java/app/gamenative/utils/ScreenshotStoreTest.kt
VinceBT
added a commit
to VinceBT/GameNative
that referenced
this pull request
Jun 26, 2026
P1: - Sanitize appId in ScreenshotStore.gameDir to prevent path traversal - Guard rememberScreenshots listing against I/O/SecurityException crashes - Make LibraryDetailPane.onNavigateRoute required and wire it through the Downloads screen so screenshot taps work there P2: - Scope screenshot combo key tracking by deviceId so keys from different controllers can't falsely combine; refresh the combo on each fresh press so a mid-session rebind takes effect - Convert screenshots_deleted_all to a <plurals> resource across all locales (one/few/other as the language requires) and use getQuantityString
Contributor
There was a problem hiding this comment.
2 issues found across 23 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
VinceBT
added a commit
to VinceBT/GameNative
that referenced
this pull request
Jun 26, 2026
…ed_all Provide the full Slavic one/few/many/other plural forms instead of an other-only fallback, so deletion counts are grammatically correct (cubic review on PR utkarshdalal#1627).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.kt`:
- Around line 99-108: Reset the keyboard latch in
PhysicalControllerHandler.resetPressedKeys() as well as the screenshot state.
This helper already clears pressedKeyCodesByDevice and screenshotComboFired when
input forwarding stops, so also clear showKeyboardPressed here to avoid leaving
the SHOW_KEYBOARD binding latched true after focus changes; use the existing
resetPressedKeys() method and the showKeyboardPressed field to locate the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eec1bfe2-d7a9-4217-92a5-8c900d213b86
📒 Files selected for processing (9)
app/src/main/java/app/gamenative/ui/component/AccentActionRow.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/component/ScreenshotsQuickMenuTab.ktapp/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotViewer.ktapp/src/main/java/app/gamenative/ui/screen/xserver/PhysicalControllerHandler.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ScreenshotManager.ktapp/src/main/java/app/gamenative/utils/ScreenshotStore.ktapp/src/test/java/app/gamenative/utils/ScreenshotStoreTest.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- app/src/main/java/app/gamenative/ui/component/AccentActionRow.kt
- app/src/main/java/app/gamenative/utils/ScreenshotStore.kt
- app/src/main/java/app/gamenative/ui/component/ScreenshotsQuickMenuTab.kt
- app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
- app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotViewer.kt
- app/src/main/java/app/gamenative/utils/ScreenshotManager.kt
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Contributor
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
VinceBT
added a commit
to VinceBT/GameNative
that referenced
this pull request
Jun 27, 2026
showKeyboardPressed has the same lifetime as the screenshot combo state: if the keyboard binding is held when a menu/editor steals focus, its ACTION_UP is routed away and the latch stays true, so the next SHOW_KEYBOARD press is ignored. Reset it alongside the combo state. Addresses PR utkarshdalal#1627 review feedback.
Contributor
There was a problem hiding this comment.
3 issues found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotViewer.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotViewer.kt:367">
P2: Duplicated hardcoded scrim color literal `Color(0x88000000)`. This same value is used on the bottom toolbar as well. Move it to a named constant in the theme (e.g., `val ScreenshotScrim = Color(0x88000000)`) and reference it from both locations to prevent drift.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/component/InfoCard.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/InfoCard.kt:92">
P2: Label Text in the header Row lacks width constraints and overflow handling, risking clipped or unstable layout with localized text.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotGalleryScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotGalleryScreen.kt:137">
P2: FocusRequester.requestFocus() returns Boolean indicating success or cancellation, but the return value is ignored. didInitialFocus is set to true unconditionally and the coroutine returns immediately, so if the request is canceled (returns false) the screen will never retry and may enter with no focused element, breaking controller/D-pad navigation.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotGalleryScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/screenshots/ScreenshotGalleryScreen.kt:137">
P2: FocusRequester.requestFocus() returns Boolean indicating success or cancellation, but the return value is ignored. didInitialFocus is set to true unconditionally and the coroutine returns immediately, so if the request is canceled (returns false) the screen will never retry and may enter with no focused element, breaking controller/D-pad navigation.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
VinceBT
force-pushed
the
feature/screenshots
branch
from
43e38ef to
c6c1354
Compare
VinceBT
added a commit
to VinceBT/GameNative
that referenced
this pull request
Jul 2, 2026
P1: - Sanitize appId in ScreenshotStore.gameDir to prevent path traversal - Guard rememberScreenshots listing against I/O/SecurityException crashes - Make LibraryDetailPane.onNavigateRoute required and wire it through the Downloads screen so screenshot taps work there P2: - Scope screenshot combo key tracking by deviceId so keys from different controllers can't falsely combine; refresh the combo on each fresh press so a mid-session rebind takes effect - Convert screenshots_deleted_all to a <plurals> resource across all locales (one/few/other as the language requires) and use getQuantityString
VinceBT
added a commit
to VinceBT/GameNative
that referenced
this pull request
Jul 2, 2026
…ed_all Provide the full Slavic one/few/many/other plural forms instead of an other-only fallback, so deletion counts are grammatically correct (cubic review on PR utkarshdalal#1627).
VinceBT
added a commit
to VinceBT/GameNative
that referenced
this pull request
Jul 2, 2026
showKeyboardPressed has the same lifetime as the screenshot combo state: if the keyboard binding is held when a menu/editor steals focus, its ACTION_UP is routed away and the latch stays true, so the next SHOW_KEYBOARD press is ignored. Reset it alongside the combo state. Addresses PR utkarshdalal#1627 review feedback.
VinceBT
force-pushed
the
feature/screenshots
branch
from
0d276fa to
ca87d0b
Compare
VinceBT
force-pushed
the
feature/screenshots
branch
from
ca87d0b to
cdaa1fb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds in-app screenshots. While a game is running you can capture the current frame (bind a controller button for it, or trigger it from the quick menu). Capture goes through PixelCopy on the game surface, so overlays and the capture toast don't end up in the shot.
Each game gets a gallery to browse its shots, plus a fullscreen viewer with pinch zoom, pan, and full controller navigation (bumpers to flip between shots, sticks/buttons to zoom and pan). From the viewer you can open the shot in an external gallery app, save it to Downloads, share it, or delete it. The game detail page shows a small preview strip that links into the gallery.
Where they're stored is configurable in Settings: keep them in app storage, or point it at a folder you pick. Strings are localized across the existing locales.
Recording
Will add a short clip (capture → gallery → viewer/share) shortly — wanted to get the PR up first.
Type of Change
Flagging honestly: this is a new feature, so it's outside the default scope. Happy to take it to Discord first or drop it if it's not something you want in core.
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds in‑game screenshot capture with a per‑game gallery and a full‑screen viewer. Capture uses PixelCopy; you can choose storage, bind a controller shortcut, tap the “saved” toast to open the viewer, and open the gallery as an in‑game overlay without losing input.
New Features
PluviaScreen.ScreenshotGallery(optional index to jump to a shot); returns to the game when closed.ScreenshotManager+ScreenshotStore(unit‑tested).Bug Fixes
appId; case‑insensitive PNG handling; atomic filename claims; prune share cache; handle bitmap OOM and always recycle; on Android Q+ delete failed MediaStore rows; on pre‑Q request WRITE_EXTERNAL_STORAGE and resume export.deviceId; refresh binding on new presses; clear held keys (incl. SHOW_KEYBOARD) when menus/overlays open or on pause; preserve library selection when returning from sub‑screens; deep‑linked opens navigate correctly.InfoCardlabel when the chevron is shown; rebind renderer view on screen changes to keep input/rendering stable.Written for commit cdaa1fb. Summary will update on new commits.
Summary by CodeRabbit