crossmate

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

commit 600e96e00966e881dca9e50761cbc48b2e027b45
parent f105f11ab105ea5342e4ecb52a739a931fa81153
Author: Michael Camilleri <[email protected]>
Date:   Wed, 29 Apr 2026 13:44:10 +0900

Fix name broadcasting in share puzzles

Two related bugs were producing wrong rosters in shared games. First, a
participant's display name never reached the other side until they happened to
rename themselves: NameBroadcaster only fanned out on observed changes to
PlayerPreferences.name, and nothing called broadcastName() on share creation or
acceptance, so the partner's resolveName fell through to the "Player" fallback.
PuzzleDisplayView now invokes nameBroadcaster?.broadcastName() whenever a
shared puzzle loads. The fan-out is idempotent and covers both the just-shared
and just-accepted cases in one place; nameBroadcaster on AppServices is exposed
as private(set) for that call.

Second, PresencePublisher was inserting a fresh PlayerEntity with name="" when
no row existed yet, and pushing that record up. PlayerRoster.refresh filters
empty-name rows out of namesMap, so the partner's name briefly resolved to
"Player" while the empty-string Player record was the most recent write.
PresencePublisher.begin now takes the local user's currentName, write() only
backfills the field when no row exists or its name is empty, and
SyncEngine.applyPlayerRecord refuses to overwrite a non-empty name with an
empty one in case an older build is on the other side of the wire.
PresencePublisherTests was updated for the new begin signature.

Finally, PlayerRoster.refresh emits a SyncMonitor diagnostic whenever the
inputs change — localAuthorID, namesMap keys, MoveEntity authorIDs, and the
share's participant userRecordIDs — to make it possible to identify the source
of any 'ghost' authorID that may show up in the roster.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

