Skip to content

Commit 22a8818

Browse files
tmgasttmgast
committed
Migrate sync write call sites; bug fixes #1 and #4
RetroArchSaveHandler.constructSavePath: - Now delegates to resolver.buildSaveFilePath - Uses raw libretro core id (resolver applies getRetroArchSaveDirName once) - Save extension comes from SavePathConfig.saveExtensions, not hardcoded .srm - Honors sort_savefiles_enable=false (issue #185 root fix on the write path) - Honors savefiles_in_content_dir - Hidden bug fixed: getRetroArchSaveDirName divergence on cores with decorated library_name (e.g. Snes9x vs snes9x) SavePathResolver.constructRetroArchSavePath: - Hand-rolled '<base>/<core>/<file>' replaced with resolver.resolve SaveDirectories. Now respects sort_savefiles_by_content_enable and the /Internal/ mirror that was previously skipped on this code path. StateCacheManager.buildStateTargetPath: - Signature gains optional emulatorId/coreName/romPath. RA branch routes through resolver.resolveStateDirectories; non-RA falls back to existing StatePathRegistry path. RestoreStateUseCase threads the new params from its existing inputs. Handler no longer needs RetroArchConfigParser (resolver owns it). Tests updated to mock resolver instead of parser; previously-red regression tests now pass.
1 parent b4c618d commit 22a8818

5 files changed

Lines changed: 80 additions & 105 deletions

File tree

‎app/src/main/kotlin/com/nendo/argosy/data/repository/StateCacheManager.kt‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -451,13 +451,27 @@ class StateCacheManager @Inject constructor(
451451
Log.d(TAG, "Deleted all ${caches.size} cached states for game $gameId")
452452
}
453453

454-
fun buildStateTargetPath(
454+
suspend fun buildStateTargetPath(
455455
config: StatePathConfig,
456456
platformId: String,
457457
romBaseName: String,
458-
slotNumber: Int
458+
slotNumber: Int,
459+
emulatorId: String? = null,
460+
coreName: String? = null,
461+
romPath: String? = null,
459462
): String? {
460-
val paths = StatePathRegistry.resolvePath(config, platformId)
463+
val paths = if (emulatorId != null &&
464+
com.nendo.argosy.data.emulator.RetroArchPathResolver.isRetroArch(emulatorId)
465+
) {
466+
val req = com.nendo.argosy.data.emulator.RetroArchPathResolver.Request(
467+
emulatorId = emulatorId,
468+
coreName = coreName,
469+
romPath = romPath,
470+
)
471+
retroArchPathResolver.resolveStateDirectories(req)
472+
} else {
473+
StatePathRegistry.resolvePath(config, platformId)
474+
}
461475
val baseDir = paths.firstOrNull { File(it).exists() } ?: paths.firstOrNull() ?: return null
462476
val fileName = config.slotPattern.buildFileName(romBaseName, slotNumber)
463477
return "$baseDir/$fileName"

‎app/src/main/kotlin/com/nendo/argosy/data/sync/SavePathResolver.kt‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -520,41 +520,27 @@ class SavePathResolver @Inject constructor(
520520
return "$baseDir/$fileName"
521521
}
522522

523-
private fun constructRetroArchSavePath(
523+
private suspend fun constructRetroArchSavePath(
524524
emulatorId: String,
525525
gameTitle: String,
526526
platformSlug: String,
527527
romPath: String?
528528
): String? {
529-
val packageName = when (emulatorId) {
530-
"retroarch_64" -> "com.retroarch.aarch64"
531-
else -> "com.retroarch"
532-
}
533-
534-
val raConfig = retroArchConfigParser.parse(packageName)
529+
val raConfig = retroArchConfigParser.parse(
530+
com.nendo.argosy.data.emulator.RetroArchPathResolver.packageForEmulatorId(emulatorId)
531+
)
535532
val coreName = SavePathRegistry.getRetroArchCore(platformSlug, raConfig?.lastLoadedCore)
536533
?: return null
537-
val saveDirName = EmulatorRegistry.getRetroArchSaveDirName(coreName)
538534
val saveConfig = SavePathRegistry.getConfig(emulatorId) ?: return null
539535
val extension = saveConfig.saveExtensions.firstOrNull() ?: "srm"
540536

541-
val baseDir = when {
542-
raConfig?.savefilesInContentDir == true && romPath != null -> {
543-
File(romPath).parent
544-
}
545-
raConfig?.savefileDirectory != null -> {
546-
if (raConfig.sortByCore) {
547-
"${raConfig.savefileDirectory}/$saveDirName"
548-
} else {
549-
raConfig.savefileDirectory
550-
}
551-
}
552-
else -> {
553-
val defaultPaths = resolveSavePaths(saveConfig, platformSlug)
554-
defaultPaths.firstOrNull { directoryExists(it) }
555-
?: defaultPaths.firstOrNull()
556-
}
557-
} ?: return null
537+
val req = com.nendo.argosy.data.emulator.RetroArchPathResolver.Request(
538+
emulatorId = emulatorId,
539+
coreName = coreName,
540+
romPath = romPath,
541+
)
542+
val dirs = retroArchPathResolver.resolveSaveDirectories(req)
543+
val baseDir = dirs.firstOrNull { directoryExists(it) } ?: dirs.firstOrNull() ?: return null
558544

559545
val fileName = buildRetroArchFileName(gameTitle, romPath, extension)
560546
return "$baseDir/$fileName"

‎app/src/main/kotlin/com/nendo/argosy/data/sync/platform/RetroArchSaveHandler.kt‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.nendo.argosy.data.sync.platform
22

33
import android.content.Context
4-
import com.nendo.argosy.data.emulator.RetroArchConfigParser
54
import com.nendo.argosy.data.emulator.SavePathRegistry
65
import com.nendo.argosy.data.storage.FileAccessLayer
76
import com.nendo.argosy.util.Logger
@@ -16,7 +15,6 @@ import javax.inject.Singleton
1615
class RetroArchSaveHandler @Inject constructor(
1716
@ApplicationContext private val context: Context,
1817
private val fal: FileAccessLayer,
19-
private val retroArchConfigParser: RetroArchConfigParser,
2018
private val retroArchPathResolver: com.nendo.argosy.data.emulator.RetroArchPathResolver,
2119
) : PlatformSaveHandler {
2220
companion object {
@@ -91,30 +89,25 @@ class RetroArchSaveHandler @Inject constructor(
9189
return null
9290
}
9391

94-
fun constructSavePath(context: SaveContext): String? {
95-
val packageName = getPackageName(context.emulatorId)
92+
suspend fun constructSavePath(context: SaveContext): String? {
9693
val coreName = SavePathRegistry.getRetroArchCore(context.platformSlug)
9794
if (coreName == null) {
9895
Logger.debug(TAG, "constructSavePath: No core mapping for platform | platform=${context.platformSlug}")
9996
return null
10097
}
10198

102-
// Parse config to get actual save directory (respects user settings)
103-
val raConfig = retroArchConfigParser.parse(packageName)
104-
val saveDir = raConfig?.savefileDirectory
105-
?: "/storage/emulated/0/RetroArch/saves"
106-
99+
val req = com.nendo.argosy.data.emulator.RetroArchPathResolver.Request(
100+
emulatorId = context.emulatorId,
101+
coreName = coreName,
102+
romPath = context.romPath,
103+
)
107104
val baseName = buildSaveFileName(context)
108-
val savePath = "$saveDir/$coreName/$baseName.srm"
105+
val saveExtension = context.config.saveExtensions.firstOrNull() ?: "srm"
106+
val savePath = retroArchPathResolver.buildSaveFilePath(req, baseName, saveExtension)
109107
Logger.debug(TAG, "constructSavePath: Constructed | path=$savePath")
110108
return savePath
111109
}
112110

113-
private fun getPackageName(emulatorId: String): String = when (emulatorId) {
114-
"retroarch_64" -> "com.retroarch.aarch64"
115-
else -> "com.retroarch"
116-
}
117-
118111
private fun buildSaveFileName(context: SaveContext): String {
119112
// RetroArch uses ROM filename (without extension) as save filename
120113
if (context.romPath != null) {

‎app/src/main/kotlin/com/nendo/argosy/domain/usecase/state/RestoreStateUseCase.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ class RestoreStateUseCase @Inject constructor(
5959
config = config,
6060
platformId = platformId,
6161
romBaseName = romBaseName,
62-
slotNumber = cache.slotNumber
62+
slotNumber = cache.slotNumber,
63+
emulatorId = emulatorId,
64+
coreName = currentCoreId,
65+
romPath = romPath,
6366
) ?: return RestoreStateResult.Error("Could not determine target path")
6467

6568
val success = stateCacheManager.restoreState(cacheId, targetPath)

‎app/src/test/kotlin/com/nendo/argosy/data/sync/platform/RetroArchSaveHandlerTest.kt‎

Lines changed: 40 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package com.nendo.argosy.data.sync.platform
22

33
import android.content.Context
4-
import com.nendo.argosy.data.emulator.RetroArchConfigParser
5-
import com.nendo.argosy.data.emulator.RetroArchSaveConfig
4+
import com.nendo.argosy.data.emulator.RetroArchPathResolver
65
import com.nendo.argosy.data.emulator.SavePathConfig
7-
import com.nendo.argosy.data.emulator.SavePathRegistry
86
import com.nendo.argosy.data.storage.FileAccessLayer
7+
import io.mockk.coEvery
98
import io.mockk.every
109
import io.mockk.mockk
10+
import kotlinx.coroutines.test.runTest
1111
import org.junit.Assert.assertEquals
1212
import org.junit.Assert.assertFalse
1313
import org.junit.Assert.assertNotNull
@@ -19,48 +19,47 @@ class RetroArchSaveHandlerTest {
1919

2020
private lateinit var context: Context
2121
private lateinit var fal: FileAccessLayer
22-
private lateinit var parser: RetroArchConfigParser
22+
private lateinit var resolver: RetroArchPathResolver
2323
private lateinit var handler: RetroArchSaveHandler
2424

2525
@Before
2626
fun setUp() {
2727
context = mockk(relaxed = true)
2828
fal = mockk(relaxed = true)
29-
parser = mockk(relaxed = true)
30-
handler = RetroArchSaveHandler(context, fal, parser)
29+
resolver = mockk(relaxed = true)
30+
handler = RetroArchSaveHandler(context, fal, resolver)
3131
}
3232

3333
private fun saveContext(
3434
emulatorId: String = "retroarch",
3535
platformSlug: String = "snes",
3636
romPath: String? = "/storage/emulated/0/Roms/SNES/Mario.smc",
3737
gameTitle: String = "Super Mario World",
38-
localSavePath: String? = null,
38+
saveExtensions: List<String> = listOf("srm"),
3939
): SaveContext = SaveContext(
40-
config = mockk(relaxed = true) { every { saveExtensions } returns listOf("srm") },
40+
config = mockk(relaxed = true) { every { this@mockk.saveExtensions } returns saveExtensions },
4141
romPath = romPath,
4242
titleId = null,
4343
emulatorPackage = null,
4444
gameId = 1L,
4545
gameTitle = gameTitle,
4646
platformSlug = platformSlug,
4747
emulatorId = emulatorId,
48-
localSavePath = localSavePath,
48+
localSavePath = null,
4949
)
5050

5151
@Test
52-
fun `constructSavePath omits core subdir when sort_savefiles_enable is false`() {
53-
every { parser.parse(any()) } returns RetroArchSaveConfig(
54-
savefileDirectory = "/storage/emulated/0/RetroArch/saves",
55-
savefilesInContentDir = false,
56-
sortByContentDirectory = false,
57-
sortByCore = false,
58-
)
52+
fun `constructSavePath omits core subdir when sort_savefiles_enable is false`() = runTest {
53+
coEvery { resolver.buildSaveFilePath(any(), any(), any()) } answers {
54+
val baseName = secondArg<String>()
55+
val ext = thirdArg<String>()
56+
"/storage/emulated/0/RetroArch/saves/$baseName.$ext"
57+
}
5958

6059
val ctx = saveContext()
6160
val path = handler.constructSavePath(ctx)
6261

63-
assertNotNull("constructSavePath should not return null when core is mapped", path)
62+
assertNotNull(path)
6463
assertEquals(
6564
"When sort=false, save path must NOT contain the core subdirectory",
6665
"/storage/emulated/0/RetroArch/saves/Mario.srm",
@@ -69,62 +68,42 @@ class RetroArchSaveHandlerTest {
6968
}
7069

7170
@Test
72-
fun `constructSavePath uses core subdir when sort_savefiles_enable is true`() {
73-
every { parser.parse(any()) } returns RetroArchSaveConfig(
74-
savefileDirectory = "/storage/emulated/0/RetroArch/saves",
75-
savefilesInContentDir = false,
76-
sortByContentDirectory = false,
77-
sortByCore = true,
78-
)
79-
80-
val ctx = saveContext()
81-
val path = handler.constructSavePath(ctx)
82-
83-
assertNotNull(path)
84-
assertTrue(
85-
"When sort=true, path must contain core subdirectory; got: $path",
86-
path!!.contains("/Snes9x/") || path.contains("/snes9x/"),
87-
)
88-
}
89-
90-
@Test
91-
fun `constructSavePath honors content-dir mode`() {
92-
every { parser.parse(any()) } returns RetroArchSaveConfig(
93-
savefileDirectory = "/storage/emulated/0/RetroArch/saves",
94-
savefilesInContentDir = true,
95-
sortByContentDirectory = false,
96-
sortByCore = false,
97-
)
71+
fun `constructSavePath delegates to resolver with raw libretro core id`() = runTest {
72+
var capturedCore: String? = null
73+
coEvery { resolver.buildSaveFilePath(any(), any(), any()) } answers {
74+
capturedCore = firstArg<RetroArchPathResolver.Request>().coreName
75+
"/some/path/file.srm"
76+
}
9877

99-
val ctx = saveContext(romPath = "/storage/emulated/0/Roms/SNES/Mario.smc")
100-
val path = handler.constructSavePath(ctx)
78+
val ctx = saveContext(platformSlug = "snes")
79+
handler.constructSavePath(ctx)
10180

102-
assertNotNull(path)
103-
assertTrue(
104-
"When savefiles_in_content_dir=true, path should land next to ROM; got: $path",
105-
path!!.startsWith("/storage/emulated/0/Roms/SNES"),
81+
assertEquals(
82+
"Handler must pass raw libretro core id; resolver applies getRetroArchSaveDirName once",
83+
"snes9x",
84+
capturedCore,
10685
)
10786
}
10887

10988
@Test
110-
fun `constructSavePath honors PSX mcd extension`() {
111-
every { parser.parse(any()) } returns RetroArchSaveConfig(
112-
savefileDirectory = "/storage/emulated/0/RetroArch/saves",
113-
savefilesInContentDir = false,
114-
sortByContentDirectory = false,
115-
sortByCore = false,
116-
)
89+
fun `constructSavePath honors PSX mcd extension`() = runTest {
90+
var capturedExt: String? = null
91+
coEvery { resolver.buildSaveFilePath(any(), any(), any()) } answers {
92+
capturedExt = thirdArg<String>()
93+
"/some/path/file.${thirdArg<String>()}"
94+
}
11795

11896
val ctx = saveContext(
11997
platformSlug = "psx",
12098
romPath = "/storage/emulated/0/Roms/PSX/FFVII.cue",
99+
saveExtensions = listOf("mcd"),
121100
)
122-
val path = handler.constructSavePath(ctx)
101+
handler.constructSavePath(ctx)
123102

124-
assertNotNull(path)
125-
assertFalse(
126-
"PSX should not get .srm extension; got: $path",
127-
path!!.endsWith(".srm"),
103+
assertEquals(
104+
"PSX should pass mcd extension to resolver",
105+
"mcd",
106+
capturedExt,
128107
)
129108
}
130109
}

0 commit comments

Comments
 (0)