diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 2f0726d..59e4ab1 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -274,7 +274,17 @@ Seeded with the 8 spec categories: `new-player-guide, maps-atlas, systems, items Seeded keys: `site_mode` (default `maintenance`), `site_mode_changed_at`, `site_mode_changed_by`, `maintenance_message`, `status_message`, `homepage_teaser`, -`contact_email` (=UOMysticmoon@gmail.com), `site_title`. +`contact_email` (=UOMysticmoon@gmail.com), `site_title`, `player_registration` +(default `disabled`), `mobile_app_links_enabled`. + +**Keys a MODULE seeds into this table.** `settings` is core's, but a module's +schema fragment may `INSERT IGNORE` its own rows into it, and module-uo seeds two: +`game_account_signup` (default `disabled`) and the one-shot migration marker +`uo_link_protocol_3_migrated`. Core seeded both until Phase 3 slice 4, which is +worth knowing for one reason beyond tidiness — a fragment runs **after** core's +schema is replayed in full, so a marker in core guarding a statement in a fragment +fires before the statement reads it. That exact ordering silently disabled the +protocol-3 migration between slices 1 and 4; see MODULE_SYSTEM.md §2.7.1. **Deliberately unseeded keys** — the theming & navigation overrides (`theme_visual`, `brand_assets`, `nav_public`, `nav_admin`, `nav_player`). All diff --git a/website/MODULE_API.md b/website/MODULE_API.md index 061405b..0755c6b 100644 --- a/website/MODULE_API.md +++ b/website/MODULE_API.md @@ -1052,6 +1052,29 @@ declare their symbols. It can talk about them in English. Core's UO-flavoured default copy is dealt with directly instead, as `MODULE_SYSTEM.md` §2.7.1's slice 4 — a rewrite with its own review, not an exemption. +**Implemented in slice 4** as `scripts/checkModuleIdentifiers.js` / `npm run check:modules`, run as +the first step of the server-tests job. Four details are contract rather than implementation, because +each was wrong in a first attempt: + +- **Matching is on whole WORDS.** Names are tokenised on camelCase humps and on `-`/`_`/`.`/`/`, and + compared word by word — so `shardStatus` is a hit and `defaultImage`, which contains the substring + "ultIma", is not. `uo`+`link` and `town`+`crier` match only as adjacent pairs; "link" on its own is + ordinary core vocabulary and a check that forbade it would be ignored within a week. +- **Comments and string bodies are masked in one left-to-right character walk, comments first.** A + comment contains quotes and a string contains `//`; no ordering of regexps gets both right. Masked + rather than deleted, so offsets survive and a route path literal is still findable at its own + position. +- **Grandfathering (§6.5) is exempted by an explicit list naming file, identifier and reason**, and + an entry that matches nothing **fails the build**. Core's `LEGACY_TABLE_PREFIXES` and + `LEGACY_STREAM_IDS`/`LEGACY_LEGS` are maps keyed by module id, so grandfathering cannot be written + down without naming who is grandfathered; that is the only sanctioned reason to add one. +- **The file list comes from `git ls-files`**, not a directory walk: an untracked, gitignored, + operator-supplied data file is not core's source and must not fail anyone's build. + +The check has its own test suite. That is not optional here — a boundary check is written when the +boundary is already clean, so it never fires again, and nothing distinguishes "still checking" from +"quietly broken" without cases it is required to reject. + ### 5.3 Zero-line route manifest diff (CI, both repos) `npm run routes:manifest -- --check` in core; the module generates and freezes its own manifest in diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index b56bfec..aa329e6 100644 --- a/website/MODULE_SYSTEM.md +++ b/website/MODULE_SYSTEM.md @@ -781,7 +781,7 @@ Each slice is one `module-uo` PR (adds), one `website` PR (deletes), and one `do | 1 | **The whole server half** | 40 files / ~9,674 lines, 25 of core's 82 test files, 27 of its 68 tables — every UO model, util, router and controller, `config/shardStreams.js`, `scripts/importSpawnAtlas.js` and the art JSON. **One merge, five commits** (below). | | 2 | **Client extension slots** | Core only, and the one slice that adds rather than moves: the client twin of `declareSlot`/`registerExtension` (API §3.7), the `site.footer.status` and `admin.users.detail` slots, and core filling both itself. `module-uo` untouched. | | 3 | **The whole client half** | **35 files / 5,332 lines** (measured; the 51/~3,700 above was counted differently) — all twelve public pages (`Shard`, `ShardActivity`, `Rules`, `Atlas`, `AtlasCreature`, `ChampSpawns`, `Market`, `MarketVendor`, `Governors`, `Guilds`, `Houses`, `Leaderboards`) under `/uo/*`, every admin and player view under `/admin/uo/*` and `/player/uo/*`, `PlayersOnline`, `VendorSales`, `CharacterStats`, `GameAccounts`, the `data/` and `lib/` UO leaves, the public nav rows, the feature provider, and all three slot fills. | -| 4 | **De-UO core's copy** | `About`, `Screenshots`, `Website`, `SiteFooter`'s prose, `heroLayout`'s defaults, `api/client.js`'s `shard`/`atlas` namespaces, and the comments in `navOverrides.js` — plus the §5.2 CI grep that keeps them out | +| 4 | **De-UO core's copy** | `About`, `Screenshots`, `Website`, `Status`, `Wiki`, `SiteFooter`'s prose, `heroLayout`'s defaults, `brand.js`'s tagline + description, `db/seed.js`'s wiki copy, two user-visible `NavEditor` strings, the comments in `navOverrides.js`, and the 190 dead lines of `api/client.js`'s `shard`/`atlas` namespaces — plus the two settings rows core seeded for module-uo, and the §5.2 CI check that keeps all of it out. `README.md` deliberately deferred to slice 5. | | 5 | **Close the phase** | `module-uo`'s frozen route manifest and release workflow; `docs/modules/uo/` and `docs/modules/rust-dryrun.md` | ##### Why the server half cannot be sliced — found 2026-08-11, before writing any of it @@ -1125,6 +1125,110 @@ no DOM, because nothing renders. It holds the agreement that rots quietly (every route this module registered) rather than restating both lists. CI now builds before it tests, since both artifact-reading tests skip without a build and were otherwise green while asking nothing. +#### Slice 4 — de-UO core's copy, and enforce it (Module-uo#5 + website#140, 2026-08-11) + +The slice that makes criterion 1 a fact rather than a promise. Core ends at **637 server + 157 +client** tests, a manifest unchanged at 158 public + 2 internal, and a byte-identical OpenAPI spec. +Three separable pieces, landed together because the check is what keeps the other two true. + +##### The bindings were dead a whole slice before anyone noticed + +`client/src/api/client.js` still carried **190 lines** of UO namespaces — `shard`, `atlas`, both SSE +URLs, `admin.shard` / `shardOps` / `atlas` / `userShard`, the uo-link and town-crier calls, +`player.shard` — with **zero core consumers** since slice 3 deleted the views that called them. +Nothing failed, nothing warned, and the client build was happy: an API binding with no caller is +inert. Worth naming as a class, because the same thing is true of any leaf a deletion slice leaves +behind, and only the §5.2 check would have found it. + +Five assertions went with them. Core's `apiClient.test.js` was still testing UO URLs — the +atlas-vs-`/shard` path split, the query-string filtering, the slug encoding, the admin atlas methods +— so deleting the bindings would have deleted the coverage too. They are `module-uo`'s +`client/test/api.test.js` now, joined by a new one pinning the seven admin URLs the shipped Android +app calls by name. The encoding test that used `governorHistory` was re-pointed at a core route +rather than dropped: what it actually guards is `req`'s encoding, which is core's. + +##### The copy, and where a game's name belongs instead + +`About`, `Screenshots`, `Website`'s cards, `Wiki`, `SiteFooter`, the default hero, `brand.js`'s +tagline and description, the seeded wiki categories, and two **user-visible** NavEditor strings that +named a module's admin screen by its proper name ("Shard Visibility"). `Status` was the interesting +one: it reports **site mode**, has never had anything to do with a game server, and was called "Shard +Status" purely by habit. + +Nothing is lost, because core already has three places an instance says which game it is: `BRAND_*` +(and `.env.uomysticmoon.example` sets both brand strings explicitly, so the live instance's wording +does not change at all), the hero editor, and CMS pages. `heroLayout.defaultLayout` is both the +rendered default *and* the editor's starting point, which is the argument for leaving the words in +code and making them neutral rather than inventing a config surface for prose. + +**Wiki page slugs are deliberately untouched.** `seedDefault*` only inserts a row that is absent, so +renaming `maps-atlas` does not rename anything — it adds a ninth page to every existing install. + +##### Two settings rows core had no business seeding — one of them broken + +`game_account_signup` and `uo_link_protocol_3_migrated` were both seeded by core's `schema.sql`. The +first is the row slice 3 explicitly kept the *key* of; the second is a one-shot migration marker. +Both move to module-uo's fragment, and the second was a live defect. + +**The marker fired before the migration that reads it.** Pre-extraction these two statements were +adjacent in core's schema: + +```sql +UPDATE uo_link_config SET protocol = 3 + WHERE id = 1 AND protocol < 3 + AND NOT EXISTS (SELECT 1 FROM settings WHERE `key` = 'uo_link_protocol_3_migrated'); +INSERT IGNORE INTO settings (`key`, value) VALUES ('uo_link_protocol_3_migrated', '1'); +``` + +Slice 1 moved the `UPDATE` to module-uo and left the `INSERT` behind. The two files do not run +together — core's schema is replayed **in full** before any module fragment (§2.6) — so the marker +existed before the guard ever read it and the one-shot could never fire. An install carrying a +protocol-2 row would have stayed pinned at 2 against a v3 sidecar: every REST call 409, the WS closed +on `ws.hello`, which is precisely what the migration exists to prevent. Latent rather than live, and +only because `edge` has not cut over: it bites an install that first boots a post-slice-1 build while +already holding a `uo_link_config` row. + +Verified against a real MariaDB, all three states: a fresh database writes both rows and has nothing +to migrate; a database with `protocol = 2` and no marker is moved to 3 and marked; and a database +deliberately pinned back to 2 **with** the marker present stays at 2 across a restart, which is the +half that makes it a one-shot rather than a re-bump. + +`schemaFragment.test.js` asserts the order, because that is the property that broke and no other kind +of test can see it — both files were individually valid SQL and both replayed cleanly. Also gone: an +orphan comment block in core's `schema.sql` describing the spawn-atlas tables slice 1 took away. + +##### The check: `scripts/checkModuleIdentifiers.js` + +`npm run check:modules`, and the **first** step of the server-tests job — before `npm ci`, because it +is plain Node over `server/` and `client/` source with no dependency of its own, so a boundary break +is the first thing a reviewer sees rather than something found under a pile of unrelated failures. +What it reads is [API §5.2](MODULE_API.md#52-zero-uo-identifiers-in-core-ci-website-repo); what it +learned by getting things wrong first is worth recording: + +- **Whole words, not substrings.** A case-insensitive substring pass flags `defaultImage`, which + contains "ultIma", and it did so four times in this repo on its first run. Names are tokenised on + camelCase humps and separators, and matched as words — with `uo`+`link` and `town`+`crier` matched + as adjacent *pairs*, since "link" alone is ordinary core vocabulary. +- **Comments and strings are masked in one character walk**, comments first. This is the + `checkImports.js` lesson for the third time: a comment contains quotes, a string contains `//`, and + the two cannot be handled in either order by regexp. Masking rather than deleting, so a route path + literal is still findable at its own offset and reported line numbers stay honest. +- **It has its own 17-test suite.** A boundary check that silently stops checking is worse than none, + and every "must not catch" case in it is a false positive an earlier version really produced. +- **The three §6.5 grandfathering allowlists are exempt by name**, with their reason — `LEGACY_TABLE_PREFIXES` + and `LEGACY_STREAM_IDS`/`LEGACY_LEGS` are maps *keyed by module id*, and there is no way to write + grandfathering down without naming who is grandfathered. An exemption that stops matching **fails + the build**: the exemption list is the only part of this check that can weaken it, so it is the part + that gets the noise. +- **It reads `git ls-files`, not a directory walk.** An operator's gitignored + `server/db/data/spawnAtlas.art.json` sits in the tree of anyone who has run the atlas import and + would otherwise report a file-name violation no commit could fix. + +**Deferred to slice 5, deliberately: `README.md`.** 48 UO mentions, including a whole +`## Shard integration (uo-link)` section and the architecture diagram. §5.2 does not cover prose, and +core's README is a rewrite that belongs with the phase-closing documentation pass rather than +half-done inside a code slice. + **Phase 4 — Delivery.** The admin-panel Modules screen (install, enable, disable, retry, purge, `startup_failed` with its recorded reason) and the Docker-environment path from §2.5. Deliberately last, so loader, packaging, schema and chunk-loading problems are not all being debugged at once.