fix: address fourth round of PR review findings

TideGear · TideGear · commit 5874237dcf8f · 2026-05-02T23:03:12.000-07:00
PluviaMain SDK-cloud-bridge dialog
- relaunch() now takes a suppressPrompt flag; only set skipBridgePrompt=true
  when persistence actually succeeded. Previous behavior unconditionally
  suppressed the prompt for the next launch even when container.saveData()
  threw and the subdir wasn't actually saved — bridge ended up "off but
  prompt hidden" instead of either fully on or normal.
- Tightened the prompt-fire gate to also skip when preferredSave != None
  or ignorePendingOperations is true. Those flags come from conflict-
  resolution dialogs; firing the bridge prompt during a conflict re-entry
  would drop the user's chosen save / dismiss-pending choice via the
  relaunch lambda.

SteamUtils.writeSharedAppManifest
- Added LastOwner to the early-return reuse criteria. Previously,
  depots/buildId/scripts could all match while the existing acf was
  attributed to a different signed-in account; reusing it would leave
  the manifest pointing at a stale LastOwner and break cloud/ownership
  checks for the current user. Force-rewrite when the owner doesn't
  match. New helper parseAcfLastOwner() and acfLastOwnerRegex.

SteamUtils.autoLoginUserChanges (loginusers.vdf)
- Ensure the Steam config dir exists before writeText(). On a fresh
  container the Wine prefix skeleton is laid down but Steam's own
  drive_c/Program Files (x86)/Steam/config/ subdir hasn't been
  populated yet — without mkdirs() the write throws
  FileNotFoundException and the auto-login fails silently.

ContainerUtils.createNewContainer
- Replace the open-coded ContainerData(...) construction in the default
  branch with getDefaultContainerData().copy(drives = drives,
  dxwrapper = initialDxWrapper). The manual builder was missing several
  fields that getDefaultContainerData() carries — localSavesOnly,
  useSteamInput, sharpnessEffect/Level/Denoise, vibrationMode/Intensity,
  lsfgEnabled — so new containers inherited Kotlin defaults instead of
  the user's PrefManager defaults. Inheriting through copy() also means
  future fields don't need to be added in two places.

Findings flagged but not changed:
- verifyRestoredState true on no DLLs: the function is meant to
  detect "DLLs are NOT the pipe-DLL" (i.e., they're the original
  Valve DLLs or absent). Returning false on absent would force
  re-extraction of pipe DLLs into game folders that don't ship them,
  which is wrong for non-Steamworks games.
- SDK cloud mirror non-recursive (4th time): by design for Dead Cells
  flat saves; recursive expansion is scope creep.
