Merge pull request #279 from ahumbert/fix/account-menu-logout-closes-connection fix(account): close the connection when logging out of the account menu - #280
Open
Noobinabox wants to merge 225 commits into
Open
Noobinabox wants to merge 225 commits into
Noobinabox wants to merge 225 commits into
Conversation
* RangerProg: Add option for Riposte * Add missing eof line
Change Teleport to Level 93
Update Keywords for New Ranger Prog
Whistling a fighting tame was a silent no-op: update_pos() reverts the forced POSITION_STANDING right back to POSITION_FIGHTING since specials.fighting is still set. Now a fighting tame gets a single do_flee() attempt instead; once it's no longer fighting, the next whistle runs the normal recall as before.
* Fix crash when a character's hit points go negative SET_INT_VALUE (and other script int operators, which all funnel through the same write function) could write an arbitrary unvalidated int into a character's hit points. report_char_health()/add_prompt()/ print_group_leader()/print_group_member()/do_report() then scanned a fixed 8-entry health/status array with no bound on the loop index, overrunning it whenever the computed health percent exceeded 1000 - via integer overflow on extreme values, or just an ordinary hit paired with a small max_hit. Bound those scans so the overrun is impossible regardless of how hit went negative (combat, regen, poison, or script). Clamp the script write paths (script.cpp's SET_INT_VALUE and mudlle's 'h' opcode) and resync position via update_pos() when the result is non-positive, or when the character was already parked at a stale DEAD/INCAP/STUNNED position from an earlier script write and needs releasing now that they've recovered - avoids both leaving a character "alive" per position while lethally wounded, and a soft-lock where a healed character stays stuck below the position required for nearly every command. Deliberately does not disengage them from their current combat target on incapacitation (differs from damage()'s own behavior), and does not call die()/stop_fighting() from these paths to avoid re-entrant script/trigger execution against live pointers. Since the position resync can land a character directly at POSITION_DEAD without going through damage()'s own die() call, extend point_update()'s sweep to pick up stray POSITION_DEAD characters and kill them safely from the main pulse loop, re-deriving position from current hit first so a character who naturally regenerated back to health isn't wrongly killed on a stale value. Also fixes a fallthrough bug in SET_INT_DIV: dividing by zero fell through into the RANDOM case instead of returning cleanly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix misleading precedent in POSITION_DEAD sweep comment The comment justified calling die() with a NULL killer by citing fast_update()'s regen-death path, but that path calls raw_kill() directly and skips the ON_DIE trigger and exp/PK/exploit bookkeeping die() performs. The real precedent is do_quit()'s die(ch, 0, 0) for a player expiring below POSITION_STUNNED with no external killer. Cite that instead and make explicit that this sweep intentionally still fires ON_DIE side effects, unlike fast_update()'s silent regen-death. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…275) Some clients (e.g. Mudlet) convert MSDP variable/value pairs into JSON, which breaks if a value contains an unescaped control character, quote, or backslash. weather_messages[] entries end in a literal "\n\r" for direct terminal display, and that same raw string was being sent as the WEATHER MSDP value, producing invalid JSON on the client side. - Add MSDPSanitizeValue() and apply it in MSDPSetString(), escaping '"', '\', and control characters for every MSDP string variable. - Apply it explicitly where act_move.cpp hand-builds the ROOM table's NAME field, since MSDPSetTable() bypasses MSDPSetString(). - Strip the trailing "\n\r" off weather_messages[] before using it as the WEATHER value, since it's data, not display text. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Added docs. * Added documentation. * Updated docs. * Changing things to cmake. * Added subagents and unit tests. * tests: expand unit coverage across combat and mage helpers * tests: expand coverage and move test seams into harness * account: add email-backed account management Introduce email-first accounts with verification, admin management, account-linked character migration, and account-backed login flow. Add account storage helpers, tests, and the latest cutover work to serve exploit history from account snapshots while preserving data across saves. * build: create account snapshot directories Update the CMake and legacy make setup targets so fresh runtime bootstraps create lib/account_characters bucket directories alongside the other account-management storage paths. * account: add account-native JSON persistence * Added the last little bit of account management before cleanup. * Split out the account management files to be more readable. * Added more commands and updated old snapshot migration file saves. * Fixed affects and user preferences. * Added true color support. * Hardened smoke tests and updated required installs * Updated .gitignore with cmake files. * Fix for migrating legacy characterr files with no followers. * Added login restrictions to only the current character logged in. * Fixed make file formatting. * account: add one-shot linked-character selection unlock Add `account unlockselect <email-or-account>` for high-trust account administrators to grant a runtime-only, account-scoped unlock when a low-level linked character is stuck active. The unlock allows one linked-character selection, is consumed at final character entry, does not unlock new-character creation, and is not persisted to account storage. Stale unlocks are discarded or replaced so they cannot apply to later unrelated active-session restrictions. Update immortal help and planning docs, and add regressions for command grant/reject paths, one-shot consumption, stale lifecycle behavior, and new-character non-bypass behavior. * Optimize character serialization and loading (both legacy and account paths). Increase frequency of autosave. (#269) * Add savebench-port design spec (legacy atomic finalize + pipeline benchmark + scheduler) Carries the intent of feature/savebench-finalize onto account-management as a focused, data-gathering branch off account-management: - Port the atomic write-temp-then-rename finalize into the legacy non-account save_player path (portable std C++), with an A/B byte-identity gtest. - Profile the full character persistence pipeline (save AND load) stage-by-stage plus an end-to-end total + minor-middle remainder, via an offline gtest and a sandboxed in-game savebench command, so we know the real cost before changing save frequency. - Bring the autosave cadence scheduler over as a behavior-neutral refactor, defaulted to 240s (today's 4-minute cadence); Crash_save_all stays dirty-gated. Snapshot de-gating, cadence reduction, and anti-rollback hooks remain deferred pending the benchmark data. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add savebench-port implementation plan Bite-sized TDD task breakdown for the approved spec: prep + .DS_Store hygiene, legacy save_player atomic finalize port (+ A/B gtest), full save/load pipeline benchmark (offline gtest + sandboxed in-game command), and the autosave scheduler defaulted to 240s. Embeds the verbatim ported finalizers/scheduler/ Stopwatch and exact wiring anchors (CMake, command index 249, config/heartbeat). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build: make the i386 Docker image run the CMake/gtest suite (Task 0) The i386 dev image shipped only g++/make and the wrapper drove the legacy src/Makefile, so the CMake gtest suite could not be built or run locally. Add cmake + libgtest-dev + libcrypt-dev to the image and a `scripts/rots-docker.sh test [gtest-args]` command that builds and runs ./bin/tests directly (ctest's gtest_discover_tests PRE_TEST finds 0 under cmake 3.18, but the binary runs fine under QEMU). Document the per-test verification convention and the ~163-test 32-bit baseline (the suite is built 64-bit on CI; our i386 container exposes 32-vs-64-bit expectation diffs in suites this plan does not touch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: add .no-autoformat marker and ignore .DS_Store * feat: add portable crash-safe player-file finalizers + A/B gtest Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * build: fix rots-docker.sh test to build the ageland_tests target `make build` only builds `ageland`; the test binary is the separate `ageland_tests` target. The `test` command now configures and builds `ageland_tests` directly, then runs ./bin/tests, so it never executes a stale test binary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(plan): note shell-quoting for gtest filters; wrapper builds test target * refactor: extract write_player_text and make save_player crash-safe via atomic rename - Extract write_player_text bool serializer from save_player (Task 3): moves the entire fprintf block verbatim into its own function that opens scratch_path instead of the hardcoded "players/temp", checks ferror/fclose, removes the partial scratch on I/O error, and returns bool. - Rewire save_player to atomic finalize (Task 4): save_player now calls write_player_text then finalize_player_file_rename; ch_file is updated only on success so a crash during finalize never destroys the live save. - Add single-file assertion to write_valid_legacy_player_file helper: after save_player, assert the bucket dir holds exactly one versioned file for the character (uses std::filesystem::directory_iterator for consistency with the rename-then-enumerate behavior of finalize_player_file_rename; POSIX readdir showed a known QEMU i386 consistency lag after rename). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add pure unit-tested autosave scheduler (seconds->pulses + timer) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: autosave_time is now seconds (default 240 = unchanged 4-minute cadence) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: drive heartbeat autosave from the seconds-based scheduler (240s default) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add portable std::chrono Stopwatch for benchmarking * refactor: expose read_text_file/write_text_file_atomically for stage timing * feat: add character persistence pipeline benchmark (offline gtest) Adds a reusable save_benchmark module that times each stage of saving and loading an account-backed character (SAVE S2-S5, LOAD L1-L5), plus the end-to-end total and a minor-middle remainder so per-stage shares sum to ~100%. Driven by an offline gtest using a temp-account fixture. The offline fixture stages the account-owned character.json via direct path I/O (mkdir + write_text_file_atomically) rather than account::create_account/ admin_link/migrate, whose opendir/readdir directory-scan lookups do not function under the 32-bit QEMU test environment (part of the ~163 known suite failures). The benchmark module itself is production-correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(savebench): reconcile per-stage shares with printed total; honest leak/doc comments - finalize_shares: rename vars for clarity; remove stale total.share=100/total.name="TOTAL" assignments (format_report now owns the footer rendering). - format_report: replace single ambiguous TOTAL row with two footer lines — the share denominator (stage_sum + other_us, the value all share% reconcile against) and the end-to-end single-pass average. When stage_sum > end_to_end, appends an explicit per-stage timing overhead note so the gap is never hidden. - StageTiming::share doc comment: updated to describe the new denominator (sum of named stages + other) rather than the old "percent of end-to-end total". - L5 comment: corrected to truthfully state that store_to_char CREATE()s inner fields on every call with no prior free, so per-iteration allocations leak (bounded, acceptable for short-lived offline benchmark; matches db_loader_tests idiom). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add implementor savebench command (sandboxed pipeline profiler) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * cleanup(savebench): propagate transform-stage errors; plug test scratch leak; skip_spaces Fix 1: profile_save/profile_load in save_benchmark.cpp previously swallowed all errors (unconditional return true). Now S5 (write_text_file_atomically) in profile_save and L2/L3/L4 (read_text_file / deserialize / apply) in profile_load are fatal: on failure they set *error and return false. S2/L1 (read_account_file) remain non-fatal so the offline SaveBenchmark test (which has no account.json) continues to pass. All stages are still timed regardless; updated header-doc comments on both functions. Fix 2: player_finalize_tests.cpp cleanup tail now unlinks pf_test_new_scratch alongside pf_test_legacy_scratch, plugging a scratch-file leak on finalize failure. Fix 3: savebench.cpp skips leading spaces before atoi(argument) via an inline while-loop (skip_spaces has no header declaration in this codebase). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(savebench): mirror the report to the MUD syslog do_savebench now logs its per-stage SAVE/LOAD report to the syslog (one entry per line, bracketed by begin/end markers) in addition to paging it to the invoker, so a run can be followed server-side, not just at the player's terminal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(savebench): record account save/load pipeline perf findings + next steps Document the first savebench measurement (SAVE ~2.1ms, LOAD ~3.7ms per char on i386/QEMU): read_account_file is the dominant AND redundant cost (57.5% save / 30.9% load), and deserialize_character_from_json is the load bottleneck (48.6%). Update the spec's gate (#11) and deferred (#12) sections to make caching the owner->account link the prerequisite optimization the snapshot/cadence work depends on, with deserialize as the secondary target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: follow-up plan — account perf optimization + consistent-snapshot autosave Hand-off plan for the next session: recaps the completed savebench-port work, details Phase 1 (cache the redundant owner->account-link resolution — read_account_file is a full accounts/ scan called 3x per save, ~57% of save cost) with a single invalidation hook at write_account_file, and Phase 2 (rework Crash_save_all into a de-gated point-in-time save-all snapshot + anti-rollback hooks + persisted-PLR_CRASH handling + cadence reduction). Sequenced cache-first so the snapshot's per-player cost is minimized; all sites refreshed to current branch line numbers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(gitignore): ignore lib/misc/xnames*; untrack stray root .DS_Store Match rots_live_modern: add lib/misc/xnames* (covers xnames and xnames.old) and stop tracking the root .DS_Store so the existing .DS_Store rules take effect. (CTest Testing/ artifacts are handled in the personal global gitignore, not here.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): design spec — account cache + parallel JSON perf (profile-before-adopt) Captures the Phase-1 performance design as parallel, profileable implementations: account-resolution cache (read_account_file_cached), stacked deserialize variants (v2a memoized lookups, v2b JsonReaderV2), stacked byte-identical serialize variants, and the savebench A/B compare-report wiring with equivalence gates. Grounded in a 5-strand code analysis; key reframe = the dominant deserialize cost is the O(N^2) skill/talk/color slug rebuild in character_json.cpp, not the JsonReader tokenizer. Decisions: hand-rolled v2 only (no third-party JSON lib); stacked variants; build the cache here as a profiled parallel path. Behavior-neutral branch; adoption deferred and gated on the measured numbers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): record g++ 9.4.0 compiler floor + integer-charconv-only guardrail Live runtime is g++ 9.4.0 (Docker build image is g++ 10). The design only uses the integer overloads of std::from_chars/std::to_chars (libstdc++ since GCC 8.1, header- inline, no runtime symbol), so it works on g++ 9.4.0 with no environment update. Guardrail: never use floating-point charconv (GCC 11); all character JSON numeric fields are int/long (no double/float in character_json.{h,cpp}). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): implementation plan — account cache + parallel JSON perf (9 TDD tasks) Bite-sized TDD tasks across four workstreams (account_cache module; JsonReaderV2 + deserialize v2a/v2b; serialize v2a/v2b; savebench compare A/B wiring + subcommand), drafted against a locked interface contract and self-reviewed for type consistency, placeholders, and cross-task seams. Behavior-neutral; adoption + re-measure are gated follow-ups per the spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(account_cache): memoized read_account_file_cached + negative-cached owner resolver Parallel, profileable cache (Phase-1 workstream 1): full-AccountData account map + char->owner map with negative ("not linked") caching, (root,name)-keyed, clear() for tests. Behavior-neutral — no live caller is routed through it yet. Tested via a set_backing_resolvers_for_testing DI seam driving counting fakes, because the real on-disk account-directory readdir scan does not resolve under QEMU i386 (the AccountManagement.* on-disk suites are pre-existing 32-bit baseline reds for the same reason). Call counts make "served from cache, no rescan" directly observable. Owner cache memoizes only successful resolutions (incl. the valid negative); genuine errors retry, mirroring read_account_file_cached. 7/7 AccountCache tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(json): add JsonReaderV2 + append_escaped_json_string (parallel v2 baseline) JsonReaderV2 is a drop-in of JsonReader with lower-allocation internals: integer std::from_chars (no substr/strtol), no-escape fast-path + move-out strings, branchless whitespace/digit tests, strlen-free literal match. JsonReader stays untouched as the measurement baseline. append_escaped_json_string is the serialize-v2 fast-path escaper (appends verbatim when nothing needs escaping). JsonPerf parse-equivalence + escape tests pass; integer from_chars confirmed working under the i386 g++ (g++ 9.4.0 floor holds). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(character_json): deserialize v2a/v2b (memoized lookups + templated parsers + JsonReaderV2) Stacked, profileable deserialize variants beside the untouched v1: v2a routes skill/talk key lookups through memoized slug->index maps (the dominant O(N^2)-slug-rebuild win, ~25k transient string builds/load eliminated); v2b additionally flows JsonReaderV2 through the now-reader-templated nested parsers. v1 deserialize_character_from_json is byte-neutral (its dispatch untouched; templating preserves output, pinned by CharacterJson.*). Equivalence gates compare v1/v2a/v2b outcomes (success-with-identical-struct OR identical rejection), so the all-skills "heavy" tier (which v1 itself rejects on duplicate slugified skill keys) is a stronger gate covering the rejection path too. 7/7 JsonPerf + all CharacterJson green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(character_json): serialize v2a/v2b (reserve+to_chars; escape-fastpath+cached keys) Stacked, byte-identical serialize variants beside the untouched v1 ostringstream path. v2a builds into one reserve()d std::string via a JsonWriter (std::to_chars integers, no per-field heap churn, move-out instead of v1's .str() double-copy). v2b adds the append_escaped_json_string fast-path (verbatim append when nothing needs escaping -- the common case for slugged keys/flag names) and lazily-cached skill/talk key tables (no per-serialize slugify), reusing v1's collect_* intermediates so order/filtering and thus the bytes are guaranteed identical. JsonPerf byte-equality gates (v1==v2a==v2b, light + heavy) pass; integer to_chars confirmed under the i386 g++. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(savebench): opt-in compare report + 'savebench compare N' subcommand (cache + JSON v2 A/B) profile_save/profile_load take a trailing PipelineReport* compare=nullptr; when set they populate a SEPARATE report (its own TOTAL runs each compared item once, so shares reconcile to ~100%) with S2/S2c/S4/S4a/S4b and L1/L1c/L3/L3a/L3b stages -- read_account_file vs read_account_file_cached, and serialize/deserialize v1 vs v2a vs v2b. The canonical breakdown is byte-unchanged (defaulted param). In-game, `savebench compare N` opts in and prints two extra COMPARE sections; plain `savebench N` stays cheap. Compare stages are pure in-memory (read-only resolvers + string transforms) -- the sandbox invariant holds. SaveBenchmark.* 2/2 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): mark implementation plan complete (9 tasks landed; deviations + gated follow-ups) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): standalone results write-up (benchmark + verified safety + rationale) Self-contained report for an external reader: savebench compare 500 results (cache ~99%, deserialize -42%, serialize -25%, no sustained regression), the equivalence/test/ boot evidence that the parallel methods are safe, and the root-cause reasoning (redundant 3x account scan; O(N^2) skill-slug rebuild; ostringstream double-alloc + per-field escape). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): session handoff snapshot for the performance thread Self-contained resume doc for a future session: committed state, the adoption follow-up (route live callers + write_account_file invalidation hook + re-measure), the Docker-i386/ CMake/savebench environment knowledge, the hard-won gotchas (QEMU scan miss -> DI seam, slug collisions, g++ 9.4.0 floor, stale GNU Makefile), and the Phase-2 coordination constraint (cache adoption must precede any autosave cadence reduction). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autosave): de-gate Crash_save_all to a point-in-time save-all snapshot (Phase 2.1/2.3c/2.4) Crash_save_all now saves EVERY connected (CON_PLYNG, non-NPC) player each cadence instead of only PLR_CRASH inventory-dirty ones, so PvP/group participants recover to the same moment. All saves use notify=0 (no "Saving X." spam on a routine snapshot); a broken descriptor is skip-and- logged rather than aborting the batch (modeled on Emergency_save). The redundant explicit REMOVE_BIT(PLR_CRASH) is dropped (Crash_crashsave already clears it). Removed the per-kill 10%- chance save in group_gain (fight.cpp) -- bundled with the snapshot, which now covers XP persistence. Documented the deliberate CON_LINKLS exclusion and updated the heartbeat comment. Cadence stays at 240s (4 min); lowering it is gated on the perf-cache adoption + re-measure. Behavior validated by clean compile + server boot; snapshot semantics need a running-server check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autosave): neutralize persisted PLR_CRASH on load (Phase 2.2) PLR_CRASH (a transient inventory-dirty marker) now round-trips through the character JSON; mask it off in apply_character_data_to_store so a character saved mid-change never reloads as already- dirty. The kPlayerFlags "crash" entry is kept so existing files that persisted it still decode without rejection. New CharacterJson.StripsPersistedPlrCrashFlagOnLoad test pins it (PLR_CRASH cleared, other flags survive). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autosave): anti-rollback save hooks for exploits and angel rerolls (Phase 2.3a/2.3b) write_exploits (db.cpp): save_char(ch, NOWHERE, 0) immediately after a CONFIRMED exploit-record write -- crash-proofs all exploit record types (PK->killer, death/level/stat/birth/...->victim) without a per-type hook. Gated on the successful write only (not the orphaned-account early return, nor a logged write failure). resetter reroll branch (spec_pro.cpp): direct save after the rerolls increment to close the crash-to-reroll exploit (a reroll records no exploit and triggered no other save). Both rely on save_char's existing IS_NPC/!ch->desc guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(autosave): Phase 2 implementation status (snapshot + anti-rollback; cadence gated) What landed (commits 5023ddc/df537f4/02d7a54), verification done (compile + PLR_CRASH test + 62/62), and what intentionally is not: Task 2.5 cadence reduction is gated on Phase-1 cache adoption + re-measure, and the snapshot/hook behavior needs a running-server validation (checklist included). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(account_cache): adopt the account-resolution cache on the live path (JSON v2 stays off) read_account_file / find_linked_character_owner_account are split into an uncached body (the real O(N) scan) plus a thin wrapper that delegates to account_cache when enabled, so every live caller (save_char's 3 scans, the load paths, the boot index build, ...) memoizes its account resolution with no call-site churn. The cache is flushed wholesale on every account.json write (write_account_file -> invalidate_all): account mutations are rare and off the hot path, so a coarse flush is provably free of relink/rename/normalization staleness traps and never regresses (worst case degrades to no-cache). boot_db enables it for the live server only. Gated by a default-OFF enabled flag so the test binary (which never calls boot_db) keeps the exact uncached behavior: proven by an A/B stash-diff -- AccountManagement.* has the IDENTICAL 71 pre-existing 32-bit baseline reds with and without this change (zero new failures). 9/9 AccountCache tests pass (incl. new invalidate_all + base-delegation cases). JSON serialize/deserialize deliberately remain on v1 (not adopted here). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(perf): mark account cache adopted in the handoff (JSON v2 still off) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autosave): lower the crash-save cadence to 30s (deferred Task 2.5, now unblocked) autosave_time 240 -> 30 (the source's original cadence). This was the one Phase-2 piece gated on the account-cache adoption, which is now live (c8f8877): the account read on the hot save path is memoized, and the save-all snapshot is a consistent point-in-time pass, so a tighter cadence is affordable. 30s is above the scheduler's 15s floor. config.cpp is CRLF (preserved); CrashsaveSchedule.* stays green (it pins the scheduler function, not the config default). Note for large deployments: the snapshot runs single-threaded inline on the heartbeat, so size per-player-save-cost x max-connected-players against the 250ms/pulse budget before using 30s in production; fine for the test instance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Noobinabox <a@a.com> * Updated Makefile with newly added files. * Added a fix to save specialization for new character files and migrating legacy. * Added unit tests for msdp --------- Co-authored-by: Noobinabox <a@a.com> Co-authored-by: drelidan7 <david.gurley@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Append items 4-9 to "Where this plan refines the spec" for the backup find -exec form, the revert's find-based help restore/relink/marker-clear, grep -cxF fixed-string source edits, the DEPLOY_IN_PROGRESS marker, sh -c command wrapping, and the restricted login character set. Add a revert drill and an interrupted-deploy drill to Task 7's manual verification, a prerequisite about checking real-port service ownership before trusting the sudo chown step, and a note that the automated test count may be stale. Mark the spec implemented and note that a help file deleted from the repo is neither listed as a help change nor deleted on the server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
src/big_brother.h is stored in git with CRLF line endings, so the uploaded copy's line 13 ends in "\r". grep -x and a sed pattern anchored with $ do not match that, so deploying 4k stopped at step 6 with "expected exactly one line". The check now counts a line with or without a trailing CR, and the sed edit keeps whichever ending the line had. Tested with a CRLF fixture and with the real header from the repo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…to account characters Crash_idlesave destroys unrentable items -- keys and ITEM_NORENT items -- before it saves, and it writes no follower section (historic; see docs/systems/idle-void-and-followers.md). Every game save is copied into the account through write_linked_character_object_file, which parsed the bytes with the strict reader, and the strict reader requires a follower section. So the idle save was refused, objects.json kept the previous 30-second autosave (Crash_save does not filter unrentables), and at the next login selection loaded that stale copy and deleted the correct legacy save. Reproduced on a server running a copy of the data: an account character idled out carrying a key, a NORENT diamond and a plain ring. The idle save held only the ring; objects.json held all three; after a reboot the character logged in carrying all three. Never observed on live. write_linked_character_object_file now reads the save with legacy_object_save_data_from_binary, the reader that matches the game's own loader: it forgives only a follower section missing entirely, and a save cut off anywhere else is still refused. write_account_object_file is left strict for direct writes, which #274's AccountNativeObjectWriteRejectsLegacyObjectFileWithoutFollowerSection pins. Follower outcomes are unchanged: the idle save holds no followers either way (IdleFollowersTest). Testing: LinkedCharacterObjectRefreshAcceptsAnIdleSaveWithoutFollowerSection failed first (the copy was refused and objects.json kept rent code 1 with the key and NORENT item) and passes now; LinkedCharacterObjectRefreshStillRejectsAPartialFollowerRecord guards the strictness that remains. Live on fixed builds: an idle-out carrying two keys, two NORENT diamonds and two rings left objects.json at rent code 5 with only the rings, and the character logged back in with only the rings. Full gtest 927 pass, 10 fail, the same 10 as the base. Server builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hen one decays obj_to_room counts the room's contents after adding an object and, at 1000 or more, logged "infinite loop in room contents" and "recovered" by resetting the room's contents to the new object. That discarded every other object on the floor while each still had in_room pointing at the room. When one of those orphans later reached timer 0, point_update -> extract_obj -> obj_from_room walked the room's list, never found it, and dereferenced NULL. A mass quit is what fills a floor: extract_char appends a quitting player's inventory straight onto the room's list with no count check, and their worn gear then goes through obj_to_room and trips the guard. The staged live-data test crashed this way after about 155 quits, on this branch and on the live tag c5a01dc alike, and both crashing runs logged the guard twice before signal 11. The guard has been there since the 2017 import. The guard could never catch what it was written for: the duplicate scan just above it walks the list with no bound, so a genuinely cyclic list would hang there first. It only ever fired on a floor that really held 1000+ objects. It now reports and leaves the list alone. Testing: RoomContents.AddingAnObjectToAFloorOfAThousandKeepsEveryObjectListed builds a 1000-object floor the way extract_char does, then drops one more through obj_to_room. It failed first (the list held 1 object, expected 1001) and passes now. Full gtest 928 pass, 10 fail, the same 10 as the base. Server builds. Not re-run as a staged mass quit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 3 used to go straight to `sudo chown` for anything unwritable, which asked for the sudo password even when chown could not help (a read-only file the deploy login already owns). Now it runs `chmod u+w` on unwritable paths the login owns first, with no sudo, and re-checks. Only what is still unwritable goes through `sudo chown`, followed by `chmod u+w` and a final check. A normal deploy and an owned read-only file need no sudo password. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`scripts/deploy.py revert <env> <user>@<host> <ssh-port>` connects once and puts an env back to its src/backup: it refuses when there is no backup, restores src/ and the help files, forces a relink, clears DEPLOY_IN_PROGRESS, and fails if bin/ageland was not rebuilt. Coders keeps no backup, so it is refused without connecting. A failed deploy now prints this short command first, with the server-side command as a fallback, because the long one-line command broke when pasted into a terminal that wrapped it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…inish Step 3's DEPLOY_IN_PROGRESS message said "see the failure report", which is long gone by the time someone re-runs a deploy. It now prints the exact `scripts/deploy.py revert <env> <user>@<host> <ssh-port>` command to run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 008d8f8, from review. Removing the reset left obj_to_room's report firing on every object added to a room whose floor already held 1000 or more. A mass quit is exactly that case: each quitter's worn gear goes through obj_to_room, so the log file and every online god would get a burst of identical lines. The wording ("infinite loop in room contents") was also wrong, since a large floor is legitimate. obj_to_room now counts to 1001 and reports only when the floor holds exactly 1000 objects: "obj_to_room: the floor of room N has reached 1000 objects." A busier floor stays silent. The contents list is still never shortened. Testing: RoomContents.AFloorReportsReachingAThousandObjectsOnlyOnce drops three objects onto a 999-object floor and captures stderr. It failed first (three "infinite loop" reports, none with the new wording) and passes now with exactly one. The no-orphan test still passes. Full gtest 929 pass, 10 fail, the same 10 as the base. Server builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mlinks chmod on a symlink changes the file it points to, and chown without -h does the same, so a symlink under src/, bin/ or lib/text/ pointing outside /rots/<dir> let step 3 change files outside the game dir (reproduced with a symlinked source file and a symlinked help file). sftp and cp would also write through such a link. Step 3 and revert now refuse to continue when src, bin, lib/text, or any symlink under them resolves outside the port dir, listing the links. The chmod pass skips symlinks and every chown uses -h, so neither follows a link even inside the port. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 3 now matches the manual deploy process: it never runs chmod, and when something is not writable it runs `sudo chown -h` only on what is inside src/ and bin/ and on the help files, never on src, bin or lib/text themselves. If one of those three folders is not writable, the deploy stops right away without asking for a sudo password and says to fix it by hand, since changing contents cannot help. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The build and revert run on the same server as the live ports. Two parallel compile jobs keep the build from competing with a running game for CPU, at the cost of a slower step 7. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches the deploy script, which now builds with two parallel jobs so a build does not compete with a running game for CPU. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test port now only warns when the checkout is not release-frodo, the same as the zzz-forge-test targets, and skips the pull. It still keeps a backup and gets a test-YYYY-MM-DD tag. live, 4k, and coders still stop unless the checkout is release-frodo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oom still show A look sends the room text, then one line per floor object, then the people in the room. The player's output buffer holds 16 KB; once it is full, write_to_output drops everything queued after it for that pulse and process_output sends "**OVERFLOW**". A floor of roughly 300 objects fills it, so the people list -- players and aggressive mobs -- never arrived, and neither did the colour reset. The automatic look on entering a room is the same code. This is not new: before 008d8f8 the same cut-off happened between ~300 and 999 objects, and the old reset at 1000 only hid it by discarding the floor. With the reset gone a mass quit leaves floors of thousands. The staged live-data mass quit (803 quits, one boot) had three rooms reach 1000 objects; an observer in the first one got 166 looks of ~300 object lines and "**OVERFLOW**" with no people, while an empty-floor look of the same room listed its guard. list_obj_to_char now lists at most 100 visible objects on a room floor (the only caller with show == false, mode 0) and then says "...and N more items are lying here." Floors of 100 or fewer look exactly as before; inventories and containers are unchanged. People before objects and stacking duplicates were considered and rejected: both change how every room looks, and stacking still overflows on a floor of distinct items. Reviewed on a demo server (1200 objects, mobs and a player in the room). Testing: RoomContents.ALookAtACrowdedFloorListsAHundredObjectsAndCountsTheRest (150 objects) and ...ALookAtAFloorOfAHundredAndOneCountsTheLastItem (101) failed first, listing every object with no summary, and pass now; ...ALookAtAFloorOfAHundredListsEveryObject pins the boundary and passed throughout. Full gtest 932 pass, 10 fail, the same 10 as the base. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
process_output copies a descriptor's queued text (up to 16383 bytes) into the static process_output_buffer, which is only 20 bytes larger. With `set wrap` on, append_lines inserts a "\n\r" for every 80 characters without a line break, into that same buffer with no bounds check. A full queue plus the snoop prefix, a leading break, "**OVERFLOW**", "\n\r" and the NUL already uses 19 of the 20 spare bytes, so a single wrap break wrote past the end of the array. It needs a wrap player (1 of 807 account characters, 52 of 4809 legacy files on live) and a full buffer with a long line -- a crowded floor, for one. In the current binary the next bytes are padding and process_input_tmp, so it was probably silent, but it is out-of-bounds memory. append_lines now takes the space it may use, NUL included, stops before a character or break that would not fit, and returns whether all of the text fitted. process_output passes the buffer size less the prefix and the room for "**OVERFLOW**" and "\n\r", and shows "**OVERFLOW**" when the wrapped text was cut short, the same marker as a full queue. Output that fits wraps exactly as before. Testing: new tests/process_output_tests.cpp. ProcessOutput.AWrappedOverflowingQueueStaysInsideTheOutputBuffer fills a wrapping player's queue with 150-character lines until it overflows and flushes it with no socket. It failed first (no NUL inside the buffer, no overflow marker) and passes now. ProcessOutput.WrapsALongLineAtEightyCharacters pins the wrapping and passed throughout. Full gtest 934 pass, 10 fail, the same 10 as the base. Makefile server build is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ploits Fix/migration verify objects exploits
The account layer validated character names with the account-name rule, which requires 3+ characters. Live data holds legacy characters older than that rule (Ao, El, Fy, Ia, Li, Pi, Ru), so linking or migrating any of them failed with "Character name Account names must be at least 3 characters long." An account record listing one would also have become unreadable. Existing character names now go through is_valid_character_name: the same path-safety rules (letters, digits, '-', '_', at most 20) with no length minimum. Account names, and the NEW name in an account rename, keep the 3-character rule; new character creation is still held to MIN_NAME_LENGTH by valid_name (ban.cpp). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The conversion guard memcmps the whole profs block, but character JSON carries only MAGE..WARRIOR, so PROF_GENERAL's slot 0 always reads back as 0. The legacy saver still writes that slot and 53 live characters hold junk there (-27008, 32000, 1, ...), so every one of them was refused with "field 'profs' does not survive the round trip" -- among them Li, and recently played characters such as Felagund and Olorin. No accessor reads slot 0 (get/set_prof_level and get_prof_coof answer PROF_GENERAL without touching the arrays), so it joins the documented accepted losses. Slots 1..MAX_PROFS are still compared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… test deploy and revert now refuse every env except test, zzz-forge-test and zzz-forge-test-4k with a usage error before anything runs. live, 4k and coders stay in the table so they can be approved later. deploy --restart runs `sudo systemctl restart rotsbuilding` over a tty as step 9, after the tag. Only test has a service; the forge-test targets refuse the flag. Without it nothing restarts, as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Feat/deploy script
… gaps Two more reasons the conversion guard refused live characters over data that cannot survive character JSON and that the game never uses: - act/pref bits with no kPlayerFlags/kPreferenceFlags entry are dropped by the writer. PRF_NOTHING2 (pref bit 6, read by nothing) is set on 58 live characters and undefined act bit 22 on one, so all 59 were refused with "field 'specials2' does not survive the round trip". The guard now ignores bits outside serializable_player_flag_mask()/serializable_preference_flag_mask(); named bits are still compared. - Character JSON reloads affects packed from slot 0, but the legacy loader places each at the index in the file, so 15 live characters with an empty slot before a used one were refused over 'affected'. Positions mean nothing to the game (store_to_char loads every non-empty slot). The guard accepts the readback only when the source's non-empty affects, packed in order, match it field for field, so a changed or missing affect is still refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 9's sudo asks again because each ssh -t is a new terminal. Document the sudoers drop-in that allows only `systemctl restart rotsbuilding` without a password, how to check it, and how to undo it. Logins stay placeholders, as elsewhere in the spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deploy tags were local only, so other deployers and devs could not see what went out or when. Step 8 now shares them through returnoftheshadow/RotS_Live, named by URL because remote names differ between checkouts: - Before naming the tag, fetch the env's <prefix>* tags (and no others) from the main repo, so a name another deployer already pushed is not reused. - After tagging, push that one tag. The deploy is already built by then, so a failed fetch or push only warns (a failed push prints the command to push by hand) and the restart still runs; the final line says when a tag stayed local. Untagged forge-test targets neither fetch nor push. --dry-run prints both commands and runs neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat(deploy): push deploy tags to the main repo
load_scalp indexed mob_proto with real_mobile's result without checking for -1, so a scalp whose mob has since been removed from the world read mob_proto[-1]. On live data Keeler's scalp (mob 30243, no longer in the world) happened to read a zero bodytype and was dropped at login with "LOAD ERROR, equipment lost"; a different neighbour could have used garbage as the mob's name or crashed. Such a scalp now falls through to the existing "An old skull" branch, the same one a deleted player's head already takes, and keeps its original head number. Scalps of existing mobs and bodyless mobs are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-character-names Let every existing character reach an account: short names, and four migration blockers
deploy and revert take an optional --password <password>. It answers the ssh master connection's password prompt so a run does not stop for it; sudo (step 3's chown) still asks for its own password. Without the flag nothing changes. The value reaches ssh through SSH_ASKPASS with SSH_ASKPASS_REQUIRE=force: a helper in the private temp directory prints the ROTS_DEPLOY_PASSWORD environment variable, so the password is never written to disk or put on the ssh command line. -o NumberOfPasswordPrompts=1 makes a wrong password fail at once instead of being sent to the server three times. The dry run shows those options but never the password. Trade-offs: the flag skips the password prompt that served as the deploy's confirmation, and the value is still in the deployer's shell history and visible to local ps while the script runs. Tested: 161 deploy tests pass (8 new), including a stand-in ssh that reads the password through the helper. Against real OpenSSH and a local sshd with a bogus user and no terminal, ssh called the helper exactly once and failed cleanly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat(deploy): --password answers the ssh login prompt
damage() gave any NPC that damaged a charmed follower a one-in-eleven chance, on every damage event, to drop the pet and hit() the pet's master instead, provided the master was in the same room. For melee the attacker had already spent its energy on the swing, so hit() skipped the attack and only called set_fighting() on both sides: the master was engaged with no message at all, even while standing idle, and the swing that triggered it did no damage to the pet. Players with recruited orcs reported this as the pets failing to rescue them. The switch is removed. A mob fighting a charmed pet now stays on the pet, and its damage lands on the pet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(combat): stop mobs switching from a charmed pet to its master
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
3 participants
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.
No description provided.