Add project health audit findings and cleanup plan
This commit is contained in:
parent
edbad4666f
commit
baf1c6ea25
1 changed files with 164 additions and 0 deletions
164
docs/plans/project-health-audit.rst
Normal file
164
docs/plans/project-health-audit.rst
Normal file
|
|
@ -0,0 +1,164 @@
|
||||||
|
Project Health Audit
|
||||||
|
====================
|
||||||
|
|
||||||
|
Feb 2026. Full codebase audit after combat-timing-fields landed.
|
||||||
|
|
||||||
|
The architecture is solid. The problem is drift: things got built ahead of
|
||||||
|
being cared about, so docs/tests/schemas are out of sync with actual code.
|
||||||
|
Nothing is broken, but the project needs a cleanup pass before pushing
|
||||||
|
forward.
|
||||||
|
|
||||||
|
|
||||||
|
Combat: Doc/Schema Mismatch
|
||||||
|
----------------------------
|
||||||
|
|
||||||
|
The most concrete issue. Three things are wrong:
|
||||||
|
|
||||||
|
1. **State machine docs are stale.** combat.rst describes
|
||||||
|
IDLE -> TELEGRAPH -> WINDOW -> RESOLVE (4 states). Code has
|
||||||
|
IDLE -> PENDING -> RESOLVE (3 states). TELEGRAPH and WINDOW got
|
||||||
|
collapsed into PENDING. The docs need to match reality.
|
||||||
|
|
||||||
|
2. **Field names drifted.** Old schema used ``timing_window_ms`` for
|
||||||
|
everything. New schema split it: ``hit_time_ms`` for attacks (time to
|
||||||
|
impact), ``active_ms``/``recovery_ms`` for defenses (active window /
|
||||||
|
lockout). builder-manual.md and combat.rst still reference the old name.
|
||||||
|
|
||||||
|
3. **No schema validation.** ``moves.py`` checks 3 required fields (name,
|
||||||
|
move_type, stamina_cost). You can define an attack with hit_time_ms=0
|
||||||
|
and it silently does nothing. Attacks should require hit_time_ms > 0,
|
||||||
|
defenses should require active_ms > 0.
|
||||||
|
|
||||||
|
Other combat notes:
|
||||||
|
|
||||||
|
- Stamina deduction is asymmetric: attacks deduct inside
|
||||||
|
``encounter.attack()``, defenses deduct in the command handler before
|
||||||
|
calling ``encounter.defend()``. Works but confusing. Consider
|
||||||
|
standardizing.
|
||||||
|
- Defense telegraphs are all empty strings. Intentional (reactive moves
|
||||||
|
don't broadcast intent) but undocumented.
|
||||||
|
- Future features (stuns, combos, lethal, multi-combatant) are NOT
|
||||||
|
half-implemented. No stubs or dead code. Clean.
|
||||||
|
|
||||||
|
Files to touch::
|
||||||
|
|
||||||
|
docs/how/combat.rst - fix state machine, fix field names
|
||||||
|
docs/builder-manual.md - fix TOML schema examples
|
||||||
|
src/mudlib/combat/moves.py - add per-move-type validation
|
||||||
|
|
||||||
|
|
||||||
|
Tests: Mixed Quality
|
||||||
|
--------------------
|
||||||
|
|
||||||
|
~1,448 test functions across 100+ files. The good tests are genuinely good
|
||||||
|
(combat encounter, quetzal roundtrip, content loading). But there's bloat.
|
||||||
|
|
||||||
|
**Trivial constructor tests (~50+).** Things like::
|
||||||
|
|
||||||
|
def test_thing_creation_minimal():
|
||||||
|
t = Thing(name="rock")
|
||||||
|
assert t.name == "rock"
|
||||||
|
|
||||||
|
These verify Python dataclasses work. Delete them.
|
||||||
|
|
||||||
|
**Over-mocking.** Many tests mock the writer then assert "was something
|
||||||
|
written?" but not what. Color bugs, format bugs slip through. Loose
|
||||||
|
assertions like ``assert "\033[" in result`` check some ANSI was emitted
|
||||||
|
but not the right color.
|
||||||
|
|
||||||
|
**Duplicated fixtures.** ``mock_writer`` is defined in 15+ test files
|
||||||
|
instead of sharing from conftest. Same for ``mock_reader`` and zone helpers.
|
||||||
|
|
||||||
|
**File sprawl.** 4 container test files, 3 portal files, 3 help files,
|
||||||
|
4 zone files. These should consolidate into 1-2 per feature area.
|
||||||
|
|
||||||
|
**Stub files (1 test each).** Seven files with a single test: test_corpse,
|
||||||
|
test_npc_behavior, test_npc_integration, test_import, test_mobs,
|
||||||
|
test_game_compatibility, test_two_way_portals. Either flesh out or delete.
|
||||||
|
|
||||||
|
**Missing test categories:**
|
||||||
|
|
||||||
|
- Error paths (bad input, wrong types, missing data)
|
||||||
|
- Edge cases (boundary values, empty inputs, max values)
|
||||||
|
- Concurrency (async code with no race condition tests)
|
||||||
|
|
||||||
|
Plan::
|
||||||
|
|
||||||
|
1. Consolidate mock_writer/mock_reader into conftest
|
||||||
|
2. Delete trivial constructor/property tests
|
||||||
|
3. Merge fragmented test files (containers, portals, help, zones)
|
||||||
|
4. Decide on stub files: flesh out or remove
|
||||||
|
5. Strengthen loose assertions where practical
|
||||||
|
|
||||||
|
|
||||||
|
Documentation: Strong Foundation, Gaps
|
||||||
|
---------------------------------------
|
||||||
|
|
||||||
|
**Current and good:** DREAMBOOK, architecture-plan, object-model,
|
||||||
|
builder-manual, protocols, persistence, terrain-generation, IF docs,
|
||||||
|
all lessons.
|
||||||
|
|
||||||
|
**Stale:**
|
||||||
|
|
||||||
|
- combat.rst (wrong state machine, wrong field names)
|
||||||
|
- builder-manual.md (old timing_window_ms references)
|
||||||
|
- roadmap.rst (phases 1-2 done but not marked)
|
||||||
|
- IF research docs (viola/zvm/mojozork audits predate implementation)
|
||||||
|
|
||||||
|
**Missing docs for existing systems:**
|
||||||
|
|
||||||
|
- NPC/mob system (mobs.py, npc_behavior.py, npc_schedule.py, dialogue.py,
|
||||||
|
conversation.py, mob_ai.py - 6+ files, no implementation doc)
|
||||||
|
- Thing/verb system (thing.py, things.py, verbs.py, verb_handlers.py)
|
||||||
|
- Time/season/weather (gametime.py, seasons.py, weather.py, timeofday.py,
|
||||||
|
visibility.py)
|
||||||
|
- Effects system (effects.py)
|
||||||
|
- Targeting (targeting.py)
|
||||||
|
- Loot/corpse (loot.py, corpse.py)
|
||||||
|
- Crafting implementation (crafting.py - builder-manual has usage but no
|
||||||
|
internals doc)
|
||||||
|
- Content loading pipeline (content.py)
|
||||||
|
- Embedded z-machine (50+ files in zmachine/, no implementation guide)
|
||||||
|
|
||||||
|
Plan: write docs as we touch each system, not all at once. Priority order
|
||||||
|
matches what we're likely to work on next (combat first, then mobs/things).
|
||||||
|
|
||||||
|
|
||||||
|
Architecture: Solid
|
||||||
|
-------------------
|
||||||
|
|
||||||
|
No action needed on architecture itself. Notes for awareness:
|
||||||
|
|
||||||
|
- Module-level globals (players, mobs, active_encounters, command registry)
|
||||||
|
are appropriate for a game server. Tests clear them properly.
|
||||||
|
- server.py is 676 lines and dense (login, shell loop, content loading) but
|
||||||
|
responsibilities are clear. Could split later if it grows.
|
||||||
|
- Player dataclass has 29 fields. Organized but growing. Watch it.
|
||||||
|
- Mode stack is string-based ("normal", "combat", "editor"). Works fine,
|
||||||
|
enum would be safer eventually.
|
||||||
|
- No dead code in core engine. Z-machine has TODOs (expected).
|
||||||
|
- No circular dependency issues. Lazy imports in commands/ handle cycles.
|
||||||
|
|
||||||
|
|
||||||
|
Execution Order
|
||||||
|
---------------
|
||||||
|
|
||||||
|
Phase 1: Combat docs and schema (small, unblocks combat work)
|
||||||
|
|
||||||
|
- Update combat.rst state machine and field names
|
||||||
|
- Update builder-manual.md TOML examples
|
||||||
|
- Add schema validation in moves.py
|
||||||
|
- Tests for validation
|
||||||
|
|
||||||
|
Phase 2: Test cleanup (can be gradual)
|
||||||
|
|
||||||
|
- Consolidate fixtures into conftest
|
||||||
|
- Delete trivial tests
|
||||||
|
- Merge fragmented test files
|
||||||
|
- Strengthen assertions
|
||||||
|
|
||||||
|
Phase 3: Doc gaps (as-needed, per system)
|
||||||
|
|
||||||
|
- Write how/ docs when touching each system
|
||||||
|
- Mark completed roadmap phases
|
||||||
|
- Consider archiving pre-implementation IF research docs
|
||||||
Loading…
Reference in a new issue