- matchesGlob unit-test nitpick: skipping; no test infra changes.
diff --git a/app/src/main/java/app/gamenative/ui/PluviaMain.kt b/app/src/main/java/app/gamenative/ui/PluviaMain.kt
@@ -792,11 +792,15 @@ fun PluviaMain(
         }
 
         DialogType.SDK_CLOUD_BRIDGE_SUGGESTION -> {
-            val relaunch = {
+            val relaunch = { suppressPrompt: Boolean ->
                 preLaunchApp(
                     context = context,
                     appId = state.launchedAppId,
-                    skipBridgePrompt = true,
+                    // Only suppress the prompt next time when we actually persisted a subdir
+                    // (or the user picked "Don't ask again"). If persistence failed, leave
+                    // the prompt enabled so it fires again on the next attempt rather than
+                    // silently disabling itself in memory.
+                    skipBridgePrompt = suppressPrompt,
                     setLoadingDialogVisible = viewModel::setLoadingDialogVisible,
                     setLoadingProgress = viewModel::setLoadingDialogProgress,
                     setLoadingMessage = viewModel::setLoadingDialogMessage,
@@ -813,36 +817,37 @@ fun PluviaMain(
                     val rec = runCatching {
                         SteamUtils.getRecommendedSdkCloudSaveSubdirAsync(context, gameId)
                     }.getOrNull()
-                    if (rec != null) {
-                        runCatching {
-                            val container = ContainerUtils.getContainer(context, state.launchedAppId)
-                            container.sdkCloudSaveSubdir = rec.subdir
-                            container.saveData()
-                        }.onFailure { Timber.w(it, "Failed to persist sdkCloudSaveSubdir=${rec.subdir}") }
-                    }
-                    withContext(Dispatchers.Main) { relaunch() }
+                    val persisted = rec != null && runCatching {
+                        val container = ContainerUtils.getContainer(context, state.launchedAppId)
+                        container.sdkCloudSaveSubdir = rec.subdir
+                        container.saveData()
+                    }.onFailure { Timber.w(it, "Failed to persist sdkCloudSaveSubdir=${rec.subdir}") }
+                        .isSuccess
+                    withContext(Dispatchers.Main) { relaunch(persisted) }
                 }
             }
             onDismissClick = {
-                // Skip this time — continue launch without setting the field.
+                // Skip this time — continue launch without setting the field. Don't suppress
+                // the prompt on the next launch since we didn't persist anything.
                 msgDialogState = MessageDialogState(false)
-                relaunch()
+                relaunch(false)
             }
             onActionClick = {
                 // Don't ask again for this game.
                 msgDialogState = MessageDialogState(false)
                 CoroutineScope(Dispatchers.IO).launch {
-                    runCatching {
+                    val persisted = runCatching {
                         val container = ContainerUtils.getContainer(context, state.launchedAppId)
                         container.putExtra("sdkCloudBridgePromptDismissed", "1")
                         container.saveData()
                     }.onFailure { Timber.w(it, "Failed to persist sdkCloudBridgePromptDismissed") }
-                    withContext(Dispatchers.Main) { relaunch() }
+                        .isSuccess
+                    withContext(Dispatchers.Main) { relaunch(persisted) }
                 }
             }
             onDismissRequest = {
                 msgDialogState = MessageDialogState(false)
-                relaunch()
+                relaunch(false)
             }
         }
 
@@ -1639,12 +1644,16 @@ fun preLaunchApp(
         // That intersection is a reliable Pattern B signal and keeps false positives low.
         //
         // Skip the prompt for non-game launches (Open Container / temporary override / explicit
-        // skipCloudSync). The post-prompt `relaunch` lambda doesn't carry those flags forward,
-        // so firing the prompt for them would silently turn an Open-Container into a game launch.
+        // skipCloudSync) and for re-entries from conflict resolution (preferredSave set, or
+        // ignorePendingOperations true). The post-prompt `relaunch` lambda doesn't carry those
+        // flags forward, so firing the prompt for them would silently drop the user's
+        // conflict-resolution choice.
         if (!skipBridgePrompt &&
             !bootToContainer &&
             !useTemporaryOverride &&
             !skipCloudSync &&
+            !ignorePendingOperations &&
+            preferredSave == SaveLocation.None &&
             gameSource == GameSource.STEAM &&
             container.isLaunchRealSteam &&
             container.sdkCloudSaveSubdir.isBlank() &&
diff --git a/app/src/main/java/app/gamenative/utils/ContainerUtils.kt b/app/src/main/java/app/gamenative/utils/ContainerUtils.kt
@@ -827,62 +827,15 @@ object ContainerUtils {
                 customConfig
             }
         } else {
-            // Use default config with drives
-            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,
+            // Build the default config from PrefManager and override only what this
+            // create-site needs to vary (drives + the per-game initial dxwrapper). Inheriting
+            // from getDefaultContainerData() means new containers automatically pick up
+            // any new fields added there (localSavesOnly, useSteamInput, sharpnessEffect/Level/
+            // Denoise, vibrationMode/Intensity, lsfgEnabled, ...) without this branch needing
+            // to be edited every time.
+            getDefaultContainerData().copy(
                 drives = drives,
-                execArgs = PrefManager.execArgs,
-                showFPS = false,
-                launchRealSteam = PrefManager.launchRealSteam,
-                disableSteamOverlay = PrefManager.disableSteamOverlay,
-                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,
-                suspendPolicy = PrefManager.suspendPolicy,
-                portraitMode = PrefManager.portraitMode,
-                externalDisplayMode = PrefManager.externalDisplayInputMode,
-                externalDisplaySwap = PrefManager.externalDisplaySwap,
-                vibrationMode = PrefManager.vibrationMode,
-                vibrationIntensity = PrefManager.vibrationIntensity,
+                dxwrapper = initialDxWrapper,
             )
         }
 
diff --git a/app/src/main/java/app/gamenative/utils/SteamUtils.kt b/app/src/main/java/app/gamenative/utils/SteamUtils.kt
@@ -596,6 +596,12 @@ object SteamUtils {
             ?: File(imageFs.wineprefix)
         val steamConfigDir = File(winePrefixBase, "drive_c/Program Files (x86)/Steam/config")
         try {
+            // Ensure the Steam config dir exists. On a fresh container the Wine prefix
+            // skeleton is in place but Steam's own subdir hasn't been laid down yet —
+            // writeText() would fail with FileNotFoundException without this.
+            if (!steamConfigDir.isDirectory && !steamConfigDir.mkdirs()) {
+                Timber.w("autoLoginUserChanges: failed to create $steamConfigDir; loginusers.vdf write may fail")
+            }
             File(steamConfigDir, "loginusers.vdf").writeText(vdfFileText)
             val userRegFile = File(winePrefixBase, "user.reg")
             val steamRoot = "C:\\Program Files (x86)\\Steam"
@@ -1108,10 +1114,15 @@ object SteamUtils {
             val existingBuildId = parseAcfBuildId(staleAcf)
             val existingDepots = parseAcfInstalledDepotIds(staleAcf)
             val existingScripts = parseAcfInstallScriptDepotIds(staleAcf)
+            val existingOwner = parseAcfLastOwner(staleAcf)
             val depotsMatch = existingDepots == presentDepotIds
             val buildIdMatch = existingBuildId == sharedBuildId
             val scriptsMatch = existingScripts == presentScriptDepotIds
-            if (depotsMatch && buildIdMatch && scriptsMatch) {
+            // LastOwner mismatch means the manifest was last written by a different
+            // signed-in account; reusing it would attribute the install to that account
+            // and break cloud/ownership checks for the current user. Force a rewrite.
+            val lastOwnerMatch = existingOwner == lastOwner
+            if (depotsMatch && buildIdMatch && scriptsMatch && lastOwnerMatch) {
                 val updateResult = parseAcfUpdateResult(staleAcf)
                 if (updateResult == 0L) return
                 staleAcf.delete()
@@ -1217,6 +1228,7 @@ object SteamUtils {
 
     private val acfBuildIdRegex = Regex("\"buildid\"\\s*\"(\\d+)\"")
     private val acfUpdateResultRegex = Regex("\"UpdateResult\"\\s*\"(\\d+)\"")
+    private val acfLastOwnerRegex = Regex("\"LastOwner\"\\s*\"(\\d+)\"")
 
     // Matches `"<depotId>" { "manifest" "<gid>" ... }` entries inside the
     // InstalledDepots block of our own acf output. Tolerant of whitespace /
@@ -1244,6 +1256,14 @@ object SteamUtils {
         }
     }
 
+    private fun parseAcfLastOwner(acf: File): String {
+        return try {
+            acfLastOwnerRegex.find(acf.readText())?.groupValues?.get(1).orEmpty()
+        } catch (_: Exception) {
+            ""
+        }
+    }
+
     private fun parseAcfInstalledDepotIds(acf: File): Set<Int> {
         return try {
             val text = acf.readText()
