crossmate

A collaborative crossword app for iOS
Log | Files | Refs | LICENSE

commit a36d710291e5d060b6b567b66b75182a9beb75bb
parent 9448e53ad968310e2e981260647760c279723f33
Author: Michael Camilleri <[email protected]>
Date:   Fri, 24 Apr 2026 12:46:09 +0900

Improve snapshots creation

This commit fixes issues from previous commits that were identified by
Claude Opus 4.7. It also adds a reset button for clearing the database
of the current user.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Diffstat:
MCrossmate/CrossmateApp.swift | 3++-
MCrossmate/Persistence/GameStore.swift | 106+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
MCrossmate/Services/AppServices.swift | 23+++++++++++++++++++----
MCrossmate/Sync/RecordSerializer.swift | 50+++++++++++++++++++++++++++++++++++++++++---------
MCrossmate/Sync/SyncEngine.swift | 27+++++++++++++++++++++------
MCrossmate/Views/PuzzleView.swift | 2++
MCrossmate/Views/SyncDiagnosticsView.swift | 18++++++++++++++++++
MTests/Unit/MoveBufferTests.swift | 2+-
MTests/Unit/MoveLogTests.swift | 22++++++++++++++--------
9 files changed, 224 insertions(+), 29 deletions(-)

diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift @@ -22,6 +22,7 @@ struct CrossmateApp: App { .environment(\.syncEngine, services.syncEngine) .environment(services.nytAuth) .environment(\.nytPuzzleFetcher, services.nytFetcher) + .environment(\.resetDatabase, { await services.resetAllData() }) } } } @@ -106,7 +107,7 @@ private struct PuzzleDisplayView: View { var body: some View { Group { if let session { - PuzzleView(session: session) + PuzzleView(session: session, onComplete: { store.markCompleted(id: gameID) }) } else if let loadError { ContentUnavailableView( "Couldn't load puzzle", diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift @@ -88,6 +88,12 @@ final class GameStore { @ObservationIgnored var onGameCreated: ((String) -> Void)? + /// Called with the `ckRecordName`s of all records that should be deleted + /// from CloudKit after a game is removed locally. Wired to + /// `SyncEngine.enqueueDeleteRecords` by `AppServices`. + @ObservationIgnored + var onGameDeleted: (([String]) -> Void)? + init(persistence: PersistenceController) { self.persistence = persistence } @@ -151,6 +157,69 @@ final class GameStore { } } + /// Checks each game in `gameIDs` and writes a `SnapshotEntity` if the + /// game is complete (has `completedAt` set) or has 200 or more moves not + /// yet covered by an existing snapshot. Returns the `ckRecordName` of + /// each newly-created snapshot so the caller can enqueue it for CloudKit. + func createSnapshotsIfNeeded(for gameIDs: Set<UUID>) -> [String] { + var created: [String] = [] + for gameID in gameIDs { + let req = NSFetchRequest<GameEntity>(entityName: "GameEntity") + req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) + req.fetchLimit = 1 + guard let entity = try? context.fetch(req).first else { continue } + + let allMoves = (entity.moves as? Set<MoveEntity>) ?? [] + let allSnapshots = (entity.snapshots as? Set<SnapshotEntity>) ?? [] + let latestCoveredLamport = allSnapshots.map(\.upToLamport).max() ?? 0 + let uncoveredCount = allMoves.filter { $0.lamport > latestCoveredLamport }.count + let highWater = entity.lamportHighWater + + let shouldSnapshot = (entity.completedAt != nil || uncoveredCount >= 200) + && highWater > latestCoveredLamport + guard shouldSnapshot else { continue } + + let snapshots: [Snapshot] = allSnapshots.compactMap { se in + guard let data = se.gridState, + let grid = try? MoveLog.decodeGridState(data) else { return nil } + return Snapshot( + gameID: gameID, + upToLamport: se.upToLamport, + grid: grid, + createdAt: se.createdAt ?? Date() + ) + } + let moves: [Move] = allMoves.map { me in + Move( + gameID: gameID, + lamport: me.lamport, + row: Int(me.row), + col: Int(me.col), + letter: me.letter ?? "", + markKind: me.markKind, + checkedWrong: me.checkedWrong, + authorID: me.authorID, + createdAt: me.createdAt ?? Date() + ) + } + let grid = MoveLog.replay(snapshot: MoveLog.latestSnapshot(from: snapshots), moves: moves) + + let snapshotEntity = SnapshotEntity(context: context) + snapshotEntity.game = entity + snapshotEntity.upToLamport = highWater + snapshotEntity.createdAt = Date() + let ckName = RecordSerializer.recordName(forSnapshotInGame: gameID, upToLamport: highWater) + snapshotEntity.ckRecordName = ckName + snapshotEntity.gridState = try? MoveLog.encodeGridState(grid) + + if context.hasChanges { + try? context.save() + } + created.append(ckName) + } + return created + } + // MARK: - Load a specific game /// Loads a game by its entity ID. Sets it as the current game. @@ -222,6 +291,14 @@ final class GameStore { guard let entity = try context.fetch(request).first else { return } + // Collect all CloudKit record names before the cascade delete wipes them. + var ckRecordNames: [String] = [] + if let name = entity.ckRecordName { ckRecordNames.append(name) } + let moveNames = ((entity.moves as? Set<MoveEntity>) ?? []).compactMap(\.ckRecordName) + let snapshotNames = ((entity.snapshots as? Set<SnapshotEntity>) ?? []).compactMap(\.ckRecordName) + ckRecordNames.append(contentsOf: moveNames) + ckRecordNames.append(contentsOf: snapshotNames) + // Clear current references if this is the active game if currentEntity?.id == id { currentGame = nil @@ -231,6 +308,7 @@ final class GameStore { context.delete(entity) try context.save() + onGameDeleted?(ckRecordNames) } // MARK: - Resign a game @@ -251,6 +329,34 @@ final class GameStore { currentEntity = nil } + /// Marks a game as completed after a normal win. No-ops if already marked. + func markCompleted(id: UUID) { + let request = NSFetchRequest<GameEntity>(entityName: "GameEntity") + request.predicate = NSPredicate(format: "id == %@", id as CVarArg) + request.fetchLimit = 1 + guard let entity = try? context.fetch(request).first, + entity.completedAt == nil else { return } + entity.completedAt = Date() + try? context.save() + } + + // MARK: - Reset + + /// Deletes every game (and its cascaded moves, snapshots, and cells) plus + /// the sync-state row. Used by the diagnostics reset button. + func resetAllData() { + for entity in (try? context.fetch(NSFetchRequest<GameEntity>(entityName: "GameEntity"))) ?? [] { + context.delete(entity) + } + for entity in (try? context.fetch(NSFetchRequest<SyncStateEntity>(entityName: "SyncStateEntity"))) ?? [] { + context.delete(entity) + } + try? context.save() + currentGame = nil + currentMutator = nil + currentEntity = nil + } + // MARK: - Legacy convenience /// Returns the single current game and its mutator, creating from diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift @@ -35,10 +35,20 @@ final class AppServices { }, afterFlush: { gameIDs in await store.replayCellCaches(for: gameIDs) + let snapshotNames = await store.createSnapshotsIfNeeded(for: gameIDs) + for name in snapshotNames { + await syncEngine.enqueueSnapshot(ckRecordName: name) + } } ) self.moveBuffer = moveBuffer store.moveBuffer = moveBuffer + store.onGameCreated = { [syncEngine] ckRecordName in + Task { await syncEngine.enqueueGame(ckRecordName: ckRecordName) } + } + store.onGameDeleted = { [syncEngine] ckRecordNames in + Task { await syncEngine.enqueueDeleteRecords(ckRecordNames) } + } } func start(appDelegate: AppDelegate) async { @@ -48,10 +58,6 @@ final class AppServices { nytAuth.loadStoredSession() driveMonitor.start() - store.onGameCreated = { [syncEngine] ckRecordName in - Task { await syncEngine.enqueueGame(ckRecordName: ckRecordName) } - } - appDelegate.onRemoteNotification = { await self.handleRemoteNotification() } @@ -92,6 +98,15 @@ final class AppServices { await moveBuffer.flush() } + /// Wipes all local games and clears the CloudKit sync-engine state. + /// The next launch will start with an empty database and fetch everything + /// fresh from the server. + func resetAllData() async { + await syncEngine.resetSyncState() + store.resetAllData() + syncMonitor.note("Database reset — all games and sync state cleared") + } + func handleOpenURL(_ url: URL) -> UUID? { guard url.pathExtension.lowercased() == "xd" else { return nil } diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift @@ -7,18 +7,42 @@ import Foundation enum RecordSerializer { static let zoneName = "CrossmateZone" + // MARK: - Device identity + + /// A stable per-device identifier appended to move and snapshot record + /// names to prevent two devices owned by the same iCloud user from + /// producing identical record names when both assign the same Lamport + /// clock value while offline. + /// + /// Stored in UserDefaults so it survives app restarts but resets on + /// reinstall (which is fine — a reinstalled app has no local moves to + /// conflict with). + static let localDeviceID: String = { + let key = "crossmate.localDeviceID" + if let stored = UserDefaults.standard.string(forKey: key) { + return stored + } + let new = UUID().uuidString.replacingOccurrences(of: "-", with: "").lowercased() + UserDefaults.standard.set(new, forKey: key) + return new + }() + // MARK: - Record names static func recordName(forGameID gameID: UUID) -> String { "game-\(gameID.uuidString)" } + /// Includes `localDeviceID` so two devices with the same Lamport value + /// produce distinct record names and can coexist in CloudKit without a + /// write-write conflict. static func recordName(forMoveInGame gameID: UUID, lamport: Int64) -> String { - "move-\(gameID.uuidString)-\(lamport)" + "move-\(gameID.uuidString)-\(lamport)-\(localDeviceID)" } + /// Includes `localDeviceID` for the same reason as move records. static func recordName(forSnapshotInGame gameID: UUID, upToLamport: Int64) -> String { - "snapshot-\(gameID.uuidString)-\(upToLamport)" + "snapshot-\(gameID.uuidString)-\(upToLamport)-\(localDeviceID)" } // MARK: - Zone @@ -153,14 +177,22 @@ enum RecordSerializer { ) -> (UUID, Int64)? { guard name.hasPrefix(prefix) else { return nil } let rest = name.dropFirst(prefix.count) - // `<UUID>-<Int64>`. UUID itself contains dashes, so split from the - // last separator rather than naively on every dash. - guard let lastDash = rest.lastIndex(of: "-") else { return nil } - let uuidPart = String(rest[rest.startIndex..<lastDash]) - let lamportPart = String(rest[rest.index(after: lastDash)...]) - guard let gameID = UUID(uuidString: uuidPart), - let lamport = Int64(lamportPart) + // Format: `<36-char UUID>-<lamport>-<deviceID>`. + // UUIDs are always 36 chars (8-4-4-4-12), so slice by fixed length. + let uuidLength = 36 + guard rest.count > uuidLength, + rest[rest.index(rest.startIndex, offsetBy: uuidLength)] == "-" else { return nil } + let uuidPart = String(rest[rest.startIndex..<rest.index(rest.startIndex, offsetBy: uuidLength)]) + guard let gameID = UUID(uuidString: uuidPart) else { return nil } + // After the UUID dash: `<lamport>-<deviceID>`. The device suffix is + // required — records without it are from an older build and can't be + // parsed. + let afterUUID = rest.index(rest.startIndex, offsetBy: uuidLength + 1) + let tail = rest[afterUUID...] + guard let nextDash = tail.firstIndex(of: "-") else { return nil } + let lamportPart = tail[tail.startIndex..<nextDash] + guard let lamport = Int64(lamportPart) else { return nil } return (gameID, lamport) } diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift @@ -5,6 +5,7 @@ import SwiftUI extension EnvironmentValues { @Entry var syncEngine: SyncEngine? = nil + @Entry var resetDatabase: (() async -> Void)? = nil } /// Owns the CloudKit sync lifecycle via `CKSyncEngine`. Zone creation, @@ -46,7 +47,6 @@ actor SyncEngine { /// wiring callbacks. func start() { let bgCtx = persistence.container.newBackgroundContext() - bgCtx.userInfo["OutboxRecorder.skip"] = true let saved: CKSyncEngine.State.Serialization? = bgCtx.performAndWait { guard let data = SyncStateEntity.current(in: bgCtx).ckEngineState else { return nil } return try? JSONDecoder().decode(CKSyncEngine.State.Serialization.self, from: data) @@ -72,6 +72,26 @@ actor SyncEngine { }) } + /// Registers record deletions as pending sends. Called before the + /// corresponding Core Data entities are deleted so their record names + /// are still readable. + func enqueueDeleteRecords(_ ckRecordNames: [String]) { + guard let engine, !ckRecordNames.isEmpty else { return } + let zoneID = RecordSerializer.zoneID() + engine.state.add(pendingRecordZoneChanges: ckRecordNames.map { name in + .deleteRecord(CKRecord.ID(recordName: name, zoneID: zoneID)) + }) + } + + /// Registers a Snapshot record as a pending send. Called after a + /// `SnapshotEntity` has been written to Core Data. + func enqueueSnapshot(ckRecordName: String) { + guard let engine else { return } + let zoneID = RecordSerializer.zoneID() + let recordID = CKRecord.ID(recordName: ckRecordName, zoneID: zoneID) + engine.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)]) + } + /// Registers a Game record as a pending send. Called when a new game is /// created locally so the parent record exists in CloudKit before its /// child Move records arrive. @@ -144,7 +164,6 @@ actor SyncEngine { /// engine. Pending records already in CloudKit are unaffected. func resetSyncState() async { let ctx = persistence.container.newBackgroundContext() - ctx.userInfo["OutboxRecorder.skip"] = true ctx.performAndWait { SyncStateEntity.current(in: ctx).ckEngineState = nil try? ctx.save() @@ -160,7 +179,6 @@ actor SyncEngine { private func saveEngineState(_ serialization: CKSyncEngine.State.Serialization) { let ctx = persistence.container.newBackgroundContext() - ctx.userInfo["OutboxRecorder.skip"] = true ctx.performAndWait { guard let data = try? JSONEncoder().encode(serialization) else { return } SyncStateEntity.current(in: ctx).ckEngineState = data @@ -177,7 +195,6 @@ actor SyncEngine { ) -> CKRecord? { let name = recordID.recordName let ctx = persistence.container.newBackgroundContext() - ctx.userInfo["OutboxRecorder.skip"] = true return ctx.performAndWait { if name.hasPrefix("game-") { let req = NSFetchRequest<GameEntity>(entityName: "GameEntity") @@ -438,7 +455,6 @@ actor SyncEngine { var newMoves: [Move] = [] let ctx = persistence.container.newBackgroundContext() - ctx.userInfo["OutboxRecorder.skip"] = true ctx.performAndWait { for mod in event.modifications { @@ -516,7 +532,6 @@ actor SyncEngine { _ event: CKSyncEngine.Event.SentRecordZoneChanges ) { let ctx = persistence.container.newBackgroundContext() - ctx.userInfo["OutboxRecorder.skip"] = true ctx.performAndWait { for record in event.savedRecords { self.writeBackSystemFields(record: record, in: ctx) diff --git a/Crossmate/Views/PuzzleView.swift b/Crossmate/Views/PuzzleView.swift @@ -2,6 +2,7 @@ import SwiftUI struct PuzzleView: View { @Bindable var session: PlayerSession + var onComplete: (() -> Void)? = nil @Environment(PlayerPreferences.self) private var preferences @State private var isRenaming = false @State private var renameDraft = "" @@ -175,6 +176,7 @@ struct PuzzleView: View { if session.isPencilMode { session.togglePencil() } + onComplete?() showSuccessScreen = true } } diff --git a/Crossmate/Views/SyncDiagnosticsView.swift b/Crossmate/Views/SyncDiagnosticsView.swift @@ -3,9 +3,11 @@ import SwiftUI struct SyncDiagnosticsView: View { @Environment(\.syncEngine) private var syncEngine + @Environment(\.resetDatabase) private var resetDatabase @Environment(SyncMonitor.self) private var syncMonitor @State private var isSyncing = false + @State private var showResetConfirmation = false private static let timestampFormatter: DateFormatter = { let formatter = DateFormatter() @@ -56,6 +58,22 @@ struct SyncDiagnosticsView: View { Task { await resetSyncState() } } .disabled(isSyncing) + + Button("Reset Database", role: .destructive) { + showResetConfirmation = true + } + .disabled(isSyncing) + .confirmationDialog( + "Delete all games?", + isPresented: $showResetConfirmation, + titleVisibility: .visible + ) { + Button("Delete All Games", role: .destructive) { + Task { await resetDatabase?() } + } + } message: { + Text("This removes every game and clears the sync state on this device. Games on other devices and in iCloud are not affected.") + } } Section("Recent Events") { diff --git a/Tests/Unit/MoveBufferTests.swift b/Tests/Unit/MoveBufferTests.swift @@ -190,7 +190,7 @@ struct MoveBufferTests { #expect(m?.checkedWrong == true) #expect(m?.authorID == "alice") #expect(m?.lamport == 1) - #expect(m?.ckRecordName == "move-\(gameID.uuidString)-1") + #expect(m?.ckRecordName == "move-\(gameID.uuidString)-1-\(RecordSerializer.localDeviceID)") } // MARK: - Helpers diff --git a/Tests/Unit/MoveLogTests.swift b/Tests/Unit/MoveLogTests.swift @@ -165,8 +165,9 @@ struct GridStateCodecTests { #expect(decoded == grid) } - @Test("Encoding is deterministic for the same grid") - func encodingIsDeterministic() throws { + @Test("Encoding sorts entries in row-major, col-minor order regardless of dictionary iteration order") + func encodingSortsEntries() throws { + // Dictionary iteration order is unspecified; encoding must sort. let grid: GridState = [ GridPosition(row: 2, col: 1): GridCell( letter: "X", markKind: 0, checkedWrong: false, authorID: nil @@ -175,9 +176,14 @@ struct GridStateCodecTests { letter: "A", markKind: 0, checkedWrong: false, authorID: nil ), ] - let first = try MoveLog.encodeGridState(grid) - let second = try MoveLog.encodeGridState(grid) - #expect(first == second) + let payload = try JSONDecoder().decode( + MoveLog.GridStatePayload.self, + from: MoveLog.encodeGridState(grid) + ) + #expect(payload.entries.map { GridPosition(row: $0.row, col: $0.col) } == [ + GridPosition(row: 0, col: 0), + GridPosition(row: 2, col: 1), + ]) } } @@ -190,13 +196,13 @@ struct RecordSerializerMoveSnapshotTests { @Test("Move record name uses the expected format") func moveRecordNameFormat() { let name = RecordSerializer.recordName(forMoveInGame: gameID, lamport: 42) - #expect(name == "move-\(gameID.uuidString)-42") + #expect(name == "move-\(gameID.uuidString)-42-\(RecordSerializer.localDeviceID)") } @Test("Snapshot record name uses the expected format") func snapshotRecordNameFormat() { let name = RecordSerializer.recordName(forSnapshotInGame: gameID, upToLamport: 100) - #expect(name == "snapshot-\(gameID.uuidString)-100") + #expect(name == "snapshot-\(gameID.uuidString)-100-\(RecordSerializer.localDeviceID)") } @Test("Move record round-trips through CKRecord fields") @@ -271,7 +277,7 @@ struct RecordSerializerMoveSnapshotTests { @Test("Parsing rejects records with the wrong record type") func parseRejectsWrongRecordType() { let zoneID = RecordSerializer.zoneID() - let recordID = CKRecord.ID(recordName: "move-\(gameID.uuidString)-1", zoneID: zoneID) + let recordID = CKRecord.ID(recordName: RecordSerializer.recordName(forMoveInGame: gameID, lamport: 1), zoneID: zoneID) let record = CKRecord(recordType: "Cell", recordID: recordID) #expect(RecordSerializer.parseMoveRecord(record) == nil) }