Conversation
…creen even in landscape - moved portrait mode to container-level instead of at the app level allowed orientations. removed allowed orientations completely.
📝 WalkthroughWalkthroughThis change introduces portrait mode as a persistent container setting across the GameNative application. The feature adds preference management, UI controls in settings, persists the setting through container data serialization, updates orientation detection in XServerScreen, and provides multilingual string resources across 13 languages. Changes
Sequence DiagramsequenceDiagram
participant User
participant GeneralTab
participant PrefManager
participant Container
participant XServerScreen
participant View
User->>GeneralTab: Toggle Portrait Mode Switch
GeneralTab->>PrefManager: portraitMode = value
PrefManager->>Container: setPortraitMode(value)
Container->>Container: saveData() persists portraitMode
Note over XServerScreen: App Initialization
XServerScreen->>Container: isPortraitMode()
Container-->>XServerScreen: boolean portraitMode
XServerScreen->>XServerScreen: SetAllowedOrientation(EnumSet)
XServerScreen->>View: AndroidView factory decides layout
View->>View: Configure controls based on isPortrait
User->>View: Interact with game
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
2 issues found across 23 files
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/com/winlator/widget/InputControlsView.java">
<violation number="1" location="app/src/main/java/com/winlator/widget/InputControlsView.java:122">
P2: Reloading elements only on width change can leave control positions stale when only height changes. Include height changes in the resize check so elements recompute Y coordinates.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/model/MainViewModel.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/model/MainViewModel.kt:439">
P2: Hardcoding LANDSCAPE here ignores the user-selected allowed orientations (including portrait or reverse landscape), so portrait mode fixes won’t take effect during launch. Use the stored preference to respect user settings.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @Override | ||
| protected void onSizeChanged(int w, int h, int oldw, int oldh) { | ||
| super.onSizeChanged(w, h, oldw, oldh); | ||
| if (profile != null && profile.isElementsLoaded() && oldw > 0 && w != oldw) { |
There was a problem hiding this comment.
P2: Reloading elements only on width change can leave control positions stale when only height changes. Include height changes in the resize check so elements recompute Y coordinates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/widget/InputControlsView.java, line 122:
<comment>Reloading elements only on width change can leave control positions stale when only height changes. Include height changes in the resize check so elements recompute Y coordinates.</comment>
<file context>
@@ -116,6 +116,14 @@ public int getSnappingSize() {
+ @Override
+ protected void onSizeChanged(int w, int h, int oldw, int oldh) {
+ super.onSizeChanged(w, h, oldw, oldh);
+ if (profile != null && profile.isElementsLoaded() && oldw > 0 && w != oldw) {
+ profile.loadElements(this);
+ }
</file context>
| if (profile != null && profile.isElementsLoaded() && oldw > 0 && w != oldw) { | |
| if (profile != null && profile.isElementsLoaded() && oldw > 0 && (w != oldw || h != oldh)) { |
| viewModelScope.launch { | ||
| setShowBootingSplash(true) | ||
| PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(PrefManager.allowedOrientation)) | ||
| PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(EnumSet.of(Orientation.LANDSCAPE))) |
There was a problem hiding this comment.
P2: Hardcoding LANDSCAPE here ignores the user-selected allowed orientations (including portrait or reverse landscape), so portrait mode fixes won’t take effect during launch. Use the stored preference to respect user settings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/model/MainViewModel.kt, line 439:
<comment>Hardcoding LANDSCAPE here ignores the user-selected allowed orientations (including portrait or reverse landscape), so portrait mode fixes won’t take effect during launch. Use the stored preference to respect user settings.</comment>
<file context>
@@ -434,7 +436,7 @@ class MainViewModel @Inject constructor(
viewModelScope.launch {
setShowBootingSplash(true)
- PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(PrefManager.allowedOrientation))
+ PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(EnumSet.of(Orientation.LANDSCAPE)))
val apiJob = viewModelScope.async(Dispatchers.IO) {
</file context>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupEmulation.kt (1)
75-121:⚠️ Potential issue | 🟠 MajorThis removes the only path to reverse-orientation support.
The old
OrientationDialogexposed all fourOrientationvalues, but the new flow only distinguishes portrait vs. landscape andXServerScreennow emits a singlePORTRAITorLANDSCAPEenum. After this removal, users can no longer allow reverse landscape/reverse portrait anywhere, which is a behavior regression for devices/controllers used in the opposite rotation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupEmulation.kt` around lines 75 - 121, The change removed reverse-orientation support by collapsing the Orientation options to only PORTRAIT/LANDSCAPE; restore full four-value orientation handling by updating the flow so OrientationDialog (and any callers) still expose all four Orientation values (PORTRAIT, REVERSE_PORTRAIT, LANDSCAPE, REVERSE_LANDSCAPE) and by changing XServerScreen to emit the full Orientation enum rather than a two-value portrait/landscape mapping; find usages that now assume only PORTRAIT/LANDSCAPE (e.g., where the dialog is created or where showConfigDialog/state is consumed) and adjust them to accept and persist the four orientations, ensuring UI bindings and PrefManager/read-write paths handle the four enum values.app/src/main/java/app/gamenative/ui/model/MainViewModel.kt (1)
437-443:⚠️ Potential issue | 🟠 MajorDon't force landscape before the container is loaded.
SetAllowedOrientation(EnumSet.of(Orientation.LANDSCAPE))runs beforegetOrCreateContainer()resolves, so portrait-enabled containers are locked to landscape first. MainActivity applies that event immediately, and XServerScreen only corrects it later, which makes the startup orientation race-prone and breaks the new portrait-mode flow.🛠️ Suggested fix
viewModelScope.launch { setShowBootingSplash(true) - PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(EnumSet.of(Orientation.LANDSCAPE))) + val container = viewModelScope.async(Dispatchers.IO) { + ContainerUtils.getOrCreateContainer(context, appId) + }.await() + PluviaApp.events.emit( + AndroidEvent.SetAllowedOrientation( + if (container.isPortraitMode) EnumSet.of(Orientation.PORTRAIT) + else EnumSet.of(Orientation.LANDSCAPE), + ), + ) val apiJob = viewModelScope.async(Dispatchers.IO) { - val container = ContainerUtils.getOrCreateContainer(context, appId) val gameSource = ContainerUtils.extractGameSourceFromContainerId(appId) if (gameSource == GameSource.STEAM) { if (container.isLaunchRealSteam()) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/model/MainViewModel.kt` around lines 437 - 443, The startup code in MainViewModel launches a coroutine that emits AndroidEvent.SetAllowedOrientation(LANDSCAPE) before awaiting ContainerUtils.getOrCreateContainer(...), which forces landscape for portrait-enabled containers; move the orientation emission to after the container is loaded and the container/gameSource is extracted (i.e., after the await of the async job or inside the async after ContainerUtils.getOrCreateContainer and ContainerUtils.extractGameSourceFromContainerId) so MainActivity/XServerScreen can apply the correct orientation based on the container; update references in MainViewModel (the viewModelScope.launch block that calls setShowBootingSplash and emits the event, and the apiJob that calls ContainerUtils.getOrCreateContainer and extractGameSourceFromContainerId) so the orientation event is only emitted once the container info is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 260-265: The current emit of
PluviaApp.events.SetAllowedOrientation overwrites reverse orientations and
collapses multi-value preferences by always sending only Orientation.PORTRAIT or
Orientation.LANDSCAPE; update the logic in the XServerScreen block that calls
PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(...)) to preserve
reverse directions and any previously-selected multiple orientations by
computing the allowed EnumSet from container.isPortraitMode and the existing
orientation preference (or by merging reverse variants when portrait -> include
PORTRAIT and REVERSE_PORTRAIT; when landscape -> include LANDSCAPE and
REVERSE_LANDSCAPE), then pass that EnumSet into SetAllowedOrientation so reverse
orientations and multi-value selections remain supported.
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`:
- Line 471: createNewContainer() currently constructs a ContainerData without
setting portraitMode, so container.setPortraitMode(containerData.portraitMode)
writes the default false; update createNewContainer() to populate the new
ContainerData.portraitMode from the shared preference (PrefManager.portraitMode)
or by using getDefaultContainerData() as the source of defaults so the correct
portrait mode is persisted when creating fresh containers.
In `@app/src/main/java/com/winlator/widget/InputControlsView.java`:
- Around line 119-124: onSizeChanged currently calls profile.loadElements(this)
which clears and reloads persisted elements (via ControlsProfile.loadElements)
causing unsaved edit-mode changes and dangling selectedElement after a resize;
change the logic in onSizeChanged to skip calling profile.loadElements when the
view is in edit mode (or when unsaved changes exist) and instead
rescale/transform the existing in-memory element list to the new dimensions, and
also trigger this path on any size change (width or height) rather than only
when w != oldw; locate and update the onSizeChanged override and any flags or
methods that indicate edit mode/unsaved changes (e.g., profile.isElementsLoaded,
selectedElement) to avoid reloading from disk during editing while still
allowing reloads when appropriate (e.g., not editing or when explicitly
requested).
In `@app/src/main/res/values-es/strings.xml`:
- Around line 569-570: Update the portrait_mode_description string to clearly
indicate vertical/portrait controllers: replace the current phrasing in the
string resource named "portrait_mode_description" so it reads something explicit
like "Para mandos verticales como 8BitDo FlipPad o GameSir Pocket Taco"
(reference the "portrait_mode_description" resource and keep "portrait_mode"
unchanged); ensure the new Spanish wording is grammatically correct and matches
app style.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/ui/model/MainViewModel.kt`:
- Around line 437-443: The startup code in MainViewModel launches a coroutine
that emits AndroidEvent.SetAllowedOrientation(LANDSCAPE) before awaiting
ContainerUtils.getOrCreateContainer(...), which forces landscape for
portrait-enabled containers; move the orientation emission to after the
container is loaded and the container/gameSource is extracted (i.e., after the
await of the async job or inside the async after
ContainerUtils.getOrCreateContainer and
ContainerUtils.extractGameSourceFromContainerId) so MainActivity/XServerScreen
can apply the correct orientation based on the container; update references in
MainViewModel (the viewModelScope.launch block that calls setShowBootingSplash
and emits the event, and the apiJob that calls
ContainerUtils.getOrCreateContainer and extractGameSourceFromContainerId) so the
orientation event is only emitted once the container info is available.
In
`@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupEmulation.kt`:
- Around line 75-121: The change removed reverse-orientation support by
collapsing the Orientation options to only PORTRAIT/LANDSCAPE; restore full
four-value orientation handling by updating the flow so OrientationDialog (and
any callers) still expose all four Orientation values (PORTRAIT,
REVERSE_PORTRAIT, LANDSCAPE, REVERSE_LANDSCAPE) and by changing XServerScreen to
emit the full Orientation enum rather than a two-value portrait/landscape
mapping; find usages that now assume only PORTRAIT/LANDSCAPE (e.g., where the
dialog is created or where showConfigDialog/state is consumed) and adjust them
to accept and persist the four orientations, ensuring UI bindings and
PrefManager/read-write paths handle the four enum values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e05e43f-63c1-45d2-8542-a681036a8ec3
📒 Files selected for processing (23)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.ktapp/src/main/java/app/gamenative/ui/model/MainViewModel.ktapp/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupEmulation.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/java/com/winlator/container/Container.javaapp/src/main/java/com/winlator/container/ContainerData.ktapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/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-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-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
| PluviaApp.events.emit( | ||
| AndroidEvent.SetAllowedOrientation( | ||
| if (container.isPortraitMode) EnumSet.of(Orientation.PORTRAIT) | ||
| else EnumSet.of(Orientation.LANDSCAPE) | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Preserve the reverse orientations here.
This now locks the activity to a single enum value, so REVERSE_PORTRAIT / REVERSE_LANDSCAPE can no longer be selected even though Orientation and SetAllowedOrientation are designed to carry them. It also bypasses the existing multi-value orientation preference, so users who previously allowed both landscape directions get a stricter lock than before.
🔧 Minimal fix
PluviaApp.events.emit(
AndroidEvent.SetAllowedOrientation(
- if (container.isPortraitMode) EnumSet.of(Orientation.PORTRAIT)
- else EnumSet.of(Orientation.LANDSCAPE)
+ if (container.isPortraitMode) {
+ EnumSet.of(Orientation.PORTRAIT, Orientation.REVERSE_PORTRAIT)
+ } else {
+ EnumSet.of(Orientation.LANDSCAPE, Orientation.REVERSE_LANDSCAPE)
+ }
),
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around
lines 260 - 265, The current emit of PluviaApp.events.SetAllowedOrientation
overwrites reverse orientations and collapses multi-value preferences by always
sending only Orientation.PORTRAIT or Orientation.LANDSCAPE; update the logic in
the XServerScreen block that calls
PluviaApp.events.emit(AndroidEvent.SetAllowedOrientation(...)) to preserve
reverse directions and any previously-selected multiple orientations by
computing the allowed EnumSet from container.isPortraitMode and the existing
orientation preference (or by merging reverse variants when portrait -> include
PORTRAIT and REVERSE_PORTRAIT; when landscape -> include LANDSCAPE and
REVERSE_LANDSCAPE), then pass that EnumSet into SetAllowedOrientation so reverse
orientations and multi-value selections remain supported.
| container.setSteamOfflineMode(containerData.steamOfflineMode) | ||
| container.setUseLegacyDRM(containerData.useLegacyDRM) | ||
| container.setUnpackFiles(containerData.unpackFiles) | ||
| container.setPortraitMode(containerData.portraitMode) |
There was a problem hiding this comment.
Fresh containers still reset this back to false.
createNewContainer() still builds ContainerData manually at Lines 791-841 and never sets portraitMode, so this line writes the default false into every newly created container on first apply. The new PrefManager.portraitMode plumbing only helps paths that go through getDefaultContainerData().
🔧 Minimal fix outside this hunk
ContainerData(
screenSize = PrefManager.screenSize,
envVars = PrefManager.envVars,
cpuList = PrefManager.cpuList,
cpuListWoW64 = PrefManager.cpuListWoW64,
graphicsDriver = PrefManager.graphicsDriver,
graphicsDriverVersion = PrefManager.graphicsDriverVersion,
graphicsDriverConfig = PrefManager.graphicsDriverConfig,
dxwrapper = initialDxWrapper,
dxwrapperConfig = PrefManager.dxWrapperConfig,
audioDriver = PrefManager.audioDriver,
wincomponents = PrefManager.winComponents,
drives = drives,
execArgs = PrefManager.execArgs,
showFPS = PrefManager.showFps,
launchRealSteam = PrefManager.launchRealSteam,
wow64Mode = PrefManager.wow64Mode,
startupSelection = PrefManager.startupSelection.toByte(),
box86Version = PrefManager.box86Version,
box64Version = PrefManager.box64Version,
box86Preset = PrefManager.box86Preset,
box64Preset = PrefManager.box64Preset,
desktopTheme = WineThemeManager.DEFAULT_DESKTOP_THEME,
language = PrefManager.containerLanguage,
containerVariant = PrefManager.containerVariant,
wineVersion = PrefManager.wineVersion,
emulator = PrefManager.emulator,
fexcoreVersion = PrefManager.fexcoreVersion,
fexcoreTSOMode = PrefManager.fexcoreTSOMode,
fexcoreX87Mode = PrefManager.fexcoreX87Mode,
fexcoreMultiBlock = PrefManager.fexcoreMultiBlock,
fexcorePreset = PrefManager.fexcorePreset,
renderer = PrefManager.renderer,
csmt = PrefManager.csmt,
videoPciDeviceID = PrefManager.videoPciDeviceID,
offScreenRenderingMode = PrefManager.offScreenRenderingMode,
strictShaderMath = PrefManager.strictShaderMath,
useDRI3 = PrefManager.useDRI3,
videoMemorySize = PrefManager.videoMemorySize,
mouseWarpOverride = PrefManager.mouseWarpOverride,
enableXInput = PrefManager.xinputEnabled,
enableDInput = PrefManager.dinputEnabled,
dinputMapperType = PrefManager.dinputMapperType.toByte(),
disableMouseInput = PrefManager.disableMouseInput,
forceDlc = PrefManager.forceDlc,
steamOfflineMode = PrefManager.steamOfflineMode,
useLegacyDRM = PrefManager.useLegacyDRM,
unpackFiles = PrefManager.unpackFiles,
+ portraitMode = PrefManager.portraitMode,
externalDisplayMode = PrefManager.externalDisplayInputMode,
externalDisplaySwap = PrefManager.externalDisplaySwap,
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt` at line 471,
createNewContainer() currently constructs a ContainerData without setting
portraitMode, so container.setPortraitMode(containerData.portraitMode) writes
the default false; update createNewContainer() to populate the new
ContainerData.portraitMode from the shared preference (PrefManager.portraitMode)
or by using getDefaultContainerData() as the source of defaults so the correct
portrait mode is persisted when creating fresh containers.
| @Override | ||
| protected void onSizeChanged(int w, int h, int oldw, int oldh) { | ||
| super.onSizeChanged(w, h, oldw, oldh); | ||
| if (profile != null && profile.isElementsLoaded() && oldw > 0 && w != oldw) { | ||
| profile.loadElements(this); | ||
| } |
There was a problem hiding this comment.
Avoid reloading the persisted profile on resize.
ControlsProfile.loadElements() clears the current element list before rebuilding it from disk, so Lines 122-123 will wipe unsaved edit-mode changes and leave selectedElement pointing at detached objects after a resize. If this is only meant to fix rotation, skip this path while editing and rescale the in-memory elements instead. The current w != oldw check also misses height-only size changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/winlator/widget/InputControlsView.java` around lines
119 - 124, onSizeChanged currently calls profile.loadElements(this) which clears
and reloads persisted elements (via ControlsProfile.loadElements) causing
unsaved edit-mode changes and dangling selectedElement after a resize; change
the logic in onSizeChanged to skip calling profile.loadElements when the view is
in edit mode (or when unsaved changes exist) and instead rescale/transform the
existing in-memory element list to the new dimensions, and also trigger this
path on any size change (width or height) rather than only when w != oldw;
locate and update the onSizeChanged override and any flags or methods that
indicate edit mode/unsaved changes (e.g., profile.isElementsLoaded,
selectedElement) to avoid reloading from disk during editing while still
allowing reloads when appropriate (e.g., not editing or when explicitly
requested).
| <string name="portrait_mode">Modo vertical</string> | ||
| <string name="portrait_mode_description">Para controles tipo 8BitDo FlipPad/GameSir Pocket Taco</string> |
There was a problem hiding this comment.
Make the helper text explicit about vertical controllers.
controles tipo ... reads awkwardly in Spanish, and it doesn’t clearly explain the use case. Both referenced devices are portrait-style mobile controllers, so wording this as Para mandos verticales como 8BitDo FlipPad o GameSir Pocket Taco would be clearer in settings. (gamesir.com)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/values-es/strings.xml` around lines 569 - 570, Update the
portrait_mode_description string to clearly indicate vertical/portrait
controllers: replace the current phrasing in the string resource named
"portrait_mode_description" so it reads something explicit like "Para mandos
verticales como 8BitDo FlipPad o GameSir Pocket Taco" (reference the
"portrait_mode_description" resource and keep "portrait_mode" unchanged); ensure
the new Spanish wording is grammatically correct and matches app style.
Summary by cubic
Adds a per-container Portrait Mode and fixes incorrect control layout when running in landscape. Orientation is now applied per container, removing the global orientation setting and preventing controls from covering half the screen.
New Features
Bug Fixes
Written for commit 4e45528. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements