commit 96cacd24bd851bd39908c7d8aebabca08528df86
parent a9d26c566e81cf738bda23084d3e5869bb620411
Author: Michael Camilleri <[email protected]>
Date: Tue, 16 Jun 2026 11:58:54 +0900
Ignore checks when highlighting changes on rejoin
When the user reopened a shared game, the highlight that marks cells a
peer changed while they were away could light up the whole board —
including squares the user had filled themselves. A Check sweep records
a fresh move on every filled cell, bumping its updatedAt without
touching the letter, and the highlight keyed purely off that timestamp,
so a partner checking the puzzle read as having changed every cell.
This commit narrows the highlight to genuine letter changes. It compares
each cell's letter as it stood at the cutoff — the player's last view —
against its current letter, so a mark-only move such as a check no
longer qualifies. Fills, edits and clears still do.
Attribution now follows the letter rather than the last touch. The
winning move preserves the original typist on TimestampedCell.authorID
even when a later check overwrites the mark, so a peer's check over
another player's answer credits the typist, not the checker — and a peer
checking the user's own letter resolves to the user and drops out. A
clear carries no preserved author, so an emptied cell falls back to the
writer of the clearing move.
The rejoin border is also drawn a point thinner, no longer borrowing the
weight of the related-cell outline it sat alongside.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
3 files changed, 112 insertions(+), 18 deletions(-)
diff --git a/Crossmate/Sync/RecentChanges.swift b/Crossmate/Sync/RecentChanges.swift
@@ -1,30 +1,49 @@
import Foundation
-/// Computes which grid cells changed since a cutoff, attributed to the player
-/// who wrote each change. Pure so it runs off the already-merged moves without
-/// touching Core Data and is unit-testable in isolation.
+/// Computes which grid cells had their *letter* changed since a cutoff,
+/// attributed to the player who wrote each change. Pure so it runs off the
+/// already-merged moves without touching Core Data and is unit-testable in
+/// isolation.
///
-/// Fills *and* clears qualify. A clear drops the cell's preserved letter author
-/// (`TimestampedCell.authorID` becomes `nil`), so the squares grid can't say who
-/// emptied a cell — but the writing user is still recorded on the winning move,
-/// which is what `GridStateMerger.mergeWithProvenance` surfaces as
-/// `writerAuthorID`. That's the attribution this returns.
+/// Fills, edits, *and* clears qualify — anything that altered what the cell
+/// reads. Mark-only moves do not: a check sweep re-stamps every filled cell
+/// with a fresh winning move (newer `updatedAt`, same letter, only the
+/// check/pencil mark changed), and those must not light up the whole board on
+/// rejoin. We therefore compare the cell's letter as it stood at the cutoff
+/// against its current letter, rather than trusting the timestamp alone.
+///
+/// Each flagged cell is attributed to the player who set its current letter,
+/// not whoever touched it last. The winning move carries that author on
+/// `TimestampedCell.authorID` — a check preserves it, so a peer's check sweep
+/// over someone else's answer still credits the original typist. A clear drops
+/// the preserved author to `nil`, so for an emptied cell we fall back to the
+/// winning move's `writerAuthorID` (whoever did the clearing), which is what
+/// `GridStateMerger.mergeWithProvenance` surfaces.
enum RecentChanges {
- /// Positions whose winning move landed strictly after `since` and was
- /// written by someone other than `excluded`, each mapped to that writer's
- /// `authorID`. Last-writer-wins (inside `mergeWithProvenance`) decides the
- /// winning touch per cell, so a cell reverted back to its old value by a
- /// later move is attributed to whoever made that later move.
+ /// Positions whose winning move landed strictly after `since` and changed
+ /// the cell's letter, each mapped to the author who wrote that letter
+ /// (`excluded` — typically the local user — is dropped). Last-writer-wins
+ /// (inside `mergeWithProvenance`) decides the winning touch per cell, so a
+ /// cell reverted back to its old value by a later move reflects that later
+ /// move.
static func changedCells(
in moves: [MovesValue],
since: Date,
excludingAuthor excluded: String
) -> [GridPosition: String] {
+ // The grid as it stood at the cutoff, so a mark-only move (a check) can
+ // be told apart from a genuine letter change by comparing letters.
+ let asOfCutoff = GridStateMerger.merge(moves, notAfter: since)
var result: [GridPosition: String] = [:]
for (position, provenance) in GridStateMerger.mergeWithProvenance(moves) {
guard provenance.cell.updatedAt > since else { continue }
- guard provenance.writerAuthorID != excluded else { continue }
- result[position] = provenance.writerAuthorID
+ let priorLetter = asOfCutoff[position]?.letter ?? ""
+ guard priorLetter != provenance.cell.letter else { continue }
+ // The letter's author, preserved through later mark-only moves; a
+ // cleared cell carries none, so credit whoever cleared it.
+ let writer = provenance.cell.authorID ?? provenance.writerAuthorID
+ guard writer != excluded else { continue }
+ result[position] = writer
}
return result
}
diff --git a/Crossmate/Views/Puzzle/GridView.swift b/Crossmate/Views/Puzzle/GridView.swift
@@ -488,9 +488,9 @@ private struct RecentChangeBorders: View {
)
for (pos, color) in shown {
// Inset by half the line width so the stroke sits inside the
- // cell, matching `LocalCursorTints`' related-cell border.
- let rect = geometry.cellRect(row: pos.row, col: pos.col).insetBy(dx: 1.5, dy: 1.5)
- context.stroke(Path(rect), with: .color(color), lineWidth: 3)
+ // cell (thinner than `LocalCursorTints`' related-cell border).
+ let rect = geometry.cellRect(row: pos.row, col: pos.col).insetBy(dx: 1, dy: 1)
+ context.stroke(Path(rect), with: .color(color), lineWidth: 2)
}
}
.opacity(visible ? 1 : 0)
diff --git a/Tests/Unit/RecentChangesTests.swift b/Tests/Unit/RecentChangesTests.swift
@@ -91,4 +91,79 @@ struct RecentChangesTests {
let changes = RecentChanges.changedCells(in: [], since: cutoff, excludingAuthor: "alice")
#expect(changes.isEmpty)
}
+
+ /// A check sweep re-stamps a cell with a fresh, newer move that keeps the
+ /// same letter and only flips the mark. `cellAuthor` is the preserved letter
+ /// author (the original typist), distinct from the checking `author`.
+ private func check(
+ author: String,
+ device: String = "d1",
+ row: Int, col: Int, letter: String, cellAuthor: String,
+ updatedAt: Date
+ ) -> MovesValue {
+ let cell = TimestampedCell(
+ letter: letter,
+ mark: .pen(checked: .right),
+ updatedAt: updatedAt,
+ authorID: cellAuthor
+ )
+ return MovesValue(
+ gameID: gameID,
+ authorID: author,
+ deviceID: device,
+ cells: [GridPosition(row: row, col: col): cell],
+ updatedAt: updatedAt
+ )
+ }
+
+ @Test("A peer's check that only changes the mark is excluded")
+ func peerCheckExcluded() {
+ // Alice typed the letter before the cutoff; Bob check-sweeps it after.
+ // The letter never changed, so it must not light up on rejoin.
+ let moves = [
+ view(author: "alice", cells: [(1, 1, "A", before)]),
+ check(author: "bob", row: 1, col: 1, letter: "A", cellAuthor: "alice", updatedAt: after),
+ ]
+ let changes = RecentChanges.changedCells(in: moves, since: cutoff, excludingAuthor: "alice")
+ #expect(changes.isEmpty)
+ }
+
+ @Test("A check sweep doesn't mask a real letter change, and credits the letter writer")
+ func realChangeSurvivesLaterCheck() {
+ // Carol overwrites the cell after the cutoff, then Bob checks it. The
+ // letter did change, so the cell is flagged — and attributed to Carol,
+ // who wrote the letter, not Bob, who merely checked it.
+ let moves = [
+ view(author: "alice", cells: [(2, 3, "A", before)]),
+ view(author: "carol", cells: [(2, 3, "B", after)]),
+ check(author: "bob", row: 2, col: 3, letter: "B", cellAuthor: "carol", updatedAt: later),
+ ]
+ let changes = RecentChanges.changedCells(in: moves, since: cutoff, excludingAuthor: "alice")
+ #expect(changes == [GridPosition(row: 2, col: 3): "carol"])
+ }
+
+ @Test("A peer checking my own post-cutoff letter doesn't flag it as a change")
+ func peerCheckOfOwnLetterExcluded() {
+ // Alice (the viewer) writes the letter after the cutoff, then Bob checks
+ // it. The preserved author is Alice, so attribution resolves to her and
+ // the cell is excluded — even though Bob's check is the winning move.
+ let moves = [
+ view(author: "alice", cells: [(4, 5, "M", after)]),
+ check(author: "bob", row: 4, col: 5, letter: "M", cellAuthor: "alice", updatedAt: later),
+ ]
+ let changes = RecentChanges.changedCells(in: moves, since: cutoff, excludingAuthor: "alice")
+ #expect(changes.isEmpty)
+ }
+
+ @Test("A check on a cell first filled after the cutoff still counts as a change")
+ func fillThenCheckIncluded() {
+ // Bob fills an empty cell after the cutoff and checks it; empty -> letter
+ // is a genuine change.
+ let moves = [
+ view(author: "bob", cells: [(0, 0, "Z", after)]),
+ check(author: "bob", row: 0, col: 0, letter: "Z", cellAuthor: "bob", updatedAt: later),
+ ]
+ let changes = RecentChanges.changedCells(in: moves, since: cutoff, excludingAuthor: "alice")
+ #expect(changes == [GridPosition(row: 0, col: 0): "bob"])
+ }
}
