#144 Update AGENTS overview

Author: tpwalke2

AGENTS.md

@@ -46,12 +46,19 @@ permission on PRs from forks by default in a public repo.
 
 ### Entry point and Minecraft hooks
 
-`BlueMapSignMarkersMod` (`DedicatedServerModInitializer`) wires three lifecycle hooks:
-- `SERVER_STARTING` → `SignProvider.loadSigns(...)` reads the world's persisted `signs.json`
+`BlueMapSignMarkersMod` (`DedicatedServerModInitializer`) wires four lifecycle hooks:
+- `SERVER_STARTING` → `SignProvider.loadSigns(...)` reads the world's persisted, region-sharded sign storage
+  (migrating a pre-sharding single `signs.json` on first boot after an upgrade)
 - `SERVER_STOPPING` → `SignProvider.saveSigns(...)` then `SignManager.stop()`
 - `BLOCK_ENTITY_LOAD` → for any loaded `SignBlockEntity`, calls `SignManager.addOrUpdate(...)`
+- `CHUNK_LOAD` → reconciles signs the mod's cache still knows about in a loading chunk against what's actually
+  there; if a tracked sign's block is gone (e.g. its region file was deleted/regenerated externally while
+  unloaded), calls `SignManager.remove(...)` for it. Addresses GitHub issue #110.
 
-The markers file path is per-world: `config/bluemapsignmarkers/<world-save-name>/signs.json`.
+Sign state is stored per-world, region-sharded: one JSON file per (dimension, 32x32-chunk region) under
+`{server_root}/bluemapsignmarkers/{level}/{dimension_namespace}/{dimension_path}/r.{regionX}.{regionZ}.json`. A
+pre-sharding single `config/bluemapsignmarkers/<world-save-name>/signs.json` is migrated in place on first boot
+after upgrading (backed up, not deleted).
 
 Two Mixins (`src/main/resources/bluemapsignmarkers.mixins.json`) catch the events the lifecycle hooks above can't:
 - `SignBlockEntityInject` injects into `SignBlockEntity.updateSignText` (a player edits a sign) → `SignManager.addOrUpdate`
@@ -62,13 +69,16 @@ Two Mixins (`src/main/resources/bluemapsignmarkers.mixins.json`) catch the event
 
 1. **`SignHelper`** builds a `SignEntry` (immutable snapshot: position/dimension key, player id, parsed front/back
    text) from a `SignBlockEntity`, running sign text through a `SignLinesParser` configured with the current
-   `MarkerGroup`s.
+   `MarkerGroup`s. `SignHelper.reloadParser()` rebuilds that parser from the current config; it's called on every
+   config reload (see below) so a sign parsed after `/bluemap reload` picks up an edited prefix/matchType.
 2. **`SignManager`** (singleton, holds a `ConcurrentMap<SignEntryKey, SignEntry>` cache of all known signs) is the
    decision point: given a new/updated `SignEntry`, it figures out whether this is an add, update, remove, or
    prefix-change (remove-then-add into a different marker group) relative to what's cached, then dispatches the
    corresponding `MarkerAction` (`AddMarkerAction`/`UpdateMarkerAction`/`RemoveMarkerAction`, built via
-   `ActionFactory`) to the BlueMap connector. It also implements `IResetHandler.reset()` to replay the whole sign
-   cache when BlueMap resets its state.
+   `ActionFactory`) to the BlueMap connector. It also implements `IResetHandler.reset()`, which BlueMap fires on
+   `/bluemap reload`: reloads config (`ConfigManager.reload()`, `SignHelper.reloadParser()`, rebuilding its
+   prefix→group lookup and `ActionFactory`) then replays the whole sign cache through the same add/update/remove
+   logic, so an edited marker-group's icon/offset/visibility/prefix takes effect without a server restart.
 3. **`BlueMapAPIConnector`** owns the `ReactiveQueue<MarkerAction>` and all actual BlueMap API calls. Because the
    BlueMap API is only available while BlueMap itself is enabled, actions are queued and only drained
    (`markerActionQueue.process()`) while `BlueMapAPI.getInstance().isPresent()`; `BlueMapAPI.onEnable`/`onDisable`
