Skip to content

#190 Stale prefixes can orphan signs - #191

Open
tpwalke2 wants to merge 4 commits into
mainfrom
bugfix/tpwalke2/190-stale-prefix
Open

#190 Stale prefixes can orphan signs#191
tpwalke2 wants to merge 4 commits into
mainfrom
bugfix/tpwalke2/190-stale-prefix

Conversation

@tpwalke2

Copy link
Copy Markdown
Owner

What

Fixes stale-prefix orphaned signs (#190): editing a marker group's prefix (especially a REGEX prefix) could orphan already-placed signs, which this PR fixes by persisting raw sign text so /bluemap reload can reparse it.

Why

SignEntry only stored the parsed label/detail, not the sign's raw text. When a group's prefix/regex was edited and the server reloaded config, SignManager had no way to reparse a sign against the new rules — it could only diff a stale cached parse, so signs that no longer matched any group silently vanished from the map and stayed gone.

Changes

  • Persistence: SignEntry now stores frontRawLines/backRawLines; bumped to SignFileVersions.V5 with a Version5Converter migrating older formats (raw lines absent/null for pre-V5 entries).
  • SignHelper: captures raw sign lines once and parses from them, instead of discarding the raw text after parsing.
  • SignManager.reloadConfig(): on /bluemap reload, reparses each cached sign from its raw text (reparseFromRawLines) against the freshly rebuilt SignLinesParser before diffing old vs. new representation, so a sign self-heals if its prefix/group now parses differently. Falls back to the old cached-parse diff when raw text isn't available (pre-V5 entries).
  • ConfigProvider: extracted effectiveType() to centralize the "null type defaults to POI" rule, removing three duplicated inline checks.
  • README: added a Troubleshooting section documenting that signs orphaned by this bug before upgrading stay broken until manually re-edited or the server restarts (no raw text to recover from) — signs created/edited on a version with this fix self-heal automatically going forward.

Testing

  • ./gradlew test — new/updated coverage in SignManagerTest (reparse-on-reload behavior), SignEntryTest, Version4/5Converter and VersionedFileSignEntryLoaderTest (V5 migration).
  • ./gradlew runServer — manually verify: place a sign under a REGEX group, edit that group's prefix in config, run /bluemap reload, confirm the sign's marker updates/moves groups instead of disappearing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Persists raw sign text and reparses cached signs after BlueMap configuration reloads to prevent stale prefixes from orphaning markers.

Changes:

  • Adds V5 persistence and migration for raw front/back sign lines.
  • Reparses signs against refreshed marker-group configuration.
  • Adds tests, troubleshooting guidance, and a minor-version bump.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
README.md Documents stale-prefix recovery.
gradle.properties Bumps mod version.
agent-context/reviews/adversarial-review-feature-tpwalke2-7-line-markers-2026-08-14.md Removes an old review artifact.
ConfigProvider.java Centralizes effective marker type.
BlueMapAPIConnector.java Uses covariant marker identifiers.
AddMarkerAction.java Adds covariant identifier accessor.
UpdateMarkerAction.java Adds covariant identifier accessor.
SignEntry.java Stores raw sign lines.
SignHelper.java Captures and parses raw lines.
SignManager.java Reparses signs during reload.
SignFileVersions.java Adds persistence V5.
RegionShardedSignEntryWriter.java Writes V5 files.
SignEntryV4.java Models the previous format.
Version1SignEntryLoader.java Migrates legacy entries through V5.
Version4Converter.java Produces V4 migration records.
Version5Converter.java Converts V4 entries to V5.
VersionedFileSignEntryLoader.java Loads and migrates through V5.
SignManagerTest.java Tests raw-line reparsing.
SignEntryTest.java Tests new fields and equality.
SignTransitionResolverTest.java Updates V5 constructors.
SignEntryHelperTest.java Updates V5 constructors.
LineGroupResolverTest.java Updates V5 constructors.
SignRegionPartitionerTest.java Updates V5 constructors.
RegionShardedSignEntryWriterTest.java Verifies V5 output.
RegionShardedSignEntryLoaderTest.java Updates persisted fixtures.
VersionedFileSignEntryLoaderTest.java Tests V4-to-V5 migration.
LegacySignFileMigratorTest.java Updates migration fixtures.
Suppressed comments (1)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignManager.java:241

  • A V5 JSON entry whose raw array contains a null element is accepted by Gson, but SignLinesParser.trimLine() then dereferences it. Because this reparse runs outside dispatchTransition()'s per-entry catch, one malformed persisted sign aborts the entire reload and prevents all later signs from being refreshed. Catch reparse failures per entry and preserve the cached parse, matching the existing malformed-entry fallback.
        var freshFront = parser.parse(entry.frontRawLines());
        var freshBack = parser.parse(entry.backRawLines());
        return entry.withParsedText(freshFront, freshBack);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignManager.java Outdated
Comment thread src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignManager.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants