From a27cb29a5b9bea66be16d965d8a7372dd695480e Mon Sep 17 00:00:00 2001 From: Joey Yakimowich-Payne Date: Sun, 19 Apr 2026 09:29:07 -0600 Subject: [PATCH] docs(adr): T2 implementation retrospective --- docs/adr/modifier-profiles.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/adr/modifier-profiles.md b/docs/adr/modifier-profiles.md index 176ea47..ce8e8ef 100644 --- a/docs/adr/modifier-profiles.md +++ b/docs/adr/modifier-profiles.md @@ -456,3 +456,26 @@ Capped at 50 snapshots to bound memory. Cleared on Save because the saved state - Infinite history — memory unbounded. 50 is generous for a single editing session. - Per-modifier undo (like per-field undo in some editors) — over-granular for structured data edits. - Persist history to localStorage — adds complexity for marginal benefit; users expect editor state to reset between sessions. + +--- + +## T2 Implementation Retrospective + +### T1 simplifications resolved + +- **Immediate-apply hot-swap → turn-boundary queue (T2-ADR-1)**: T1's `handleModifierProfileUpdate` applied profile changes immediately on receipt; T2's implementation enqueues into `Room.pendingProfile` and applies via `applyPendingProfileIfAny` after the next successful `applyMove`. NACKs at apply time (re-validation against post-move session) route to the original proposer's socket via `findSocketByToken`. +- **Host-only authority → two-player consent (T2-ADR-2)**: T1 broadcast modifier swaps from any host message; T2 requires the opponent to send `modifier-profile.consent` with `approve` before the swap is enqueued. Solo-mode preserves the host-shortcut path (`modifier-profile.update`) since the sole participant is trivially the sole consenter. + +### Discovered patterns + +- **Reused `modifier-profile.queued` ack for proposer** rather than introducing a `modifier-profile.proposal-sent`. Client receipt handlers stay uniform; the observable difference is the opponent's wire traffic (`proposal-pending` vs. silence). Documented in protocol.ts. +- **Capture-phase `stopImmediatePropagation` for nested modal Esc handling**: ModifierProfileEditor's Esc handler now uses capture phase with `stopImmediatePropagation` so the RulesDrawer's own Esc handler does NOT also fire. Without this, both would close on a single keystroke, leaving the drawer's pointer-events backdrop briefly lingering. Discovered post-T1 via the solo-smoke regression test. +- **Token-keyed pending-proposer tracking**: `Room.pendingProposerToken` (and `proposalState.proposedByToken`) survive socket reconnects. NACK routing on apply-time validation failure finds the proposer by token, not socket id, so a brief disconnect doesn't lose the rejection notification. +- **Timeout stale-handle guard**: 60s proposal timeouts use `setTimeout` whose handler checks `room.proposalState !== currentProposalRef` before firing. Prevents firing rejection broadcasts on stale (already-consented or already-superseded) proposals. +- **Client-side undo/redo via `pushSnapshot`**: ModifierProfileEditor maintains its history-stack purely client-side. The 50-snapshot cap is generous for a single editing session. History clears on Save/Cancel (the saved state becomes a new baseline). +- **Component-local clipboard for copy/paste**: No OS clipboard interaction. Cross-panel sharing goes through a `clipboard` state lifted to the editor root. Dedup by `(pieceType, color, kind)` for type modifiers and `(square, kind)` for instance modifiers prevents pastes from creating additive duplicates. +- **Inline conflict resolution via `applyFix` dispatcher**: Each error/warning code routes to a specific resolution. `E_PROFILE_NO_KING` and `E_PROFILE_ATTR_LIMIT` are advisory-only (require user judgement); the rest auto-resolve. + +### Deviations + +- None — implementation matched ADRs as written.