Jmap連携修正

[tier940]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces several major features, including JourneyMap team waypoint sharing, client-side claim and party data persistence across reconnects, a client-side protection status HUD, and new Fair Play configuration options. It also resolves synchronization issues with BetterQuesting-linked parties and enforces a party requirement for claiming chunks. The review feedback highlights critical issues that need to be addressed: a potential dedicated server crash due to direct references to the client-only ProtectionStatusHud class, a bug preventing client cache saves on disconnect, and several desync risks related to waypoint limits, party membership changes, multiple waypoint deletions in a single tick, and listener cleanup.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[claude]

Claude finished @tier940's task in 4m 53s —— View job

---

Review of the Gemini findings on this PR

No specific instruction was given beyond @claude, so I read through the diff and verified each issue the automated Gemini review raised against the actual code, rather than re-stating them blindly.

Todo list

• Gather context (PR diff, changed files, Gemini review comments)
• Understand the request (bare mention → verify/expand on the existing review findings)
• Review ClientCachePersistence.java disconnect-save bug claim
• Review BLPCMod.java dedicated-server crash claim (ProtectionStatusHud)
• Review PlayerLoginHandler.java waypoint sync-on-join/leave claim
• Review WaypointAction.java rollback-on-limit claim
• Review ClientWaypointCache.java listener-clear claim
• Review JMapWaypointOutgoing.java pendingRemoveId race claim
• Post consolidated review

---

Confirmed, real bugs

1. PlayerLoginHandler — waypoints don't sync on mid-session party join/leave (src/main/java/com/github/gtexpert/blpc/common/network/PlayerLoginHandler.java:124)
Confirmed: SyncAllWaypoints is only ever sent from onPlayerLogin. PartyEvent.Post.MemberJoined/MemberLeft are posted (common/network/message/PartyAction.java:364,397,629) but nothing subscribes to them to re-sync or clear ClientWaypointCache. A player who creates/joins a party mid-session won't see its shared waypoints until they relog, and a player who leaves keeps stale waypoints from their old party forever (until relog). Gemini's fix (subscribe to the Post events and send a fresh/empty SyncAllWaypoints) is correct.
Fix this →

2. WaypointAction.Handler — hitting the per-party waypoint cap desyncs the sender (src/main/java/com/github/gtexpert/blpc/common/network/message/WaypointAction.java:147-150)
Confirmed: the non-owner path (line 137-140) correctly calls rollback(...) before returning, but the MAX_WAYPOINTS_PER_PARTY cap check just return;s. The sender's client-side JourneyMap already has the waypoint (added optimistically), so the server silently ignoring it leaves that client's local map out of sync with the server/party. Needs the same rollback(data, partyId, message.waypointId, player); call before the early return.
Fix this →

3. ClientCachePersistence — cache may silently fail to save on disconnect
Plausible and worth fixing defensively: snapshotAndWrite() (client/cache/ClientCachePersistence.java:127-129) re-derives the key via ClientCacheKey.current() rather than reusing the key captured when the connection was established. onClientDisconnect (core/CoreEventHandler.java:94-98) defers the save via Minecraft.addScheduledTask, so it runs on a later tick than the disconnect event — by which point mc.isIntegratedServerRunning()/mc.getCurrentServerData() backing ClientCacheKey.current() may already be cleared, especially for singleplayer world unload. Gemini's suggested fix (capture the key into a static field on connect, use it on save, clear it on disconnect) removes the dependency on live connection state entirely and is the safer approach regardless of whether the failure reproduces on every disconnect path.

Real, but already effectively mitigated today

4. ClientWaypointCache.clearAll() also wipes changeListeners (common/waypoint/ClientWaypointCache.java:56-59)
Confirmed as written — clearAll() clears both cache and changeListeners, which is surprising for a method whose name only suggests clearing data. In the current code this doesn't manifest as a visible bug because JMapModule explicitly unregisters and re-registers its listener around every connect/disconnect (integration/jmap/JMapModule.java:61-81), so the JMap mirror listener always comes back on reconnect. But it's fragile: any future listener that isn't tied to that same reconnect lifecycle would silently stop working after the first disconnect. Gemini's fix (only clear cache in clearAll()) is correct and low-risk to apply.

5. BLPCMod.java — new ProtectionStatusHud() referenced from common code (src/main/java/com/github/gtexpert/blpc/BLPCMod.java:69)
This is the same pattern already used two lines above for KeyInputHandler (also @SideOnly(Side.CLIENT), registered the same way inside the same if (event.getSide().isClient()) block) — so it's not a new risk this PR introduces; if this pattern reliably crashed dedicated servers via class-verification, KeyInputHandler would already be doing so today. That said, Gemini's suggested @Mod.EventBusSubscriber(modid=..., value=Side.CLIENT) + static-method pattern is the more idiomatic/defensive approach and would be worth adopting for both classes at some point — but I wouldn't block this PR on it without an actual dedicated-server boot test.

Lower priority

6. JMapWaypointOutgoing.pendingRemoveId (integration/jmap/JMapWaypointOutgoing.java:32)
Confirmed limitation: it's a single volatile String, so if more than one waypoint is deleted within the same client tick, only the last delete reaches the server. Gemini's Set<String> suggestion is correct; likely low-frequency in practice (JourneyMap's UI usually deletes one at a time) but easy to fix.

I did not implement any of these fixes since the request was a bare mention on a PR that already had review feedback — let me know if you'd like me to apply any of them.