Slice 2: Session state machine + linkdead reconnect#1
Conversation
Disconnecting without `quit` now marks a player Linkdead instead of immediately tearing down their Thing, so reconnecting with the same credentials within a grace window resumes the same character in place and an in-progress combat encounter freezes instead of ending. State lives on PlayerBehavior as a small LayeredCraft.OptimizedEnums state machine (Playing<->Linkdead) per ADR-0004/PLAN-0004, not a MEF-style polymorphic per-state hierarchy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dda0b3d117
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- SessionLoop: mutate ConnectionState/remove-from-world before the awaited disconnect save (not after), and guard it on the player's Session still being this loop's session - closes a window where a fast reconnect during a slow save got marked Linkdead by the old, now-superseded session. - LoginFlow: re-check the grace window (and that the Thing still has a parent) at password-verification time, not just ConnectionState - password entry takes real wall-clock time, so the window can expire or LinkdeadSweeper can remove the Thing between the earlier lookup and this point. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ncipollina
left a comment
ncipollina
left a comment
There was a problem hiding this comment.
🛑 GitHub won't let me submit a formal "Request changes" review on my own PR, so recording the verdict here instead: this should be treated as blocked pending a fix. One blocking bug: the reordered SessionLoop disconnect path saves ParentId = null on every normal quit, silently losing the player's last room (see inline comment). Also flagged: a new class using a primary constructor against the repo's constructor-style rule, missing XML docs on the new ConnectionState/ReconnectPolicy/LinkdeadSweeper/PlayerBehavior public surface, a few new uncommented null-forgiving operators, and a minor ADR/code signature drift on Reconnect(). Solid design/process work otherwise — ADR-0004 and PLAN-0004 are thorough and the reconnect-race fixes in the second commit are good catches.
- Fix quit-path regression from the earlier P1 fix: SessionLoop now saves before removing on explicit quit (was saving after, which persisted ParentId=null and silently lost the room a player quit from - only the Linkdead transition needs to race ahead of the save, not quit's removal). - LinkdeadSweeper: traditional constructor instead of a primary constructor, per coding-standards.md (new classes don't get the pre-existing-code carve-out). - Add XML doc comments to the new public API surface: ConnectionState, ReconnectPolicy, LinkdeadSweeper, and PlayerBehavior's new members. - Comment every null-forgiving (!) use added in this PR (CombatManager, LinkdeadSweeper) with the invariant it relies on. - ADR-0004: append a post-acceptance implementation note (not an in-place edit, since the ADR is Accepted/immutable) correcting the documented Reconnect(DateTimeOffset) signature to the shipped parameterless Reconnect(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
Summary
ConnectionState(Playing/Linkdead) state machine onPlayerBehavior, built onLayeredCraft.OptimizedEnums(per user direction, not a WheelMUD-style MEF/polymorphic-per-state hierarchy) — see ADR-0004.quit) now marks the playerLinkdeadand leaves them in-room instead of instantly removing them; a newLinkdeadSweepertick job force-removes them afterReconnectPolicy.GraceWindow(3 min placeholder) if nobody reconnects.LoginFlowresumes aLinkdeadcharacter on reconnect ("Welcome back.") instead of only rejecting; the existing "already logged in" rejection for an actively-connected character is unchanged.CombatManagerfreezes an in-progress encounter while the attacker isLinkdeadand only abandons it once the same grace window expires, instead of ending it the instant the connection drops.networking.md/combat.md.Test plan
dotnet test— 101 tests passing, including 15 new (ConnectionState,PlayerBehaviortransitions,LinkdeadSweeper,CombatManagerfreeze/abandon,LoginFlowreconnect/reject)quitstill removes immediately with no linkdead window🤖 Generated with Claude Code