crossmate

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

commit aeb3e878ffca33755491e42f6fdfce6d7b5d697d
parent 23e50ac6bfecf87c6c59138b2d435684a9532ec1
Author: Michael Camilleri <[email protected]>
Date:   Sat,  9 May 2026 12:01:30 +0900

Load puzzles before roster refresh

Opening a shared puzzle could wait on CloudKit-backed roster work before
rendering the game, leaving the destination blank for a long time. This commit
renders the puzzle as soon as local game state is loaded, moves roster and
sharing setup into cancellable follow-up work, and shows waiting player rows
until game-specific Player records arrive.

Co-Authored-By: Codex GPT 5.5 <[email protected]>

Diffstat:
MCrossmate/CrossmateApp.swift | 50++++++++++++++++++++++++++++++++++++++------------
MCrossmate/Models/GamePlayerColorStore.swift | 37+++++++++++++++++++++++++------------
MCrossmate/Models/PlayerRoster.swift | 30+++++++-----------------------
MTests/Unit/GamePlayerColorStoreTests.swift | 20++++++++++++++++++++
MTests/Unit/PlayerRosterTests.swift | 6+++---
5 files changed, 93 insertions(+), 50 deletions(-)

diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift @@ -318,6 +318,7 @@ private struct PuzzleDisplayView: View { @State private var session: PlayerSession? @State private var roster: PlayerRoster? @State private var loadError: String? + @State private var openPuzzleFollowUpTask: Task<Void, Never>? var body: some View { Group { @@ -336,6 +337,9 @@ private struct PuzzleDisplayView: View { systemImage: "exclamationmark.triangle", description: Text(loadError) ) + } else { + ProgressView("Loading puzzle...") + .frame(maxWidth: .infinity, maxHeight: .infinity) } } .navigationTitle("") @@ -345,6 +349,8 @@ private struct PuzzleDisplayView: View { await pollOpenSharedPuzzle() } .task(id: gameID) { + openPuzzleFollowUpTask?.cancel() + openPuzzleFollowUpTask = nil session = nil roster = nil loadError = nil @@ -354,18 +360,13 @@ private struct PuzzleDisplayView: View { let newSession = PlayerSession(game: game, mutator: mutator) let newRoster = services.makePlayerRoster(for: gameID, preferences: preferences) roster = newRoster - await newRoster.refresh() session = newSession - if mutator.isShared && preferences.isICloudSyncEnabled { - services.syncMonitor.note( - "PuzzleDisplay[\(gameID.uuidString.prefix(8))]: loaded shared roster" + openPuzzleFollowUpTask = Task { @MainActor in + await finishOpeningPuzzle( + session: newSession, + roster: newRoster, + isShared: mutator.isShared ) - await activateSharing(for: newSession) - } else { - services.syncMonitor.note( - "PuzzleDisplay[\(gameID.uuidString.prefix(8))]: loaded local roster" - ) - await services.presencePublisher.clear() } } catch { loadError = String(describing: error) @@ -385,6 +386,8 @@ private struct PuzzleDisplayView: View { updateActiveNotificationPuzzleID(for: newPhase) } .onDisappear { + openPuzzleFollowUpTask?.cancel() + openPuzzleFollowUpTask = nil NotificationState.clearActivePuzzleID(if: gameID) let presence = services.presencePublisher let movesUpdater = services.movesUpdater @@ -397,6 +400,27 @@ private struct PuzzleDisplayView: View { } } + private func finishOpeningPuzzle( + session loadedSession: PlayerSession, + roster loadedRoster: PlayerRoster, + isShared: Bool + ) async { + await loadedRoster.refresh() + guard !Task.isCancelled, session === loadedSession else { return } + + if isShared && preferences.isICloudSyncEnabled { + services.syncMonitor.note( + "PuzzleDisplay[\(gameID.uuidString.prefix(8))]: loaded shared roster" + ) + await activateSharing(for: loadedSession, refreshRoster: false) + } else { + services.syncMonitor.note( + "PuzzleDisplay[\(gameID.uuidString.prefix(8))]: loaded local roster" + ) + await services.presencePublisher.clear() + } + } + private func updateActiveNotificationPuzzleID(for phase: ScenePhase) { if phase == .active { NotificationState.setActivePuzzleID(gameID) @@ -408,7 +432,7 @@ private struct PuzzleDisplayView: View { /// Initialises shared-game state (roster, presence, name broadcast) for /// the open session. Called when the puzzle first appears as shared, and /// again if a previously-solo game becomes shared mid-session. - private func activateSharing(for session: PlayerSession) async { + private func activateSharing(for session: PlayerSession, refreshRoster: Bool = true) async { Task { await AppDelegate.requestNotificationAuthorizationIfNeeded() } let activeRoster: PlayerRoster if let roster { @@ -418,7 +442,9 @@ private struct PuzzleDisplayView: View { roster = newRoster activeRoster = newRoster } - await activeRoster.refresh() + if refreshRoster { + await activeRoster.refresh() + } // 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 diff --git a/Crossmate/Models/GamePlayerColorStore.swift b/Crossmate/Models/GamePlayerColorStore.swift @@ -61,7 +61,7 @@ final class GamePlayerColorStore { // Palette exhausted — fall back to any palette colour. chosen = PlayerColor.palette.randomElement(using: &rng) ?? .blue } - setColor(chosen, forGame: gameID, authorID: authorID) + writeColor(chosen, forGame: gameID, authorID: authorID, notify: false) return chosen } @@ -83,31 +83,44 @@ final class GamePlayerColorStore { // MARK: - Write func setColor(_ color: PlayerColor, forGame gameID: UUID, authorID: String) { + writeColor(color, forGame: gameID, authorID: authorID, notify: true) + } + + func clearColor(forGame gameID: UUID, authorID: String, notify: Bool = true) { + var s = rawStore + guard s[gameID.uuidString]?[authorID] != nil else { return } + s[gameID.uuidString]?.removeValue(forKey: authorID) + if s[gameID.uuidString]?.isEmpty == true { + s.removeValue(forKey: gameID.uuidString) + } + rawStore = s + if notify { + postChange(gameID: gameID) + } + } + + private func writeColor(_ color: PlayerColor, forGame gameID: UUID, authorID: String, notify: Bool) { + if self.color(forGame: gameID, authorID: authorID)?.id == color.id { + return + } var s = rawStore var gameMap = s[gameID.uuidString] ?? [:] gameMap[authorID] = color.id s[gameID.uuidString] = gameMap rawStore = s - postChange(gameID: gameID) + if notify { + postChange(gameID: gameID) + } } func clearColors(forGame gameID: UUID) { var s = rawStore + guard s[gameID.uuidString] != nil else { return } s.removeValue(forKey: gameID.uuidString) rawStore = s postChange(gameID: gameID) } - func clearColor(forGame gameID: UUID, authorID: String) { - var s = rawStore - s[gameID.uuidString]?.removeValue(forKey: authorID) - if s[gameID.uuidString]?.isEmpty == true { - s.removeValue(forKey: gameID.uuidString) - } - rawStore = s - postChange(gameID: gameID) - } - // MARK: - Private private var rawStore: [String: [String: String]] { diff --git a/Crossmate/Models/PlayerRoster.swift b/Crossmate/Models/PlayerRoster.swift @@ -254,7 +254,7 @@ final class PlayerRoster { var remoteEntries: [Entry] = [] let reservedColorIDs: Set<String> = [preferences.color.id] for authorID in otherAuthorIDs.sorted() { - let name = resolveName(authorID: authorID, namesMap: namesMap, share: share) + let name = resolveName(authorID: authorID, namesMap: namesMap) let color = colorStore.ensureColor( forGame: gameID, authorID: authorID, @@ -269,7 +269,7 @@ final class PlayerRoster { // Garbage-collect stale colour entries. let currentRemoteIDs = Set(remoteEntries.map { $0.authorID }) for staleID in colorStore.storedAuthorIDs(forGame: gameID).subtracting(currentRemoteIDs) { - colorStore.clearColor(forGame: gameID, authorID: staleID) + colorStore.clearColor(forGame: gameID, authorID: staleID, notify: false) } let localEntry = Entry( @@ -352,27 +352,11 @@ final class PlayerRoster { return nil } - private func resolveName( - authorID: String, - namesMap: [String: String], - share: CKShare? - ) -> String { - // (a) PlayerEntity for this (game, author) + private func resolveName(authorID: String, namesMap: [String: String]) -> String { + // The game-specific Player record is authoritative for display names. + // CKShare metadata can arrive earlier, but it may expose an unrelated + // contact/iCloud name and then visibly rename the row a moment later. if let name = namesMap[authorID], !name.isEmpty { return name } - // (b–c) CKShare participant name / email - if let share, - let participant = share.participants.first(where: { - $0.userIdentity.userRecordID?.recordName == authorID - }) { - if let components = participant.userIdentity.nameComponents { - let formatted = PersonNameComponentsFormatter().string(from: components) - if !formatted.isEmpty { return formatted } - } - if let email = participant.userIdentity.lookupInfo?.emailAddress { - return email - } - } - // (d) Fallback - return "Player" + return "Waiting for player..." } } diff --git a/Tests/Unit/GamePlayerColorStoreTests.swift b/Tests/Unit/GamePlayerColorStoreTests.swift @@ -74,6 +74,26 @@ struct GamePlayerColorStoreTests { #expect(PlayerColor.palette.contains(fallback)) } + @Test("ensureColor persists without posting a change notification") + func ensureColorDoesNotPostChangeNotification() { + let store = makeStore() + let gameID = UUID() + var notificationCount = 0 + let observer = NotificationCenter.default.addObserver( + forName: .gamePlayerColorsChanged, + object: nil, + queue: nil + ) { note in + guard let id = note.userInfo?["gameID"] as? UUID, id == gameID else { return } + notificationCount += 1 + } + defer { NotificationCenter.default.removeObserver(observer) } + + _ = store.ensureColor(forGame: gameID, authorID: "_A", reservedColorIDs: []) + + #expect(notificationCount == 0) + } + // MARK: - setColor / color round-trip @Test("setColor and color round-trip correctly") diff --git a/Tests/Unit/PlayerRosterTests.swift b/Tests/Unit/PlayerRosterTests.swift @@ -157,8 +157,8 @@ struct PlayerRosterTests { #expect(remote?.name == "Alice") } - @Test("Entry name falls back to 'Player' when no PlayerEntity and no share") - func entryNameFallsBackToPlayer() async throws { + @Test("Entry name waits for PlayerEntity when no game-specific name has arrived") + func entryNameWaitsForPlayerEntity() async throws { let (persistence, gameID) = try makePersistenceWithGame() addMoves(authorIDs: ["_B"], gameID: gameID, persistence: persistence) // No PlayerEntity for "_B". @@ -167,7 +167,7 @@ struct PlayerRosterTests { await roster.refresh() let remote = roster.entries.first { !$0.isLocal } - #expect(remote?.name == "Player") + #expect(remote?.name == "Waiting for player...") } @Test("Current user placeholder is not shown as a remote player")