commit ed7c2f607c5711a56371788b90e61c6d262fbd1c
parent fde29e2799e42bd38804e5c99ffa4d7cfa5f6e18
Author: Michael Camilleri <[email protected]>
Date: Sat, 2 May 2026 20:29:59 +0900
Unwind changes to reduce CloudKit calls
In a mistaken attempt to address the cause of an unresponsiveness that
was the result of unnecessary parsing, CloudKit calls were reduced. This
has complicated the synchronisation logic. This commit undoes that.
In addition, the snapshot logic can fail because Lamports are
independently assigned per player so can conflict. In shared games,
snapshotting is disabled.
Co-Authored-By: Codex GPT 5.5 <[email protected]>
Diffstat:
5 files changed, 88 insertions(+), 42 deletions(-)
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -339,6 +339,7 @@ final class GameStore {
req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
req.fetchLimit = 1
guard let entity = try? bgCtx.fetch(req).first else { continue }
+ guard !Self.usesSharedSync(entity) else { continue }
let allMoves = (entity.moves as? Set<MoveEntity>) ?? []
let allSnapshots = (entity.snapshots as? Set<SnapshotEntity>) ?? []
@@ -421,6 +422,10 @@ final class GameStore {
snapshot.needsPruning = false
continue
}
+ guard !Self.usesSharedSync(game) else {
+ snapshot.needsPruning = false
+ continue
+ }
let covered = ((game.moves as? Set<MoveEntity>) ?? [])
.filter { $0.lamport <= snapshot.upToLamport }
@@ -791,6 +796,10 @@ final class GameStore {
snapshot.needsPruning = false
continue
}
+ guard !Self.usesSharedSync(game) else {
+ snapshot.needsPruning = false
+ continue
+ }
let covered = ((game.moves as? Set<MoveEntity>) ?? [])
.filter { $0.lamport <= snapshot.upToLamport }
for move in covered {
@@ -804,6 +813,10 @@ final class GameStore {
return prunedMoveNames
}
+ fileprivate nonisolated static func usesSharedSync(_ game: GameEntity) -> Bool {
+ game.databaseScope == 1 || game.ckShareRecordName != nil
+ }
+
/// Marks the active game read-only when the sync engine sees its shared
/// zone disappear from the shared database (owner revoked access).
func markAccessRevoked(gameID: UUID) {
diff --git a/Crossmate/Sync/MoveBuffer.swift b/Crossmate/Sync/MoveBuffer.swift
@@ -44,10 +44,6 @@ actor MoveBuffer {
/// different cell flushes first; subsequent enqueues for the same cell
/// replace the pending value without flushing.
private var lastCell: Key?
- /// Moves already persisted locally but not yet handed to CloudKit. Cell
- /// changes flush for Lamport ordering; CloudKit enqueue waits for a
- /// trailing/explicit flush so typing does not poke CKSyncEngine per cell.
- private var pendingSinkMoves: [Move] = []
private var debounceTask: Task<Void, Never>?
/// Per-game timestamp of the last SessionPing fired. The first
/// `enqueue` for a game with no entry — or one stale beyond
@@ -85,12 +81,7 @@ actor MoveBuffer {
let key = Key(gameID: gameID, row: row, col: col)
if let lastCell, lastCell != key {
- // Cell-change flush exists to keep lamport ordering correct. Skip
- // the afterFlush callback here so the cache rebuild and snapshot
- // creation don't run on the main actor between every keystroke;
- // the trailing-edge debounced flush picks them up once typing
- // settles.
- await performFlush(runAfterFlush: false)
+ await performFlush()
}
if buffer[key] == nil {
@@ -134,7 +125,7 @@ actor MoveBuffer {
func flush() async {
debounceTask?.cancel()
debounceTask = nil
- await performFlush(runAfterFlush: true)
+ await performFlush()
}
private func scheduleDebounce() {
@@ -149,30 +140,23 @@ actor MoveBuffer {
private func debouncedFlush() async {
debounceTask = nil
- await performFlush(runAfterFlush: true)
+ await performFlush()
}
- private func performFlush(runAfterFlush: Bool) async {
- guard !buffer.isEmpty || (runAfterFlush && !pendingSinkMoves.isEmpty) else { return }
+ private func performFlush() async {
+ guard !buffer.isEmpty else { return }
- if !buffer.isEmpty {
- let snapshot = buffer
- let snapshotOrder = order
- buffer.removeAll(keepingCapacity: true)
- order.removeAll(keepingCapacity: true)
- lastCell = nil
-
- let persistedMoves = persistAndAssignLamports(
- snapshot: snapshot,
- order: snapshotOrder
- )
- pendingSinkMoves.append(contentsOf: persistedMoves)
- }
-
- guard runAfterFlush, !pendingSinkMoves.isEmpty else { return }
- let moves = pendingSinkMoves
- pendingSinkMoves.removeAll(keepingCapacity: true)
+ let snapshot = buffer
+ let snapshotOrder = order
+ buffer.removeAll(keepingCapacity: true)
+ order.removeAll(keepingCapacity: true)
+ lastCell = nil
+ let moves = persistAndAssignLamports(
+ snapshot: snapshot,
+ order: snapshotOrder
+ )
+ guard !moves.isEmpty else { return }
await sink(moves)
if let afterFlush {
await afterFlush(Set(moves.map { $0.gameID }))
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -162,8 +162,8 @@ actor SyncEngine {
}
/// Re-enqueues locally persisted moves that do not yet have CloudKit
- /// system fields. This closes the small crash window introduced by
- /// deferring CKSyncEngine enqueue until the trailing edit flush.
+ /// system fields. This covers crash/relaunch recovery and any save that
+ /// reached Core Data before CKSyncEngine recorded the pending change.
@discardableResult
func enqueueUnconfirmedMoves() -> Int {
let ctx = persistence.container.newBackgroundContext()
diff --git a/Tests/Unit/GameStoreSnapshotPruningTests.swift b/Tests/Unit/GameStoreSnapshotPruningTests.swift
@@ -75,8 +75,57 @@ struct GameStoreSnapshotPruningTests {
#expect(fetchMoveNames(store: store, gameID: gameID).count == 1)
}
+ @Test("Shared games do not create scalar-Lamport snapshots")
+ func sharedGamesDoNotCreateSnapshots() async throws {
+ let (store, gameID) = try makeStoreWithCompletedGame(
+ moveCount: 2,
+ configure: { game in
+ game.databaseScope = 1
+ }
+ )
+
+ let result = await store.createSnapshotsIfNeeded(for: [gameID])
+ store.persistence.viewContext.refreshAllObjects()
+
+ #expect(result.snapshotNames.isEmpty)
+ #expect(result.prunedMoveNames.isEmpty)
+ #expect(fetchMoveNames(store: store, gameID: gameID).count == 2)
+ #expect(fetchPendingPruneSnapshotNames(store: store).isEmpty)
+ }
+
+ @Test("Shared games do not prune moves from existing scalar snapshots")
+ func sharedGamesDoNotPruneExistingSnapshots() throws {
+ let (store, gameID) = try makeStoreWithCompletedGame(
+ moveCount: 1,
+ configure: { game in
+ game.ckShareRecordName = "share-test"
+ }
+ )
+ let context = store.persistence.viewContext
+ let game = try #require(try fetchGame(store: store, gameID: gameID))
+ let snapshot = SnapshotEntity(context: context)
+ snapshot.game = game
+ snapshot.ckRecordName = RecordSerializer.recordName(
+ forSnapshotInGame: gameID,
+ upToLamport: 1
+ )
+ snapshot.ckSystemFields = Data([1])
+ snapshot.createdAt = Date()
+ snapshot.gridState = try MoveLog.encodeGridState([:])
+ snapshot.upToLamport = 1
+ snapshot.needsPruning = true
+ try context.save()
+
+ let pruned = store.pruneMoves()
+
+ #expect(pruned.isEmpty)
+ #expect(fetchMoveNames(store: store, gameID: gameID).count == 1)
+ #expect(fetchPendingPruneSnapshotNames(store: store).isEmpty)
+ }
+
private func makeStoreWithCompletedGame(
- moveCount: Int
+ moveCount: Int,
+ configure: (GameEntity) -> Void = { _ in }
) throws -> (GameStore, UUID) {
let persistence = makeTestPersistence()
let store = GameStore(persistence: persistence)
@@ -91,6 +140,7 @@ struct GameStoreSnapshotPruningTests {
game.completedAt = Date()
game.ckRecordName = "game-\(gameID.uuidString)"
game.lamportHighWater = Int64(moveCount)
+ configure(game)
for lamport in 1...moveCount {
let move = MoveEntity(context: context)
diff --git a/Tests/Unit/MoveBufferTests.swift b/Tests/Unit/MoveBufferTests.swift
@@ -57,7 +57,7 @@ struct MoveBufferTests {
#expect(moves.first?.lamport == 1)
}
- @Test("Enqueuing a different cell persists the previous cell before CloudKit enqueue")
+ @Test("Enqueuing a different cell persists and enqueues the previous cell")
func cellChangeFlushesPrevious() async throws {
// A long debounce makes this test insensitive to timer jitter:
// only the cell-change trigger (and the final explicit flush) can
@@ -73,10 +73,9 @@ struct MoveBufferTests {
await buffer.enqueue(gameID: gameID, row: 0, col: 0, letter: "A", markKind: 0, checkedWrong: false, authorID: nil)
await buffer.enqueue(gameID: gameID, row: 0, col: 1, letter: "B", markKind: 0, checkedWrong: false, authorID: nil)
- // The cell-change flush allocates Lamport 1 and writes the prior
- // cell durably, but CloudKit enqueue is deferred until the explicit
- // or trailing flush.
- #expect(await capture.flushCount == 0)
+ // The cell-change flush allocates Lamport 1, writes the prior cell
+ // durably, and immediately hands it to the sink.
+ #expect(await capture.flushCount == 1)
let persistedBeforeFinalFlush = fetchMoveValues(gameID: gameID, persistence: persistence)
#expect(persistedBeforeFinalFlush.count == 1)
#expect(persistedBeforeFinalFlush.first?.letter == "A")
@@ -85,9 +84,9 @@ struct MoveBufferTests {
await buffer.flush()
let flushes = await capture.flushes
- #expect(flushes.count == 1)
- #expect(flushes.first?.map(\.letter) == ["A", "B"])
- #expect(flushes.first?.map(\.lamport) == [1, 2])
+ #expect(flushes.count == 2)
+ #expect(flushes.map { $0.map(\.letter) } == [["A"], ["B"]])
+ #expect(flushes.map { $0.map(\.lamport) } == [[1], [2]])
}
@Test("Lamports are allocated from GameEntity.lamportHighWater and bump it")
