Scope addon-created BlueMap marker sets to their own worlds - #3041
Merged
Conversation
createMarkerSet() attached the new set to every map on the server via api.getMaps(), and populateAll() re-attached it to every map again after a BlueMap reload. An addon registering, say, a warps marker set for one game mode put that set on the sidebar of every other game mode's map, and of any non-BentoBox world's map too. removeMarkerSet() had the same fan-out. MapHook#createMarkerSet carries no world, so the set's worlds are now learned lazily from the markers themselves: addPointMarker/addAreaMarker/addPolygonMarker record the world they were given and attach the set to just that world's maps. populateAll() replays the recorded worlds after a BlueMap reload, and removeMarkerSet() detaches from only those worlds. A set with no markers yet is no longer attached anywhere, which is also the more useful behaviour - an empty set was previously listed on every map. Island marker sets are unaffected; registerGameMode() already scoped them to the game mode's own worlds. Also guards the api field before use. createMarkerSet() and removeMarkerSet() dereferenced it unconditionally, so an addon calling either before BlueMap had loaded, or while it was reloading, hit an NPE. getBlueMapAPI() is annotated @nullable for the same reason - it was marked @nonnull but returns null in both those windows, which suppressed the null checks callers needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016pHWC6XapJj5gRtsFr74Tf
|
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
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.



Problem
BlueMapHook.createMarkerSet()attached every new marker set to every map on the server:populateAll()did the same again after a BlueMap reload, andremoveMarkerSet()had the matching fan-out. So an addon registering a warps marker set for one game mode put that set in the sidebar of every other game mode's map, and of any non-BentoBox world's map as well. A set created but never populated was listed on every map while permanently empty.Island marker sets were never affected —
registerGameMode()already scopes those to the game mode's own overworld/nether/end.Fix
MapHook#createMarkerSet(id, label)has no world parameter, so there is nothing to scope by at creation time. The worlds are instead learned lazily from the markers themselves:addPointMarker/addAreaMarker/addPolygonMarkerrecord the world they were handed and attach the set to just that world's maps.populateAll()replays the recorded worlds after a BlueMap reload instead of hittingapi.getMaps().removeMarkerSet()detaches from only the worlds the set was attached to.A set with no markers yet is now attached nowhere, which is also the more useful behaviour.
Also in here
createMarkerSet()andremoveMarkerSet()dereferenced theapifield unconditionally, so an addon calling either before BlueMap finished loading — or during a/bluemap reload, when the hook nullsapiin itsonDisablecallback — got an NPE. Both now guard, as doesaddMarkerSetToWorld.getBlueMapAPI()was annotated@NonNullbut returns null in exactly those two windows. It is now@Nullablewith the windows documented; the old annotation told null-analysis to skip the check callers actually needed. Annotation-only change, so binary compatibility is unaffected.Testing
8 new tests in
BlueMapHookTest, including the direct regression test (testAddPointMarkerDoesNotAttachSetToUnrelatedWorld: two worlds with their own maps, marker added in one, assert the other's map stays clean), reload re-attachment, detach-on-remove, and the API-not-loaded path.Full suite: 3374 tests, 0 failures.
Version
None needed —
buildVersionis already3.21.1and3.21.0is the latest release, so this lands in the pending version.Context
Spotted while investigating an admin's BlueMap report on Discord. It is not the cause of that report — that one is looking like an nginx
gzip_static always/ webroot issue on their side — but it is a real defect in the same code.🤖 Generated with Claude Code
https://claude.ai/code/session_016pHWC6XapJj5gRtsFr74Tf