@@ -90,12 +100,17 @@ line to match the pattern (unlike `STARTS_WITH`, a regex prefix can't share its
 
 ### Sign persistence and versioning
 
-Sign state is persisted per-world as `signs.json`, wrapped in a `VersionedSignFile` envelope (`{version, data}`) so
-the format can evolve without breaking old saves. `SignProvider.loadSigns` tries, in order: the versioned-file loader
-(`VersionedFileSignEntryLoader`, handling V2→V3 migration via `Version3Converter`, and current V3 files directly),
-then falls back to `Version1SignEntryLoader` for pre-versioning files. When adding a new persisted field, bump
+Sign state is stored per-world, region-sharded (one file per dimension + 32x32-chunk region — see "Entry point"
+above), with each region file wrapped in a `VersionedSignFile` envelope (`{version, data}`) so the format can evolve
+without breaking old saves. `SignProvider.loadSigns` checks whether the storage root already has region files
+(`RegionShardedSignEntryLoader.hasSignData`); if so, it loads every region file the same version-aware way as
+before sharding — the versioned-file loader (`VersionedFileSignEntryLoader`, handling V2→V3 migration via
+`Version3Converter`, and current V3 files directly), falling back to `Version1SignEntryLoader` for pre-versioning
+files. If no region files exist yet, `LegacySignFileMigrator` reads a pre-sharding single `signs.json` (if present)
+through that same version chain, writes it out region-sharded, and backs up the legacy file (renamed, not deleted)
+only once every expected region file is confirmed on disk. When adding a new persisted field, bump
 `SignFileVersions` and add a loader/converter rather than changing an existing version's shape in place — old
-`signs.json` files on live servers must keep loading.
+region files (or a not-yet-migrated legacy `signs.json`) on live servers must keep loading.
 
 ### Adding a new marker/BlueMap action
 
@@ -107,15 +122,17 @@ compile.
 ### Testable vs. game-coupled code
 
 When adding logic, prefer keeping it in plain Java classes with no Minecraft/Fabric/BlueMap API types in their
-signature (like `SignLinesParser`, `SignEntryHelper`, `MarkerGroup`/`MarkerGroupMatchType`, `ConfigManager`/
-`ConfigProvider`, `ReactiveQueue`, the persistence loaders/converters, `ActionFactory`/`MarkerSetIdentifierCollection`)
+signature (like `SignLinesParser`/`ParsingContext`, `SignEntry`/`SignEntryHelper`, `SignChunkKey`/`SignChunkIndex`,
+`MarkerGroup`/`MarkerGroupMatchType`, `ConfigManager`/`ConfigProvider`, `ReactiveQueue`, `HtmlUtils`, `FileUtils`,
+the persistence loaders/converters (including `Version1SignEntryLoader`), `ActionFactory`/`MarkerSetIdentifierCollection`)
 — these can be unit tested directly (see `src/test/java/.../core/signs/SignLinesParserTest.java` for the pattern).
 Code that must reference game types (`SignHelper`, the mixins, `BlueMapSignMarkersMod`, `BlueMapAPIConnector`)
 should stay thin glue around the testable core, since it can only be verified manually via `runServer`.
 
-There is a documented, currently-unfixed gap here: `BlueMapAPIConnector` passes sign text into BlueMap's POI marker
-`detail` field unescaped, and BlueMap renders `detail` as raw HTML (unlike `label`, which BlueMap escapes itself) —
-see `plans/html-detail-escaping-plan.md` for the planned fix.
+`BlueMapAPIConnector` escapes sign text (`HtmlUtils.toHtmlDetail`, in `common`) before it reaches BlueMap's POI
+marker `detail` field — BlueMap renders `detail` as raw HTML (unlike `label`, which BlueMap escapes itself), and
+sign text is player-controlled, so this closes a live XSS vector. See `plans/html-detail-escaping-plan.md` for the
+design. Persisted sign data stays raw/unescaped; escaping happens only at this BlueMap API call site.
 
 ## Planning documents