Core server-health update: MSDP, socket/latency, combat reentrancy, scripting fixes - #276
Open
ahumbert wants to merge 32 commits into
Open
Core server-health update: MSDP, socket/latency, combat reentrancy, scripting fixes#276ahumbert wants to merge 32 commits into
ahumbert wants to merge 32 commits into
Conversation
Guard checked in_room >= 0 (bail on valid rooms) instead of in_room < 0 (bail on the actual invalid case), so the ROOM MSDP table never updated on normal movement.
MSDPSetString() already ran values through MSDPSanitizeValue(), but MSDPSendPair/MSDPSendList and msdp_room_update()'s TERRAIN field didn't. No live caller currently passes attacker/player-controlled text through these paths, but close the gap for defense-in-depth.
Nagle's algorithm was unconditionally enabled everywhere in the stack (game accept path, proxy-to-client, proxy-to-game), adding avoidable latency to small interactive packets.
write_to_descriptor() treated any negative write() return as fatal, including EAGAIN/EWOULDBLOCK on a non-blocking socket whose send buffer was momentarily full -- disconnecting players during output bursts on slow links instead of retrying next pulse.
The do-while read loop had no iteration or byte cap, only exiting on a newline or EWOULDBLOCK -- a connection that kept sending data with no newline (flood, huge paste, or a buggy client) could keep one process_input() call spinning through many read() cycles, stalling every other player for that pulse (single-threaded select loop).
Connections stuck at the name/password/menu prompts (no char_data yet) were invisible to check_idling() (which only walks character_list) and had no SO_KEEPALIVE, so a client that went silently dead at the network level (dropped wifi, sleep, mobile handoff) held its fd forever.
The restart loop had zero delay between a crash and the next bin/ageland invocation, so a crash-loop could repeatedly re-trigger the full boot-time world-load memory spike with no cooldown.
update_skill_timer() erased an expired entry then unconditionally advanced i via the for loop's own increment, skipping whatever entry the vector shifted into that slot -- that entry missed a decrement for one tick.
do_flee() and on_windblast_hit() each call check_simple_move() directly to test a forced move, then call do_move() for the same transition on success -- do_move() internally calls check_simple_move() again, firing ON_BEFORE_ENTER a second time. Any trigger side effect (message, resource decrement, damage) ran twice per successful flee/windblast. do_move() is ACMD-macro-generated so its signature can't easily grow a skip-trigger parameter without touching every command handler; instead a narrow, self-resetting flag lets the caller's own already-fired trigger result stand instead of re-firing it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
do_flee() sets g_skip_next_before_enter_for = ch unconditionally before calling do_move(), but do_move()'s IS_RIDDEN(ch) branch short-circuits into perform_move_mount() without ever calling check_simple_move(ch, ...) again -- perform_move_mount() only re-checks ch's own riders (a different pointer), never ch itself. When the fleeing character is itself being ridden, the flag was left set indefinitely and wrongly suppressed ON_BEFORE_ENTER on a later, unrelated move for the same character. Defensively consume the flag right in the IS_RIDDEN branch of do_move(), rather than only guarding the two call sites that set it (do_flee(), on_windblast_hit()) -- this centralizes the fix at do_move()'s other check_simple_move-adjacent entry point so it also protects any future caller of the flag, not just the two known today. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
WAIT_STATE_BRIEF/FULL's interrupt path ran complete_delay(ch) (which can reentrantly queue a brand-new delay via a follow-up action) then unconditionally called abort_delay(ch) and overwrote ch->delay with the interrupting action's data, silently destroying whatever the reentrant completion had just queued. Detect that case and back off the same way the existing lower-priority rejection already does, rather than clobbering it with no indication anything happened. Minimal correctness patch -- the underlying single-slot ch->delay design is unchanged; a real fix would replace it with a proper delay queue, which is out of scope here.
…ed blocks The for loop's own curr = curr->next increment fired unconditionally every iteration, including right after the recursive get_next_command() call for a nested SCRIPT_BEGIN had already advanced curr past that block's own matching END -- skipping one extra command and sometimes running past the current level's real terminating END/END_ELSE_BEGIN, read by players as script sections being silently ignored.
Two review findings against the core-server-health-update bundle: - comm.cpp (Task 4 EAGAIN fix): write_to_descriptor()'s EAGAIN/sofar==0 case returned 0, indistinguishable from a normal successful write, so process_output() fell through to its buffer-reset code regardless and silently dropped that pulse's output instead of retrying it as the comment claimed. write_to_descriptor() now returns a distinct -2 sentinel for that case; process_output() checks for it explicitly and returns early (0) without touching t->output/bufptr/bufspace, so the exact same buffered text is retried whole next pulse. The main loop's process_output() call site and process_input()'s "line too long" notice write (the only other call site that inspected the return value) were both updated so a deferred write is treated as non-fatal/no-op rather than a disconnect. - act_move.cpp (Task 9 double-fire fix): g_skip_next_before_enter_for was only consumed inside do_move()'s check_simple_move() call sites, after the AFF_HAZE dizzy-reroll could already change cmd to a different direction, or after an early return (no exit, closed door, wrong mount) that never reaches check_simple_move() at all. Either case left do_flee()/on_windblast_hit()'s suppression flag dangling (wrongly suppressing the real destination's trigger, or bleeding into a later unrelated move). The flag is now consumed once, unconditionally, at function entry, and only re-armed immediately before each check_simple_move(ch, cmd, ...) call site, gated on the direction still matching what was originally requested pre-haze. The now- redundant defensive consume in the IS_RIDDEN early-return branch was removed since the entry-point consume already covers it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
…TE_FULL log
Two minor review findings, both against code that only started actually
executing once Tasks 1/2 fixed msdp_room_update()'s inverted NOWHERE guard:
- act_move.cpp: msdp_room_update() dereferenced ch->desc (checking
->pProtocol) with no prior null check, even though it's called with a
spell victim as ch from mage.cpp -- a victim with no descriptor would
crash. Added an early `if (!ch->desc) return;` guard. Also fixed an
internal inconsistency: two lines read
world[ch->desc->character->in_room] for ROOM_NAME/ROOM_VNUM while the
rest of the function uses world[ch->in_room] for VNUM/NAME/EXITS/
TERRAIN -- for a switched immortal these can differ, producing
inconsistent MSDP data in one update. Standardized on ch->in_room.
- utils.h: WAIT_STATE_FULL's new reentrant-delay diagnostic (Task 10)
used printf() instead of log(), unlike its WAIT_STATE_BRIEF sibling,
so the message wouldn't reach the syslog operators actually watch.
Changed to log() to match. The macro's other, pre-existing
printf("double delay?\n") line is untouched (out of scope).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
Found and confirmed during manual testing of the core-server-health bundle (not one of its original 11 tasks) -- a years-old bug where the prompt randomly appears prepended to a game message, on any line (chat, combat, room name, etc.), worse under rapid command spam. Three mechanisms, all fixed: 1. select() reporting a descriptor's socket not currently writable (FD_ISSET false) -- process_output() was skipped entirely that pulse, but prompt_mode (set moments earlier by command processing) was never cleared, so the immediate prompt write fired anyway, ahead of the still-buffered, unflushed text. Pre-existing, present as long as this select()-loop has existed. 2. The EAGAIN-deferral path added earlier in this same bundle (Task 4) -- write_to_descriptor() returning early on a momentarily-full kernel send buffer left prompt_mode set too, for the same reason. This path didn't exist before Task 4 (hitting it used to disconnect the player instead). 3. The actual root cause, confirmed by direct byte-level reproduction with a scripted test client, not just static analysis: process_output() used prompt_mode for two unrelated purposes at once -- "print a prompt after this flush" (set by command processing) and "was there a still-unbroken bare prompt from a prior pulse, needing a leading newline before new content" (read by process_output()). Processing a new command sets prompt_mode=1 for the first purpose before process_output() reads it for the second, masking a real dangling prompt and skipping the leading break. This needs no EAGAIN or full send buffer at all -- it reproduces on a perfectly healthy connection from ordinary successive commands, which is why it's been happening for years. Fixed (1) and (2) by clearing prompt_mode when a flush is skipped or deferred. Fixed (3) properly by adding a new, genuinely separate descriptor_data field, bare_prompt_pending, set only when a bare prompt is actually written (the three write_to_descriptor() prompt sites in the "give the people some prompts" block) and consumed (checked + cleared) by process_output() to decide the leading-newline break -- fully decoupled from prompt_mode, which keeps its original, narrower meaning. Verified via a scripted Python telnet client against a scratch server instance: captured raw bytes showing the prompt glued to the next line before the fix, and a proper break after, across repeated rapid look/score/inventory commands. User separately confirmed against their live session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
Found during a full MSDP/telnet protocol monitoring pass (not part of the original core-server-health bundle) -- SEND SERVER_ID returned an empty string instead of the mud's name. SERVER_ID is announced once via a raw MSDPSendPair() call inside PerformHandshake()'s TELOPT_MSDP/DO branch, but that call only writes to the wire -- it never populates the variable's own backing storage, which only MSDPSetString()/MSDPSetNumber() do. A later client-issued SEND SERVER_ID reads that storage via MSDPSend() and got nothing. Deeper wrinkle hit while fixing this: bMSDP defaults to true at connection creation, unlike every sibling protocol flag (bMSSP/bATCP/ bMSP/bMXP/bMCCP, all false by default) -- so the "if (!pProtocol->bMSDP)" guard around the whole announcement block is dead in practice on a normal DO/WILL exchange. An initial attempt to add MSDPSetString() inside that guard silently did nothing, since the guard itself essentially never fires. Also confirmed MSDPSend() requires a logged-in character with PRF_MSDP set -- this announcement fires during telnet negotiation, before login, so it can't be used as a drop-in replacement for the existing pre-login send. Fixed by moving MSDPSetString(apDescriptor, eMSDP_SERVER_ID, MUD_NAME) outside/above the dead bMSDP guard so it runs unconditionally on every DO MSDP, leaving the existing MSDPSendPair() wire announcement untouched. The sibling ATCP-branch call site already got the same fix applied correctly, since bATCP genuinely defaults false there and its guard does fire on first negotiation. Verified via a scripted MSDP protocol monitor against a scratch server instance: SEND SERVER_ID now correctly returns "Return of the Shadow", both via explicit SEND and via the natural per-pulse report sweep. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
…ATHER push ROOM_EXITS and WORLD_TIME were each sent immediately via a raw MSDPSend(), which doesn't clear the dirty flag, so the trailing MSDPUpdate()/msdp_update() sweep sent them a second time. Switched both to MSDPFlush(), which sends and clears dirty in one call. WEATHER's own immediate push in weather_change() is removed entirely: it duplicated comm.cpp's msdp_update(), which already recomputes and sends WEATHER every pulse with a proper in_room bounds check that this call site lacked (a latent out-of-bounds world[] read if in_room were ever invalid at the moment weather ticked). Since weather_and_time() runs earlier in the same pulse as msdp_update(), removing the push adds no observable latency. Verified on a scratch server: ROOM_EXITS sent exactly once per move across 6 moves, WORLD_TIME sent exactly once per mud-hour tick, zero MSDP structural (VAR/VAL/TABLE/ARRAY) regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
…rn test) Marks the MSDP section's NPC-body/null-desc item as verified: initial wording was inaccurate twice over (possessing a mob via switch never exercises the guard since is_npc() short-circuits first; ordinary link-death doesn't null ch->desc either, that assignment is deliberately commented out in close_socket()). Corrected to the real scenario and verified it directly by launching the server as gdb's own child (works around the ptrace_scope restriction on attaching to an already-running process without root), forcing ch->desc to null on a real connected non-NPC character, and calling msdp_room_update() directly -- confirmed it returns cleanly. Also documents a live switch/return test: the long-standing "return crashes after switch" report didn't reproduce (tested twice, server stayed up and kept accepting connections). Notes a plausible-but-unproven connection to the msdp_room_update() null-desc guard already merged in this bundle, since the abandoned original body is left in the same null-desc, non-NPC state that guard protects against. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJcAWNHUoaGJEsrz9vduhN
autorun is an environment-specific launcher script that was never meant to be checked in -- this version isn't compatible with the live deploy environments. Untrack it (keeping the local copy on disk) and add it alongside the other runtime-generated entries already ignored here (autorun4000, switchserver.sh, crashes, syslog, .killscript). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GYbvB1G8LzoPCQPSvg1Ljm
…ated moves do_flee()/on_windblast_hit() set this flag immediately before calling do_move(), which re-arms it right before its own check_simple_move() call so that call's ON_BEFORE_ENTER trigger is skipped for the room they already validated. But check_simple_move() only consumed the flag at the trigger-check point, and five different early returns -- a special() intercept (mode is SCMD_FLEE, not SCMD_MOVING, so special() runs before the consume point), GET_POS/PLR_WRITING, missing dir_option, a nonzero wait_value, or no destination room -- all sit before that point. Any of them firing left the flag armed on the global, to be silently consumed by the character's next, completely unrelated move and wrongly suppress its ON_BEFORE_ENTER trigger (letting them walk past a guard-room check once). Reachable in practice via do_flee() alone: special() runs twice per flee (once in do_flee()'s own validation call, once in do_move()'s internal call), so a stateful or randomized special proc intercepting the second call is enough to trigger the dangle. Fixed by consuming the flag unconditionally as the first statement of check_simple_move(), captured into a local bool and branched on at the trigger site -- a dangle across early returns is now structurally impossible, regardless of how the function exits. Found via an adversarial review of the core-server-health-update PR (docs/superpowers/plans/2026-07-27-pr-276-adversarial-review-handoff.md), traced and confirmed against source before fixing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5FB9ut43RMb9oL5bXgcSG
…-circuited prompt writes Two related gaps in bare_prompt_pending (added in 8ae32c9 to fix the prompt-race bug) let it get out of sync with what was actually written to the wire, both reintroducing the glued-prompt bug on the exact paths that fix was meant to cover: 1. process_output() checks and clears bare_prompt_pending up front, placing the leading "\n\r" break only in the local output buffer. If write_to_descriptor() then returns -2 (EAGAIN, nothing written), the existing code correctly leaves t->output untouched for a retry next pulse -- but the flag was already cleared. The retry rebuilds output without the leading break, gluing the still-dangling prior prompt to the next line. EAGAIN happens under exactly the bursty send-buffer pressure where this bug matters most. Fixed by restoring the flag in the -2 branch before returning. 2. The three prompt-write sites in the "give the people some prompts" block set bare_prompt_pending = true unconditionally, without checking write_to_descriptor()'s return. If a prompt write itself silently drops (-2) or fails (-1), the flag would still claim a bare prompt reached the wire, causing a spurious leading blank line on the next flush. Fixed by only setting the flag when the write returns 0 (write_to_descriptor()'s actual full-success value here -- not the >0 a first pass assumed, which never happens for this function and would have silently disabled the flag entirely). Found via an adversarial review of the core-server-health-update PR (docs/superpowers/plans/2026-07-27-pr-276-adversarial-review-handoff.md), traced and confirmed against source before fixing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5FB9ut43RMb9oL5bXgcSG
All three set_nodelay(true)? call sites (GameAddr::connect(), handle_tcp(), handle_ws()) propagated a nodelay failure via ? and aborted the connection entirely -- disproportionate for what should be a best-effort latency optimization, and inconsistent with the C server side, which just perror()s and continues when the equivalent setsockopt() fails. Changed all three to log a warning and keep going instead. Found via an adversarial review of the core-server-health-update PR (docs/superpowers/plans/2026-07-27-pr-276-adversarial-review-handoff.md). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5FB9ut43RMb9oL5bXgcSG
…eshadow#276 review doc The manual test checklist's debug-account section pointed at a session-scoped /tmp/claude-1000/... path from another machine, which won't survive between sessions and reads oddly out of context (a friend's adversarial review of PR returnoftheshadow#276 flagged it, alongside two other claims about this account -- that it "apparently exists on real servers" and that the email-verification-bypass steps documented here are a meaningful exposure -- that don't hold up: lib/accounts/ is fully gitignored, so the account itself was never committed, and this was always throwaway local-only test infrastructure for a scratch server, not a real credential). Replaced the tmp path with a note pointing back to the durable login-sequence description already in the doc, and added framing up front so a future review doesn't re-flag it as leaked credentials. Also adds the review doc itself (docs/superpowers/plans/2026-07-27-pr-276-adversarial-review-handoff.md), relocated here from the repo root to sit alongside its companion plan doc and checklist, same naming convention. Three of its findings are fixed and pushed (508bec4, a837cd8, 3c865a3); the autorun-backoff claim (finding 6) was resolved by editing the PR description directly since the script isn't tracked in this repo; findings 3-5 are open questions for the author, not yet acted on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5FB9ut43RMb9oL5bXgcSG
…finding 3 correction Marks findings 1, 2, and both minor items [FIXED] with their commit hashes; findings 6 and 7 [RESOLVED] with how each was actually closed. Finding 3 (MSDPSend -> MSDPFlush requiring REPORT) gets a substantive rewrite, not just a status tag: the original claim -- that clients which never REPORT would go silent on ROOM_EXITS/WORLD_TIME/WEATHER -- doesn't hold on this codebase. bReport defaults to true for every variable on every connection (protocol.cpp:323, a separate, already pre-existing, catalogued bug -- REPORT is a no-op because everything's already "reported" from connect), so MSDPFlush's bReport gate isn't actually restricting anything today. Confirmed live, not just re-derived from source: a scratch-server test client that never negotiated MSDP and never sent REPORT still received ROOM_EXITS and the full ROOM table unsolicited, on login and on the first move. Reclassified RESOLVED / not an issue. Findings 4 and 5 remain open, needing the author's judgment call, not further investigation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5FB9ut43RMb9oL5bXgcSG
do_wizset's field-lookup loop used strncmp (case-sensitive) instead of this codebase's own strn_cmp, which is used two lines above for the file/player/mob prefix check. "OB" was the one mixed-case entry in the fields table, so `wizset <victim> ob <value>` silently failed with "Can't set that!" while `OB` worked. Swapping to strn_cmp fixes the comparison generally rather than special-casing the table entry. Confirmed via a scratch server: ob/OB/Ob all now set and persist correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLFtfZkDgrjesvdeeqd4MQ
Bind-mounted build/runtime artifacts (bin/ageland, *.o, lib/accounts/*, lib/account_characters/*, etc.) were being created as root inside the container, showing up root-owned on the host. rots-docker.sh now exports the current host uid/gid, and docker-compose.yml's `user:` directive uses them so the container runs as the host user instead of the image default (root). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLFtfZkDgrjesvdeeqd4MQ
…adow#276 finding 4) Consolidates the WAIT_STATE_BRIEF/FULL + complete_delay() reentrancy mechanism, the distinction between the clean-override branch (24/24 live-confirmed) and the untested reentrant-collision branch, and the open design question of which delay should win when a force-completed action reentrantly queues its own follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLFtfZkDgrjesvdeeqd4MQ
Author confirmed the design intent: casting should be interruptible by any attack, resisted only by Battle Mage spec. That system already exists (does_spell_get_interrupted()/break_spell(), wired into damage()) but do_bash (and ranger.cpp trap/ambush, olog_hai.cpp apply_victim_delay) call WAIT_STATE_FULL before damage(), so complete_delay() always force-fires the interrupted spell before the AFF_WAITWHEEL interrupt check in damage() ever runs. Reframes the fix target from WAIT_STATE_FULL's branch logic to call ordering, and notes the 24/24 "clean" test samples were actually all wrong outcomes under this reading (spell force-cast instead of interrupted). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLFtfZkDgrjesvdeeqd4MQ
do_bash forced complete_delay() on any pending action with lower priority (a spellcast is 30, bash is 80), which clears AFF_WAITWHEEL before damage() ever reaches the does_spell_get_interrupted() roll that ordinary weapon hits and mental attacks already respect (fight.cpp, clerics.cpp). Bash was the one attack that always broke a battle mage's concentration regardless of tactics/levels. Skip the WAIT_STATE_FULL clobber when the victim is mid-cast and their battle-mage resistance roll succeeds, mirroring the same silent-skip pattern used at the other two call sites. Bash still lands its own damage; it just doesn't override a successfully-defended cast. Live-confirmed with a fresh Wood Elf battle-mage test character: of 6 bash landings caught mid-cast, 4 resisted and the cast completed normally, 2 didn't and interrupted as before -- matching the existing resistance-roll behavior already proven at the melee/mental-attack call sites. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015LsV3dGQREFk5uixtFtTzr
Adds the 2026-07-29 re-investigation results to Task 10: confirms 999c167's reentrancy fix is sound (not a regression), and documents Bonus fix returnoftheshadow#6 -- the separate battle-mage cast-interrupt bypass bug found, fixed, and live-confirmed the same day (both the resist and non-resist branches). Also flags the 3 commits now sitting on top of the pushed PR returnoftheshadow#276 branch that aren't part of it yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015LsV3dGQREFk5uixtFtTzr
WAIT_STATE_BRIEF's copy of the reentrant-delay guard (999c167) has an explanatory comment; WAIT_STATE_FULL's copy didn't. Minor item from the PR returnoftheshadow#276 review (finding 4), flagged as fix-without-asking. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015LsV3dGQREFk5uixtFtTzr
…cope, all resolved Finding 5 (script.cpp get_next_command nested if/else mis-skip) is a real, pre-existing limitation, but fixing it risks changing execution for live scripts -- not a fit for a PR that's now only taking safe corrections rather than new intended changes. Marked out of scope, to be picked up separately with its own testing pass if ever done. With finding 4 already resolved by deferral, this closes out every item in the PR returnoftheshadow#276 adversarial review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015LsV3dGQREFk5uixtFtTzr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core server-health update: MSDP, socket/latency, combat reentrancy, scripting fixes
Summary
A bundled "core server-health" pass covering MSDP, socket/connection
latency, combat reentrancy, and NPC scripting bugs, implemented via
subagent-driven development (fresh implementer + reviewer per task, plus
a final whole-branch review), followed by manual testing and a few
additional bugs found and fixed along the way.
MSDP
NOWHEREguard inmsdp_room_update()(was a no-op)MSDPSendPair/MSDPSendListand theROOMtable'sTERRAINfieldSERVER_ID's backing storage soSEND SERVER_IDreturns datainstead of an empty string
ROOM_EXITS/WORLD_TIMEfrom being double-sent per event (rawMSDPSenddoesn't clear the dirty flag, so the next per-pulse sweepresent them) — switched to
MSDPFlushWEATHERpush inweather_change()thatduplicated the already-guarded per-pulse computation in
msdp_update()and carried a latent out-of-bounds
world[]readSocket / connection latency
TCP_NODELAYon the game socket and both proxy legsEAGAIN/EWOULDBLOCKwriteprocess_input()'s read-loop iterations per call (one floodingconnection could otherwise stall every player each pulse)
SO_KEEPALIVEgame messages under rapid command spam — root cause was
prompt_modeoverloaded for two unrelated purposes; added a dedicated
bare_prompt_pendingfieldCombat / scripting
update loop
ON_BEFORE_ENTERon flee/windblast (plus a follow-upfix for the
IS_RIDDENbranch and anAFF_HAZEre-roll edge case)get_next_command()from double-advancing past nestedif/begin/end script blocks, silently skipping content
Test plan
scripts/rots-docker.sh compilethroughoutwhole-branch review (2 additional issues found and fixed: EAGAIN
silent buffer-drop,
AFF_HAZEre-roll trigger dangle)monitor: VAR/VAL pairing, TABLE/ARRAY nesting balance) — zero
structural findings
scripted telnet client, byte-for-byte before/after capture
ROOM_EXITSsentexactly once per move,
WORLD_TIMEsent exactly once per mud-hourtick
msdp_room_update()null-desc guard verified directly (gdb,launched as its own child to work around
ptrace_scope): forcedch->descto null on a real connected non-NPC character, calledmsdp_room_update()directly, confirmed clean returnLIST/SEND/REPORT/UNREPORT, script vnum spot-checks,switch/return (a previously-reported crash source, not reproduced
after 2 cycles)
docs/superpowers/plans/2026-07-24-core-server-health-update.mdand the companion manual test checklist for full detail
🤖 Generated with Claude Code