feat: add default screen size detection and 960p screen size option (e.g. retroid pocket nova) - #1730
feat: add default screen size detection and 960p screen size option (e.g. retroid pocket nova)#1730joshuatam wants to merge 7 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughScreen-size configuration now defines explicit 16:9, 16:10, and 4:3 presets, detects the device aspect ratio at runtime, and uses the detected preset across preference, state, container-data, and intent defaults. A new 1280x960 (4:3) option is available. ChangesScreen Size Defaults
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PluviaApp
participant DisplayManager
participant Display
participant PrefManager
participant IntentLaunchManager
PluviaApp->>DisplayManager: Obtain the default display
DisplayManager-->>PluviaApp: Return the display
PluviaApp->>Display: Read display dimensions
Display-->>PluviaApp: Return width and height
PluviaApp->>PluviaApp: Select aspect-specific preset
PrefManager->>PluviaApp: Request default screen size
IntentLaunchManager->>PluviaApp: Request fallback screen size
PluviaApp-->>PrefManager: Return selected preset
PluviaApp-->>IntentLaunchManager: Return selected preset
Possibly related PRs
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2fa0467-e858-4c6e-afdd-896fd7746b82
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/data/XServerState.ktapp/src/main/java/app/gamenative/utils/IntentLaunchManager.ktapp/src/main/java/com/winlator/container/Container.javaapp/src/main/java/com/winlator/container/ContainerData.ktapp/src/main/java/com/winlator/container/ContainerManager.java
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/PluviaApp.kt (1)
288-288: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
getRealMetrics()instead ofgetMetrics().
display.getMetrics()returns logical dimensions that exclude system decorations like the navigation bar. Subtracting the navigation bar can alter the calculated aspect ratio enough to misidentify the device's true screen size (e.g., miscategorizing a 16:10 device as 16:9). UsegetRealMetrics()to get the physical dimensions.🐛 Proposed fix
val displayMetrics = DisplayMetrics() `@Suppress`("DEPRECATION") - display.getMetrics(displayMetrics) + display.getRealMetrics(displayMetrics)🤖 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/PluviaApp.kt` at line 288, In the display metrics setup around display.getMetrics(), use getRealMetrics() so displayMetrics contains the physical screen dimensions including system decorations. Leave the surrounding aspect-ratio and device-size calculation logic unchanged.
🤖 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/PluviaApp.kt`:
- Line 274: Qualify the DISPLAY_SERVICE reference in the companion object’s
displayManager initialization with android.content.Context.DISPLAY_SERVICE so
the getSystemService call compiles without relying on an out-of-scope symbol.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/PluviaApp.kt`:
- Line 288: In the display metrics setup around display.getMetrics(), use
getRealMetrics() so displayMetrics contains the physical screen dimensions
including system decorations. Leave the surrounding aspect-ratio and device-size
calculation logic unchanged.
🪄 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: ba09c406-9d1d-4165-8d7d-f5797f39757d
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/PluviaApp.kt
There was a problem hiding this comment.
1 issue found across 2 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/PluviaApp.kt">
<violation number="1" location="app/src/main/java/app/gamenative/PluviaApp.kt:291">
P2: Pre-Android-11 16:9 devices can be misclassified as 16:10 because `getMetrics()` excludes system decorations from the measured area. Using the real display metrics here preserves the device aspect ratio used by the default-size detection.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
afa5e0d to
fcae650
Compare
fcae650 to
425e8d1
Compare
Description
feat: add 960p default screen size (e.g. retroid pocket nova)
Recording
Type of Change
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
Add 1280x960 (4:3) and auto-detect the default screen size via
DisplayManagerwith agetRealMetricsfallback and caching. Apply the detected aspect ratio to defaults for preferences, new containers, XServer state, imports, and launches (4:3, 16:10, or 16:9; 16:9 fallback), and fix config merging so screen-size overrides are respected — improving support for 960p handhelds like the Retroid Pocket Nova.createDefaultContainerFutureinContainerManager.Written for commit 425e8d1. Summary will update on new commits.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improved Defaults