Diffstat:
MCrossmate/CrossmateApp.swift | 12+++++++++++-
MCrossmate/Models/PlayerRoster.swift | 21++++++++++++++++++++-
MCrossmate/Services/AppServices.swift | 7+++++--
MCrossmate/Sync/PresencePublisher.swift | 24+++++++++++++++++++++---
MCrossmate/Sync/SyncEngine.swift | 7++++++-
MTests/Unit/PresencePublisherTests.swift | 14+++++++-------
6 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift @@ -217,9 +217,19 @@ private struct PuzzleDisplayView: View { let newSession = PlayerSession(game: game, mutator: mutator) if mutator.isShared { roster = services.makePlayerRoster(for: gameID, preferences: preferences) + // Fan out the local user's name to every shared/joined + // game's zone before any presence write — otherwise the + // partner sees "Player" until we happen to rename + // ourselves and trigger NameBroadcaster's observer. + // Idempotent if the name has already been broadcast. + await services.nameBroadcaster?.broadcastName() if let authorID = services.identity.currentID { let presence = services.presencePublisher - await presence.begin(gameID: gameID, authorID: authorID) + await presence.begin( + gameID: gameID, + authorID: authorID, + currentName: preferences.name + ) newSession.onSelectionChanged = { selection in Task { await presence.publish(selection) } } diff --git a/Crossmate/Models/PlayerRoster.swift b/Crossmate/Models/PlayerRoster.swift @@ -45,9 +45,11 @@ final class PlayerRoster { private let preferences: PlayerPreferences private let persistence: PersistenceController private let container: CKContainer + private let tracer: (@MainActor @Sendable (String) -> Void)? private var cachedShare: CKShare? private var observationTasks: [Task<Void, Never>] = [] + private var lastTracedSignature: String? init( gameID: UUID, @@ -55,7 +57,8 @@ final class PlayerRoster { authorIdentity: AuthorIdentity, preferences: PlayerPreferences, persistence: PersistenceController, - container: CKContainer + container: CKContainer, + tracer: (@MainActor @Sendable (String) -> Void)? = nil ) { self.gameID = gameID self.colorStore = colorStore @@ -63,6 +66,7 @@ final class PlayerRoster { self.preferences = preferences self.persistence = persistence self.container = container + self.tracer = tracer startObserving() } @@ -207,6 +211,21 @@ final class PlayerRoster { otherAuthorIDs.insert(authorID) } + // Diagnostic — surfaces the inputs that drive the entries list so we + // can tell whether a "ghost" authorID came from a stale PlayerEntity, + // a stray MoveEntity, or the share's participant list. Trim noisy + // re-entries by only emitting when the signature actually changes. + if let tracer { + let participantIDs: [String] = share?.participants.compactMap { + $0.userIdentity.userRecordID?.recordName + } ?? [] + let signature = "local=\(localAuthorID) | names=\(namesMap.keys.sorted()) | moves=\(moveAuthorIDs.sorted()) | share=\(participantIDs.sorted())" + if signature != lastTracedSignature { + lastTracedSignature = signature + tracer("PlayerRoster[\(gameID.uuidString.prefix(8))]: \(signature)") + } + } + // Build remote entries, assigning stable colours. var remoteEntries: [Entry] = [] let reservedColorIDs: Set<String> = [preferences.color.id] diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift @@ -20,7 +20,7 @@ final class AppServices { private let ckContainer = CKContainer(identifier: "iCloud.net.inqk.crossmate.v2") private var started = false - private var nameBroadcaster: NameBroadcaster? + private(set) var nameBroadcaster: NameBroadcaster? private var isReadyForShareAcceptance = false private var isProcessingShareAcceptanceQueue = false private var pendingShareMetadatas: [CKShare.Metadata] = [] @@ -132,6 +132,8 @@ final class AppServices { await identity.refresh(using: ckContainer) // NameBroadcaster fans out name changes to all shared/joined games. + // PuzzleDisplayView also calls `broadcastName()` when a shared puzzle + // is opened, which covers first-sync-after-share-create / accept. nameBroadcaster = NameBroadcaster( preferences: preferences, persistence: persistence, @@ -192,7 +194,8 @@ final class AppServices { authorIdentity: identity, preferences: preferences, persistence: persistence, - container: ckContainer + container: ckContainer, + tracer: { [syncMonitor] message in syncMonitor.note(message) } ) } diff --git a/Crossmate/Sync/PresencePublisher.swift b/Crossmate/Sync/PresencePublisher.swift @@ -16,6 +16,12 @@ actor PresencePublisher { private var debounceTask: Task<Void, Never>? private var gameID: UUID? private var authorID: String? + /// The local user's display name at session start. Used as a fallback + /// when `write` has to insert a fresh `PlayerEntity` row before + /// `NameBroadcaster` has fanned out — without it the row would have no + /// name and the SyncEngine would refuse to build a CKRecord, dropping + /// the cursor on the floor. + private var fallbackName: String = "" init( debounceInterval: Duration = .milliseconds(300), @@ -28,10 +34,13 @@ actor PresencePublisher { } /// Starts publishing for a new puzzle session. Resets dedupe state so the - /// first selection from the new session always flushes. - func begin(gameID: UUID, authorID: String) { + /// first selection from the new session always flushes. `currentName` is + /// the local user's display name at the time the puzzle was opened — used + /// only when no PlayerEntity row exists yet for this (game, author). + func begin(gameID: UUID, authorID: String, currentName: String) { self.gameID = gameID self.authorID = authorID + fallbackName = currentName pending = nil lastPublished = nil debounceTask?.cancel() @@ -106,6 +115,7 @@ actor PresencePublisher { ) async { let context = persistence.container.newBackgroundContext() let now = Date() + let fallbackName = self.fallbackName context.performAndWait { let recordName = RecordSerializer.recordName(forPlayerInGame: gameID, authorID: authorID) let req = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity") @@ -114,6 +124,12 @@ actor PresencePublisher { let entity: PlayerEntity if let existing = try? context.fetch(req).first { entity = existing + // Don't overwrite a name that NameBroadcaster has set; only + // backfill if it's missing or empty so the outgoing record is + // never `name=""`. + if (entity.name ?? "").isEmpty, !fallbackName.isEmpty { + entity.name = fallbackName + } } else { let gameReq = NSFetchRequest<GameEntity>(entityName: "GameEntity") gameReq.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) @@ -123,7 +139,9 @@ actor PresencePublisher { entity.game = game entity.ckRecordName = recordName entity.authorID = authorID - entity.name = "" + if !fallbackName.isEmpty { + entity.name = fallbackName + } } entity.updatedAt = now if let selection { diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift @@ -544,7 +544,12 @@ actor SyncEngine { entity.ckRecordName = ckName entity.ckSystemFields = RecordSerializer.encodeSystemFields(of: record) entity.authorID = authorID - entity.name = renderedName + // An empty `name` is what older builds shipped from PresencePublisher + // before the fix; treat it as "no information" rather than letting it + // clobber a previously-resolved name. + if !renderedName.isEmpty { + entity.name = renderedName + } entity.updatedAt = updatedAt if let selection = RecordSerializer.parsePlayerSelection(from: record) { entity.selRow = NSNumber(value: Int64(selection.row)) diff --git a/Tests/Unit/PresencePublisherTests.swift b/Tests/Unit/PresencePublisherTests.swift @@ -42,7 +42,7 @@ struct PresencePublisherTests { sink: { id, author in await capture.append(id, author) } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") await publisher.publish(PlayerSelection(row: 3, col: 4, direction: .down)) await publisher.flush() @@ -65,7 +65,7 @@ struct PresencePublisherTests { sink: { id, author in await capture.append(id, author) } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") let selection = PlayerSelection(row: 0, col: 0, direction: .across) await publisher.publish(selection) try await Task.sleep(for: .milliseconds(120)) @@ -87,7 +87,7 @@ struct PresencePublisherTests { sink: { id, author in await capture.append(id, author) } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") for col in 0...4 { await publisher.publish(PlayerSelection(row: 0, col: col, direction: .across)) try await Task.sleep(for: .milliseconds(20)) @@ -110,7 +110,7 @@ struct PresencePublisherTests { sink: { id, author in await capture.append(id, author) } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") await publisher.publish(PlayerSelection(row: 1, col: 2, direction: .down)) await publisher.flush() await publisher.clear() @@ -136,7 +136,7 @@ struct PresencePublisherTests { sink: { id, author in await capture.append(id, author) } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") await publisher.clear() try await Task.sleep(for: .milliseconds(50)) @@ -182,7 +182,7 @@ struct PresencePublisherTests { persistence: persistence, sink: { _, _ in } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") await publisher.publish(PlayerSelection(row: 5, col: 6, direction: .across)) await publisher.flush() @@ -203,7 +203,7 @@ struct PresencePublisherTests { sink: { id, author in await capture.append(id, author) } ) - await publisher.begin(gameID: gameID, authorID: "alice") + await publisher.begin(gameID: gameID, authorID: "alice", currentName: "Alice") await publisher.publish(PlayerSelection(row: 1, col: 1, direction: .across)) await publisher.flush() await publisher.clear()