Merged
Conversation
There are some work actually to port grout on arm32, and this code was merged before it was ready. Revert back to the old launch.sh for spruce
Sync main
…pings moved to the sync menu
Installed on Anbernix RG34XX without any issues. Saw instance and tested download of single game as well.
Bump gabagool to v2.9.5 (table sections), sqlite to v1.46.1. Add save_sync_history table in schema v10 with indexes on rom_id and device_id.
- cache/save_sync.go: sync history recording, querying, GetSyncedRomIDs - cache/games.go: add GetRomByFSLookup for resolving ROMs by fs_slug - internal/config.go: add SlotPreferences, GetSlotPreference, SetSlotPreference, GetDirectoryMapping - romm/saves.go: switch device tracking endpoints from query to body params, add SaveDeviceBody type
- sync/models.go: action types and sync plan models - sync/flow.go: core sync flow with conflict detection - sync/flow_test.go: tests for sync action determination - sync/roms.go: rewrite to resolve local ROMs against cache - cfw/roms.go: local ROM filesystem scanning (moved from sync) - cfw/saves.go: add GetSaveDirectory helper - tools/save-sync-dry-run/: dry-run testing tool
- ui/sync_menu.go: sync hub with Sync Now, Synced Games, History - ui/save_sync.go: sync execution with progress display - ui/save_conflict.go: conflict resolution (keep local/remote) - ui/synced_games.go: platform-grouped view from local sync history - ui/sync_history.go: table-based history with action icons and platform slugs, grouped by date then action - ui/device_registration.go: device name input for registration - ui/game_options.go: add save slot selection when multiple slots exist - ui/actions.go: add action types for new screens
- app/screens.go: register new screen constants - app/router.go: register screen handlers - app/transitions.go: add transition functions for sync menu, synced games, history, and save sync settings - ui/platform_selection.go: add Y button for sync menu access - ui/settings.go: add Save Sync entry in settings menu - ui/rebuild_cache.go: use cache Clear() instead of folder delete
Add locale keys for sync menu, synced games, sync history, save conflicts, device registration, and save slot options across all supported languages (en, de, es, fr, it, ja, pt, ru).
Use outline cloud icons for upload/download actions, add cloud outline as the action column header. Convert UTC timestamps to local time for display. Sort rows by action group then alphabetically.
Load all sync history records instead of capping at 50. Performance tested with 2000+ records without issue.
fix(muos): handle grout icons for muos jacaranda and prior
Catchup to main
# Conflicts: # cfw/saves.go
Previously, FetchRemoteSaves and DiscoverRemoteSaves silently continued on HTTP failures, causing local saves to appear as new uploads and potentially overwriting newer remote data. Now errors are propagated and the UI shows a connection error instead of proceeding with a misleading sync. Closes #144
…cleanup dead code - Add concurrent fetching (8 workers) for FetchRemoteSaves and DiscoverRemoteSaves - Guard against empty emulator string on upload - Add backup retention setting (5/10/15/No Limit) with cleanup logic - Show last synced relative time in sync menu - Add health check before starting save sync - Remove unused API methods (TrackSave, UntrackSave, GetSaveIdentifiers) - Remove dead ScreenSaveConflict router registration - Set with_filter_values and with_char_index to false on ROM queries - Add missing i18n keys to all locale files
Greptile SummaryThis PR rewrites the Save Sync feature for grout, replacing the previous auto-sync model with an explicit, user-driven Redux-style flow. The new architecture introduces a clean resolve → conflict-resolution → execute pipeline ( Key findings:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User: Tap Sync Now] --> B[SaveSyncScreen.Execute]
B --> C{ResolvedItems provided?}
C -- Yes --> G
C -- No --> D[Health check: GetHeartbeat]
D -- Fail --> E[Show error message\nReturn empty output]
D -- OK --> F[ResolveSaveSync\nScan local saves\nFetch remote saves concurrently\nDiscoverRemoteSaves for ROMs without local saves]
F --> F1{Multiple slots\non first-time downloads?}
F1 -- Yes --> F2[Show slot picker\nUpdate config.SlotPreferences\nSave to save_slots.json]
F1 -- No --> F3
F2 --> F3{Any conflicts?}
F3 -- Yes --> H[Return NeedsConflictResolution=true\nto router]
H --> I[SaveConflictScreen\nKeep Local / Keep Remote per game]
I -- Cancelled --> J[popOrExit: back to Sync Menu]
I -- Resolved --> K[transitionSaveConflict\nMerge resolved items back into AllItems]
K --> B
F3 -- No --> G[executeSyncPhase]
G --> L[ExecuteSaveSync\nFor each item: upload / download / skip / conflict]
L --> L1[upload: POST save file\nMarkDeviceSynced\nChtimes to match server mtime]
L --> L2[download: backup existing file\nGET save content\nwrite file\nMarkDeviceSynced]
L --> M[RecordSaveSync for successful items\nINSERT INTO save_sync_history]
M --> N[showReport: Uploaded / Downloaded / Errors / Conflicts]
Last reviewed commit: 86d120b |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } else { | ||
| ctx.state.Config.Hosts = []romm.Host{ctx.state.Host} | ||
| } | ||
| internal.SaveConfig(ctx.state.Config) |
There was a problem hiding this comment.
SaveConfig error silently discarded
internal.SaveConfig returns an error, but it is ignored here. If the write fails (e.g., read-only filesystem, disk full on the handheld), the device registration (DeviceID, DeviceName) and backup-limit changes are held in memory for the current session but will be permanently lost on the next restart with no feedback to the user.
Suggested change
| internal.SaveConfig(ctx.state.Config) | |
| if err := internal.SaveConfig(ctx.state.Config); err != nil { | |
| gaba.GetLogger().Error("Failed to save config after save-sync settings update", "error", err) | |
| } |
Comment on lines
+119
to
+131
| err := cm.db.QueryRow(` | ||
| SELECT synced_at FROM save_sync_history | ||
| WHERE device_id = ? | ||
| ORDER BY synced_at DESC | ||
| LIMIT 1 | ||
| `, deviceID).Scan(&syncedAt) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
|
|
||
| t, err := time.Parse(time.RFC3339, syncedAt) | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
GetLastSyncTime silently swallows parse error and is never called
Two separate concerns here:
-
When
time.Parsefails, the function returnsnilwithout logging anything.GetSaveSyncHistory(line 60–65 of the same file) logs aWarnfor the identical scenario — this is an inconsistency that could make silent DB corruption harder to diagnose. -
GetLastSyncTimeis not called anywhere in the codebase. If it is intended for future use, keeping it around without callers means it will drift out of sync with the schema changes around it.
Consider adding the same Warn log that GetSaveSyncHistory uses, and either adding a caller or removing the function until it is needed.
Suggested change
| err := cm.db.QueryRow(` | |
| SELECT synced_at FROM save_sync_history | |
| WHERE device_id = ? | |
| ORDER BY synced_at DESC | |
| LIMIT 1 | |
| `, deviceID).Scan(&syncedAt) | |
| if err != nil { | |
| return nil | |
| } | |
| t, err := time.Parse(time.RFC3339, syncedAt) | |
| if err != nil { | |
| return nil | |
| t, err := time.Parse(time.RFC3339, syncedAt) | |
| if err != nil { | |
| gaba.GetLogger().Warn("Failed to parse last sync timestamp", "value", syncedAt, "error", err) | |
| return nil | |
| } |
Comment on lines
+368
to
376
| _, err = tx.Exec(`CREATE INDEX IF NOT EXISTS idx_save_sync_history_rom_id ON save_sync_history(rom_id)`) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Track per-platform game sync status | ||
| _, err = tx.Exec(` | ||
| CREATE TABLE IF NOT EXISTS platform_sync_status ( | ||
| platform_id INTEGER PRIMARY KEY, | ||
| last_successful_sync TEXT, | ||
| last_attempt TEXT, | ||
| games_synced INTEGER DEFAULT 0, | ||
| status TEXT DEFAULT 'pending' | ||
| ) | ||
| `) | ||
| _, err = tx.Exec(`CREATE INDEX IF NOT EXISTS idx_save_sync_history_device_id ON save_sync_history(device_id)`) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
save_sync_history missing composite index for primary query pattern
The main read path in GetSaveSyncHistory is:
SELECT … FROM save_sync_history
WHERE device_id = ?
ORDER BY synced_at DESCThere are separate indexes on rom_id and device_id, but no composite (device_id, synced_at) index. SQLite will use the device_id index to satisfy the WHERE, but must then do a separate sort pass for ORDER BY synced_at DESC. On a handheld device with many sync entries this in-memory sort adds unnecessary overhead.
Adding a composite index eliminates the sort:
CREATE INDEX IF NOT EXISTS idx_save_sync_history_device_synced
ON save_sync_history(device_id, synced_at DESC)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #134 #135