commit 34685d68a58e23489cf2c229a6f8378897e56a1c
parent dbc7c471e765c111d5e9c376c307c8575307e794
Author: Michael Camilleri <[email protected]>
Date: Fri, 5 Jun 2026 00:30:20 +0900
Refresh the roster on new contributors only
A co-solving flurry felt choppy because the player roster — and with it
the whole grid view — was rebuilt on every peer keystroke. The roster
refresh was posted from BatchEffects.affected, the union of every Game,
Moves, Player, and deletion applied in a batch, so each inbound Moves
record (a collaborator's letter) tripped .playerRosterShouldRefresh,
which runs PlayerRoster.refresh() and re-evaluates the GridView body. In
a flurry those Moves land at typing cadence.
The set is renamed to rosterRelevant and the Moves case no longer feeds
it unconditionally. But the roster does depend on Moves in one case: it
fetches MovesEntity author IDs and surfaces a contributor who has not yet
published a Player record as 'Waiting for player...', so a new
collaborator whose first move arrives before their Player record must
still trigger a refresh. applyMovesRecord now reports that precise signal
through a new onNewAuthor closure — it fires only when the record is the
first MovesEntity row for a remote author in that game, checked off the
new-row path so a repeat move (foundExisting) and a known author's second
device (an existing row under a different record name) both stay silent.
The appliers mark such a game rosterRelevant.
The result: the roster refreshes when a new contributor first appears and
on every Player, Game, and deletion change, but a repeat move from a
known author — the overwhelming majority during a flurry — no longer
touches the roster or the grid view.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
5 files changed, 164 insertions(+), 23 deletions(-)
diff --git a/Crossmate/Models/PlayerRoster.swift b/Crossmate/Models/PlayerRoster.swift
@@ -133,9 +133,13 @@ final class PlayerRoster {
private func startObserving() {
let gameID = self.gameID
- // Remote record changes affecting this game — name records arriving
- // from other participants, new participants joining (move records
- // carry a fresh authorID), share metadata changes. Invalidate the
+ // Roster-relevant remote changes for this game — a peer's Player
+ // record (name / cursor), a Game record, a deletion, or a new
+ // contributor's first Moves row (see `BatchEffects.rosterRelevant`).
+ // `refresh()` discovers participants from PlayerEntity, the CKShare,
+ // and Moves authorIDs, so a brand-new collaborator surfaces here even
+ // before their Player record arrives; repeat moves from a known author
+ // don't post, since they add nothing to the roster. Invalidate the
// cached share so the next refresh re-fetches it; participant lists
// may have moved.
observationTasks.append(
diff --git a/Crossmate/Sync/RecordApplier.swift b/Crossmate/Sync/RecordApplier.swift
@@ -4,7 +4,16 @@ import Foundation
struct BatchEffects {
var movesUpdated = Set<UUID>()
- var affected = Set<UUID>()
+ /// Games whose roster-relevant state changed in this batch — a Player
+ /// record (name / selection / readAt), a Game record (share metadata), a
+ /// deletion (participant removal), or a *new* contributor's first Moves
+ /// row (a participant the roster can only discover from their moves; see
+ /// `applyMovesRecord`'s `onNewAuthor`). Drives `.playerRosterShouldRefresh`.
+ /// A repeat Moves row from a known contributor is excluded: it changes the
+ /// grid, not the roster, and during a co-solve flurry those land at
+ /// keystroke cadence — refreshing the roster (and re-evaluating the grid
+ /// view) on each one was pure overhead.
+ var rosterRelevant = Set<UUID>()
var pings: [Ping] = []
var playersUpdated = Set<UUID>()
var playerPresenceChanged = Set<UUID>()
@@ -44,17 +53,17 @@ extension SyncEngine {
onEngagementChange: { effects.engagementChanged.insert($0) },
onCompletedTransition: { effects.completedTransitions.insert($0) }
)
- if let id = entity.id { effects.affected.insert(id) }
+ if let id = entity.id { effects.rosterRelevant.insert(id) }
case "Moves":
if let value = RecordSerializer.parseMovesRecord(record) {
let cellsChanged = RecordSerializer.applyMovesRecord(
record,
value: value,
to: ctx,
- localAuthorID: localAuthorID
+ localAuthorID: localAuthorID,
+ onNewAuthor: { _ in effects.rosterRelevant.insert(value.gameID) }
)
if cellsChanged { effects.movesUpdated.insert(value.gameID) }
- effects.affected.insert(value.gameID)
}
case "Player":
if let (gameID, _) = RecordSerializer.parsePlayerRecordName(record.recordID.recordName) {
@@ -66,7 +75,7 @@ extension SyncEngine {
onPresenceChange: { effects.playerPresenceChanged.insert($0) },
onReadCursor: { effects.readCursors.append(($0, $1)) }
)
- effects.affected.insert(gameID)
+ effects.rosterRelevant.insert(gameID)
}
default:
break
@@ -79,7 +88,7 @@ extension SyncEngine {
in: ctx
)
if let id = self.gameID(fromRecordName: deletion.0.recordName) {
- effects.affected.insert(id)
+ effects.rosterRelevant.insert(id)
}
}
for gameID in effects.movesUpdated {
@@ -130,11 +139,11 @@ extension SyncEngine {
if let onPingDeleted, !pingDeletedGameIDs.isEmpty {
await onPingDeleted(pingDeletedGameIDs)
}
- if !effects.affected.isEmpty {
+ if !effects.rosterRelevant.isEmpty {
NotificationCenter.default.post(
name: .playerRosterShouldRefresh,
object: nil,
- userInfo: ["gameIDs": effects.affected]
+ userInfo: ["gameIDs": effects.rosterRelevant]
)
}
}
diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift
@@ -649,12 +649,20 @@ enum RecordSerializer {
/// record is fresher. Returns `true` when cells/updatedAt were adopted,
/// `false` when the local-device-row guard short-circuited the body so
/// callers can skip the downstream grid refresh.
+ ///
+ /// `onNewAuthor` fires with the authorID when this record creates the first
+ /// `MovesEntity` row by a *remote* contributor — i.e. a participant the
+ /// roster can only discover from their moves (no `Player` record yet, see
+ /// `PlayerRoster.refresh`). Callers use it to trigger a one-off roster
+ /// refresh on a new collaborator's first move without refreshing on every
+ /// subsequent keystroke or sibling-device row.
@discardableResult
static func applyMovesRecord(
_ record: CKRecord,
value: MovesValue,
to ctx: NSManagedObjectContext,
- localAuthorID: String? = nil
+ localAuthorID: String? = nil,
+ onNewAuthor: ((String) -> Void)? = nil
) -> Bool {
let ckName = record.recordID.recordName
let req = NSFetchRequest<MovesEntity>(entityName: "MovesEntity")
@@ -663,10 +671,21 @@ enum RecordSerializer {
let entity: MovesEntity
let foundExisting: Bool
+ let authorAlreadyKnown: Bool
if let existing = try? ctx.fetch(req).first {
entity = existing
foundExisting = true
+ authorAlreadyKnown = true
} else {
+ let authorReq = NSFetchRequest<MovesEntity>(entityName: "MovesEntity")
+ authorReq.predicate = NSPredicate(
+ format: "game.id == %@ AND authorID == %@",
+ value.gameID as CVarArg,
+ value.authorID
+ )
+ authorReq.fetchLimit = 1
+ authorAlreadyKnown = ((try? ctx.fetch(authorReq).first) != nil)
+
let game = ensureGameEntity(
forGameID: value.gameID,
zoneID: record.recordID.zoneID,
@@ -718,6 +737,17 @@ enum RecordSerializer {
game.updatedAt.map({ $0 < mergedUpdatedAt }) ?? true {
game.updatedAt = mergedUpdatedAt
}
+ // A newly-seen remote author is the roster's only cue to
+ // a contributor who hasn't published a `Player` record yet. Signal it
+ // once here; repeat moves and sibling-device rows by a known author
+ // don't.
+ if !foundExisting,
+ !authorAlreadyKnown,
+ value.authorID != localAuthorID,
+ value.authorID != CKCurrentUserDefaultName,
+ !value.authorID.isEmpty {
+ onNewAuthor?(value.authorID)
+ }
return true
}
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -19,10 +19,13 @@ extension EnvironmentValues {
}
extension Notification.Name {
- /// Posted by `SyncEngine` after applying fetched record zone changes
- /// that affected one or more games. `userInfo["gameIDs"]` is a
- /// `Set<UUID>`. `PlayerRoster` observes this to refresh in response
- /// to remote name updates and new participants joining.
+ /// Posted by `SyncEngine` after applying fetched record zone changes that
+ /// touched a game's roster-relevant state — a Player record, a Game record,
+ /// a deletion, or a new contributor's first Moves row (see
+ /// `BatchEffects.rosterRelevant`). `userInfo["gameIDs"]` is a `Set<UUID>`.
+ /// `PlayerRoster` observes this to refresh in response to remote name /
+ /// cursor updates and new participants joining. Repeat Moves from a known
+ /// contributor (peer letters) are excluded — they don't change the roster.
static let playerRosterShouldRefresh = Notification.Name("playerRosterShouldRefresh")
/// Posted by `SyncEngine` when an inbound `Journal` record lands for one or
@@ -1223,17 +1226,17 @@ actor SyncEngine {
onEngagementChange: { effects.engagementChanged.insert($0) },
onCompletedTransition: { effects.completedTransitions.insert($0) }
)
- if let id = entity.id { effects.affected.insert(id) }
+ if let id = entity.id { effects.rosterRelevant.insert(id) }
case "Moves":
if let value = RecordSerializer.parseMovesRecord(record) {
let cellsChanged = RecordSerializer.applyMovesRecord(
record,
value: value,
to: ctx,
- localAuthorID: localAuthorID
+ localAuthorID: localAuthorID,
+ onNewAuthor: { _ in effects.rosterRelevant.insert(value.gameID) }
)
if cellsChanged { effects.movesUpdated.insert(value.gameID) }
- effects.affected.insert(value.gameID)
}
case "Player":
if let (gameID, _) = RecordSerializer.parsePlayerRecordName(record.recordID.recordName) {
@@ -1245,7 +1248,7 @@ actor SyncEngine {
onPresenceChange: { effects.playerPresenceChanged.insert($0) },
onReadCursor: { effects.readCursors.append(($0, $1)) }
)
- effects.affected.insert(gameID)
+ effects.rosterRelevant.insert(gameID)
}
case "Ping":
if let ping = Ping.parseRecord(record) {
@@ -1293,7 +1296,7 @@ actor SyncEngine {
in: ctx
)
if let id = self.gameID(fromRecordName: deletion.recordID.recordName) {
- effects.affected.insert(id)
+ effects.rosterRelevant.insert(id)
}
}
for gameID in effects.movesUpdated {
@@ -1361,11 +1364,11 @@ actor SyncEngine {
if let onPingDeleted, !pingDeletedGameIDs.isEmpty {
await onPingDeleted(pingDeletedGameIDs)
}
- if !effects.affected.isEmpty {
+ if !effects.rosterRelevant.isEmpty {
NotificationCenter.default.post(
name: .playerRosterShouldRefresh,
object: nil,
- userInfo: ["gameIDs": effects.affected]
+ userInfo: ["gameIDs": effects.rosterRelevant]
)
}
if !effects.journalsSynced.isEmpty {
diff --git a/Tests/Unit/Sync/MovesInboundTests.swift b/Tests/Unit/Sync/MovesInboundTests.swift
@@ -126,6 +126,101 @@ struct MovesInboundTests {
#expect(decoded == cells2)
}
+ @Test("onNewAuthor fires once for a remote contributor's first row, not repeats")
+ func onNewAuthorFiresForNewRemoteContributor() throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let cell: [GridPosition: TimestampedCell] = [
+ GridPosition(row: 0, col: 0): TimestampedCell(
+ letter: "A", mark: .none,
+ updatedAt: Date(timeIntervalSince1970: 1_700_000_000),
+ authorID: "bob"
+ ),
+ ]
+ let (rec, value) = try record(
+ in: ctx, authorID: "bob", deviceID: "d1",
+ cells: cell, updatedAt: Date(timeIntervalSince1970: 1_700_000_000)
+ )
+
+ var newAuthors: [String] = []
+ RecordSerializer.applyMovesRecord(
+ rec, value: value, to: ctx, localAuthorID: "alice",
+ onNewAuthor: { newAuthors.append($0) }
+ )
+ #expect(newAuthors == ["bob"])
+
+ // A later update lands on the same row (ckRecordName), so it is not a
+ // new contributor and must not re-trigger the roster.
+ let (rec2, value2) = try record(
+ in: ctx, authorID: "bob", deviceID: "d1",
+ cells: cell, updatedAt: Date(timeIntervalSince1970: 1_700_000_500)
+ )
+ RecordSerializer.applyMovesRecord(
+ rec2, value: value2, to: ctx, localAuthorID: "alice",
+ onNewAuthor: { newAuthors.append($0) }
+ )
+ #expect(newAuthors == ["bob"])
+ }
+
+ @Test("onNewAuthor does not fire for a known contributor's second device")
+ func onNewAuthorSkipsKnownAuthorSecondDevice() throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let cell: [GridPosition: TimestampedCell] = [
+ GridPosition(row: 0, col: 0): TimestampedCell(
+ letter: "A", mark: .none,
+ updatedAt: Date(timeIntervalSince1970: 1_700_000_000),
+ authorID: "bob"
+ ),
+ ]
+ let (phoneRecord, phoneValue) = try record(
+ in: ctx, authorID: "bob", deviceID: "phone",
+ cells: cell, updatedAt: Date(timeIntervalSince1970: 1_700_000_000)
+ )
+ let (ipadRecord, ipadValue) = try record(
+ in: ctx, authorID: "bob", deviceID: "ipad",
+ cells: cell, updatedAt: Date(timeIntervalSince1970: 1_700_000_500)
+ )
+
+ var newAuthors: [String] = []
+ RecordSerializer.applyMovesRecord(
+ phoneRecord, value: phoneValue, to: ctx, localAuthorID: "alice",
+ onNewAuthor: { newAuthors.append($0) }
+ )
+ RecordSerializer.applyMovesRecord(
+ ipadRecord, value: ipadValue, to: ctx, localAuthorID: "alice",
+ onNewAuthor: { newAuthors.append($0) }
+ )
+
+ #expect(newAuthors == ["bob"])
+ }
+
+ @Test("onNewAuthor does not fire for the local author's own moves")
+ func onNewAuthorSkipsLocalAuthor() throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let cell: [GridPosition: TimestampedCell] = [
+ GridPosition(row: 0, col: 0): TimestampedCell(
+ letter: "A", mark: .none,
+ updatedAt: Date(timeIntervalSince1970: 1_700_000_000),
+ authorID: "alice"
+ ),
+ ]
+ // A sibling device of the local author (same authorID, new row) is not
+ // a new participant.
+ let (rec, value) = try record(
+ in: ctx, authorID: "alice", deviceID: "sibling-device",
+ cells: cell, updatedAt: Date(timeIntervalSince1970: 1_700_000_000)
+ )
+
+ var newAuthors: [String] = []
+ RecordSerializer.applyMovesRecord(
+ rec, value: value, to: ctx, localAuthorID: "alice",
+ onNewAuthor: { newAuthors.append($0) }
+ )
+ #expect(newAuthors.isEmpty)
+ }
+
@Test("Inbound local-device record does not clobber existing local row")
func inboundLocalDeviceRecordDoesNotClobberExistingLocalRow() throws {
let persistence = makeTestPersistence()
