-
-
Notifications
You must be signed in to change notification settings - Fork 268
feat: add GOG cloud save conflict resolution #1273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,12 @@ class GOGCloudSavesManager( | |
| NONE | ||
| } | ||
|
|
||
| data class SyncCheckResult( | ||
| val action: SyncAction, | ||
| val localTimestampMs: Long, | ||
| val remoteTimestampMs: Long, | ||
| ) | ||
|
|
||
| /** | ||
| * Represents a local save file | ||
| */ | ||
|
|
@@ -94,6 +100,23 @@ class GOGCloudSavesManager( | |
| Timber.e(e, "Failed to calculate metadata for $absolutePath") | ||
| } | ||
| } | ||
|
|
||
| suspend fun calculateTimestamp() = withContext(Dispatchers.IO) { | ||
| try { | ||
| val file = File(absolutePath) | ||
| if (!file.exists() || !file.isFile) { | ||
| Timber.w("File does not exist: $absolutePath") | ||
| return@withContext | ||
| } | ||
|
|
||
| val timestamp = file.lastModified() | ||
| val instant = Instant.ofEpochMilli(timestamp) | ||
| updateTime = DateTimeFormatter.ISO_INSTANT.format(instant) | ||
| updateTimestamp = timestamp / 1000 | ||
| } catch (e: Exception) { | ||
| Timber.e(e, "Failed to read timestamp for $absolutePath") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -214,18 +237,33 @@ class GOGCloudSavesManager( | |
| // Handle preferred action | ||
| if (preferredAction == "download" && downloadableCloud.isNotEmpty()) { | ||
| Timber.tag("GOG-CloudSaves").i("Forcing download of ${downloadableCloud.size} file(s) (user requested)") | ||
| downloadableCloud.forEach { file -> | ||
| downloadFile(credentials.userId, clientId, dirname, file, syncDir, credentials.accessToken) | ||
| val allDownloaded = downloadableCloud.fold(true) { allSucceeded, file -> | ||
| val downloaded = downloadFile( | ||
| credentials.userId, | ||
| clientId, | ||
| dirname, | ||
| file, | ||
| syncDir, | ||
| credentials.accessToken, | ||
| ) | ||
| allSucceeded && downloaded | ||
| } | ||
| return@withContext currentTimestamp() | ||
| return@withContext if (allDownloaded) currentTimestamp() else 0L | ||
| } | ||
|
|
||
| if (preferredAction == "upload" && localFiles.isNotEmpty()) { | ||
| Timber.tag("GOG-CloudSaves").i("Forcing upload of ${localFiles.size} file(s) (user requested)") | ||
| localFiles.forEach { file -> | ||
| uploadFile(credentials.userId, clientId, dirname, file, credentials.accessToken) | ||
| val allUploaded = localFiles.fold(true) { allSucceeded, file -> | ||
| val uploaded = uploadFile( | ||
| credentials.userId, | ||
| clientId, | ||
| dirname, | ||
| file, | ||
| credentials.accessToken, | ||
| ) | ||
| allSucceeded && uploaded | ||
| } | ||
| return@withContext currentTimestamp() | ||
| return@withContext if (allUploaded) currentTimestamp() else 0L | ||
| } | ||
|
|
||
| // Complex sync scenario - use classifier | ||
|
|
@@ -331,10 +369,51 @@ class GOGCloudSavesManager( | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Read-only pre-flight check: returns the sync action and file timestamps in a single network | ||
| * round-trip. Returns null if credentials or cloud file listing are unavailable. | ||
| */ | ||
| suspend fun checkSync( | ||
| localPath: String, | ||
| dirname: String, | ||
| clientId: String, | ||
| clientSecret: String, | ||
| lastSyncTimestamp: Long = 0, | ||
| ): SyncCheckResult? = withContext(Dispatchers.IO) { | ||
| try { | ||
| val credentials = GOGAuthManager.getGameCredentials(context, clientId, clientSecret) | ||
| .getOrNull() ?: return@withContext null | ||
|
|
||
| val syncDir = File(localPath) | ||
| val localFiles = if (syncDir.exists()) scanLocalFiles(syncDir, calculateHashes = false) else emptyList() | ||
| val cloudFiles = getCloudFiles(credentials.userId, clientId, dirname, credentials.accessToken) | ||
| ?: return@withContext null | ||
|
|
||
| val action = when { | ||
| localFiles.isEmpty() && cloudFiles.isEmpty() -> SyncAction.NONE | ||
| localFiles.isNotEmpty() && cloudFiles.isEmpty() -> SyncAction.UPLOAD | ||
| localFiles.isEmpty() && cloudFiles.any { !it.isDeleted } -> SyncAction.DOWNLOAD | ||
| else -> classifyFiles(localFiles, cloudFiles, lastSyncTimestamp).determineAction() | ||
| } | ||
|
|
||
| val localMax = localFiles.maxOfOrNull { it.updateTimestamp ?: 0L } ?: 0L | ||
| val remoteMax = cloudFiles | ||
| .filter { !it.isDeleted } | ||
| .maxOfOrNull { it.updateTimestamp ?: 0L } ?: 0L | ||
| SyncCheckResult(action, localMax * 1000, remoteMax * 1000) | ||
|
Comment on lines
+399
to
+403
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
♻️ Proposed fix- val localMax = localFiles.maxOfOrNull { it.updateTimestamp ?: 0L } ?: 0L
- val remoteMax = cloudFiles.maxOfOrNull { it.updateTimestamp ?: 0L } ?: 0L
+ val localMax = localFiles.maxOfOrNull { it.updateTimestamp ?: 0L } ?: 0L
+ val remoteMax = cloudFiles
+ .filter { !it.isDeleted }
+ .maxOfOrNull { it.updateTimestamp ?: 0L } ?: 0L
SyncCheckResult(action, localMax * 1000, remoteMax * 1000)🤖 Prompt for AI Agents |
||
| } catch (e: Exception) { | ||
| Timber.tag("GOG-CloudSaves").e(e, "checkSync failed") | ||
| null | ||
| } | ||
| } | ||
|
kiequoo marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Scan local directory for save files | ||
| */ | ||
| private suspend fun scanLocalFiles(directory: File): List<SyncFile> = withContext(Dispatchers.IO) { | ||
| private suspend fun scanLocalFiles( | ||
| directory: File, | ||
| calculateHashes: Boolean = true, | ||
| ): List<SyncFile> = withContext(Dispatchers.IO) { | ||
| val files = mutableListOf<SyncFile>() | ||
|
|
||
| fun scanRecursive(dir: File, basePath: String) { | ||
|
|
@@ -352,8 +431,13 @@ class GOGCloudSavesManager( | |
|
|
||
| scanRecursive(directory, directory.absolutePath) | ||
|
|
||
| // Calculate metadata for all files | ||
| files.forEach { it.calculateMetadata() } | ||
| files.forEach { file -> | ||
| if (calculateHashes) { | ||
| file.calculateMetadata() | ||
| } else { | ||
| file.calculateTimestamp() | ||
| } | ||
| } | ||
|
|
||
| files | ||
| } | ||
|
|
@@ -467,7 +551,7 @@ class GOGCloudSavesManager( | |
| dirname: String, | ||
| file: SyncFile, | ||
| authToken: String | ||
| ) = withContext(Dispatchers.IO) { | ||
| ): Boolean = withContext(Dispatchers.IO) { | ||
| try { | ||
| val localFile = File(file.absolutePath) | ||
| val fileSize = localFile.length() | ||
|
|
@@ -495,15 +579,18 @@ class GOGCloudSavesManager( | |
| response.use { | ||
| if (response.isSuccessful) { | ||
| Timber.tag("GOG-CloudSaves").i("Successfully uploaded: ${file.relativePath}") | ||
| true | ||
| } else { | ||
| val errorBody = response.body?.string() ?: "No response body" | ||
| Timber.tag("GOG-CloudSaves").e("Failed to upload ${file.relativePath}: HTTP ${response.code}") | ||
| Timber.tag("GOG-CloudSaves").e("Upload error body: $errorBody") | ||
| false | ||
| } | ||
| } | ||
|
|
||
| } catch (e: Exception) { | ||
| Timber.tag("GOG-CloudSaves").e(e, "Failed to upload ${file.relativePath}") | ||
| false | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -517,7 +604,7 @@ class GOGCloudSavesManager( | |
| file: CloudFile, | ||
| syncDir: File, | ||
| authToken: String | ||
| ) = withContext(Dispatchers.IO) { | ||
| ): Boolean = withContext(Dispatchers.IO) { | ||
| try { | ||
| Timber.tag("GOG-CloudSaves").i("Downloading: ${file.relativePath}") | ||
|
|
||
|
|
@@ -536,10 +623,10 @@ class GOGCloudSavesManager( | |
| val errorBody = response.body?.string() ?: "No response body" | ||
| Timber.tag("GOG-CloudSaves").e("Failed to download ${file.relativePath}: HTTP ${response.code}") | ||
| Timber.tag("GOG-CloudSaves").e("Download error body: $errorBody") | ||
| return@withContext | ||
| return@withContext false | ||
| } | ||
|
|
||
| val bytes = response.body?.bytes() ?: return@withContext | ||
| val bytes = response.body?.bytes() ?: return@withContext false | ||
| Timber.tag("GOG-CloudSaves").d("Downloaded ${bytes.size} bytes for ${file.relativePath}") | ||
|
|
||
| // resolve against on-disk casing to avoid creating duplicate dirs | ||
|
|
@@ -556,10 +643,12 @@ class GOGCloudSavesManager( | |
| } | ||
|
|
||
| Timber.tag("GOG-CloudSaves").i("Successfully downloaded: ${file.relativePath}") | ||
| true | ||
| } | ||
|
|
||
| } catch (e: Exception) { | ||
| Timber.tag("GOG-CloudSaves").e(e, "Failed to download ${file.relativePath}") | ||
| false | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forced Keep Local/Keep Remote aborts on first failure, leaving a partial sync.
.all { ... }short‑circuits: the moment a singledownloadFile/uploadFilereturnsfalse, the remaining files are never attempted. With the newpreLaunchAppbehavior that surfacesSYNC_FAILon a0Lreturn, a transient failure on file#1means:#2..#N are silently skipped even though they would likely have succeeded.SYNC_FAILand retries, but local/cloud state is already partially transferred (anything done before the failure persists), and the next retry may itself conflict because only a subset of files was propagated.For an explicit user decision ("Keep Local" / "Keep Remote"), prefer best‑effort: attempt every file, then report overall success.
♻️ Proposed fix
if (preferredAction == "download" && downloadableCloud.isNotEmpty()) { Timber.tag("GOG-CloudSaves").i("Forcing download of ${downloadableCloud.size} file(s) (user requested)") - val allDownloaded = downloadableCloud.all { file -> - downloadFile(credentials.userId, clientId, dirname, file, syncDir, credentials.accessToken) - } + val allDownloaded = downloadableCloud.fold(true) { acc, file -> + val ok = downloadFile(credentials.userId, clientId, dirname, file, syncDir, credentials.accessToken) + acc && ok + } return@withContext if (allDownloaded) currentTimestamp() else 0L } if (preferredAction == "upload" && localFiles.isNotEmpty()) { Timber.tag("GOG-CloudSaves").i("Forcing upload of ${localFiles.size} file(s) (user requested)") - val allUploaded = localFiles.all { file -> - uploadFile(credentials.userId, clientId, dirname, file, credentials.accessToken) - } + val allUploaded = localFiles.fold(true) { acc, file -> + val ok = uploadFile(credentials.userId, clientId, dirname, file, credentials.accessToken) + acc && ok + } return@withContext if (allUploaded) currentTimestamp() else 0L }🤖 Prompt for AI Agents