Skip to content

Fix runtime packet handling and swimming state#37

Open
llucasandersen wants to merge 2 commits into
TuffNetwork:mainfrom
llucasandersen:fix/runtime-issues
Open

Fix runtime packet handling and swimming state#37
llucasandersen wants to merge 2 commits into
TuffNetwork:mainfrom
llucasandersen:fix/runtime-issues

Conversation

@llucasandersen

@llucasandersen llucasandersen commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

This fixes the async block read path, the broken section block update decoding below Y0, the unreliable Viablocks chunk send path, and the swim state regression. It also cuts cache churn when the relevant features are off or no Tuff players are active.

Closes #21
Closes #14
Closes #8
Closes #5
Refs #17
Refs #30

Summary by CodeRabbit

  • Bug Fixes

    • Improved plugin messaging channel cleanup on shutdown
    • Enhanced swimming state tracking and synchronization
    • Optimized chunk and block change packet handling
    • Refined conditional feature management based on configuration settings
  • Tests

    • Established JUnit 5 testing framework
    • Added test coverage for block coordinate decoding

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR fixes thread safety issues in async block-change handling by moving Via data resolution to the main thread, refactors swimming state management via periodic scheduler tasks, adds resource and state guards throughout ViaBlocks and Y0 plugins, and establishes JUnit 5 testing infrastructure.

Changes

Core Thread Safety and State Management

Layer / File(s) Summary
Test Infrastructure Setup
build.gradle, src/test/java/tf/tuff/netty/ChunkHandlerTest.java
JUnit 5 dependencies and test task configuration added; initial test cases for coordinate decoding with negative values.
ChunkHandler Async Main-Thread Resolution
src/main/java/tf/tuff/netty/ChunkHandler.java
Introduces BlockChangePosition record and coordinate decoding utilities (readVarLong, position decoders). Adds isViaActive() helper and refactors single/multi-block change handling to resolve Via data on the Bukkit main thread via scheduler, then write on Netty event loop with buffer retention/release guarantees.
CustomBlockListener Thread-Safe Callback Delivery
src/main/java/tf/tuff/viablocks/CustomBlockListener.java
Unifies chunk delivery via cacheChunkWithCallback with new deliverCallback helper ensuring primary-thread execution. Updates sendChunksBatched, handleChunkLoad, processChunkForSinglePlayer to use callback pattern. Adds hasViaBlocksPlayersInWorld guard and refines chunk iteration order (x/z/y nesting) and air-block filtering (CAVE_AIR, VOID_AIR).
ViaBlocks Plugin State and Resource Guards
src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java
Conditionally creates chunkExecutor only when enabled flag is true (otherwise null). Clears viaBlocksEnabledPlayers on reload and disable. Adds early-return guards in enable flow and handlePacket. Protects /viablocks refresh command with enabled check.
Y0 Plugin World Presence Optimization
src/main/java/tf/tuff/y0/Y0Plugin.java
handleChunkLoad skips caching work when no ready players are present in the chunk's world; adds hasReadyPlayersInWorld helper to check online ready-player presence.
Swimming State Management via Periodic Scheduler
src/main/java/tf/tuff/tuffactions/swimming/Swimming.java
Introduces swimStateTask field and new enable(boolean) override to start repeating scheduler task. disable() cancels task and clears tracked swimmers. Refactors handleSwimState to delegate state application and broadcasting to new helpers. handleToggleSwim schedules main-thread re-application when needed. Adds maintainSwimmingState() for periodic reconciliation of offline/out-of-water players and applySwimmingState() for conditional state updates.
Plugin Lifecycle Cleanup
src/main/java/tf/tuff/TuffX.java
Unregisters plugin's incoming messaging channel during onDisable() shutdown.

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix runtime packet handling and swimming state' clearly summarizes the main changes: addressing runtime packet issues and swimming state problems.
Linked Issues check ✅ Passed All linked issue requirements are addressed: #21 (async world reads) fixed via main-thread scheduling in ChunkHandler/Swimming, #14 (Y<0 block decoding) fixed with new decoding utilities, #8 (Viablocks visibility) and #5 (swimming) both remedied through refactored state management.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issues: block update handling, async corrections, swimming state, and Viablocks visibility. No extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)
src/main/java/tf/tuff/netty/ChunkHandler.java (1)

160-160: 📐 Maintainability & Code Quality | 💤 Low value

Unnecessary decode call.

decodeSectionCoordX(chunkSectionPos) is called but its result is discarded. The comment says it "validates bit layout," but no validation exception is thrown, and the coordinates are decoded again inside decodeMultiBlockChangePosition. This call can be removed.

Suggested fix
-        decodeSectionCoordX(chunkSectionPos); // validates bit layout alongside the packed entries below
         buf.readBoolean();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/netty/ChunkHandler.java` at line 160, Remove the
redundant call to decodeSectionCoordX(chunkSectionPos) — it returns a value that
is discarded and the coordinates are already decoded inside
decodeMultiBlockChangePosition; delete the standalone call and any comment that
implies validation occurs there, leaving decodeMultiBlockChangePosition to
perform the decoding/validation for chunkSectionPos.
src/main/java/tf/tuff/TuffX.java (1)

93-105: 📐 Maintainability & Code Quality | ⚡ Quick win

Consider also unregistering outgoing plugin channels for completeness.

The incoming channel cleanup is correct, but sub-plugins like ViaEntitiesPlugin register outgoing channels with TuffX as the plugin owner. For defensive cleanup, consider also calling getServer().getMessenger().unregisterOutgoingPluginChannel(this); to ensure all registered channels are cleaned up during shutdown.

♻️ Proposed enhancement
     PacketEvents.getAPI().terminate();
     getServer().getMessenger().unregisterIncomingPluginChannel(this);
+    getServer().getMessenger().unregisterOutgoingPluginChannel(this);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/TuffX.java` around lines 93 - 105, The onDisable method
currently unregisters incoming plugin channels but misses outgoing channels;
update onDisable in TuffX.java (method onDisable) to also call
getServer().getMessenger().unregisterOutgoingPluginChannel(this) (in addition to
the existing unregisterIncomingPluginChannel(this)) so any outgoing channels
registered by sub-plugins (e.g., ViaEntitiesPlugin) are cleaned up; ensure this
call is placed alongside the existing messenger cleanup and executed before
returning from onDisable.
src/main/java/tf/tuff/tuffactions/swimming/Swimming.java (2)

127-142: 📐 Maintainability & Code Quality | 💤 Low value

Use explicit Iterator for cleaner removal during iteration.

While ConcurrentHashMap.newKeySet() won't throw ConcurrentModificationException, removing via the set reference (rather than iterator.remove()) during iteration can lead to skipped elements in edge cases. Using an explicit Iterator provides more predictable behavior.

♻️ Suggested refactor using Iterator
 private void maintainSwimmingState() {
-    for (UUID playerId : swimmingPlayers) {
+    var iterator = swimmingPlayers.iterator();
+    while (iterator.hasNext()) {
+        UUID playerId = iterator.next();
         Player player = Bukkit.getPlayer(playerId);
         if (player == null || !player.isOnline()) {
-            swimmingPlayers.remove(playerId);
+            iterator.remove();
             continue;
         }
         if (!player.isInWater()) {
-            swimmingPlayers.remove(playerId);
+            iterator.remove();
             applySwimmingState(player, false);
             broadcastSwimState(player, false);
             continue;
         }
         applySwimmingState(player, true);
     }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/tuffactions/swimming/Swimming.java` around lines 127 -
142, The loop in maintainSwimmingState removes from swimmingPlayers directly
while iterating, which can skip elements; change it to use an explicit Iterator
over swimmingPlayers (Iterator<UUID> it = swimmingPlayers.iterator()) and call
it.remove() when you need to remove the current UUID; keep the same logic
calling Bukkit.getPlayer(playerId), applySwimmingState(player, false/true) and
broadcastSwimState(player, false) but ensure removals use iterator.remove() so
iteration is predictable.

79-90: 🎯 Functional Correctness | 💤 Low value

Consider cancelling the event to prevent state flicker.

The current approach lets the event proceed (setting swimming to false), then schedules a task to re-apply swimming. This creates a brief one-tick window where the player's swimming state is false. Cancelling the event directly would be more responsive and prevent the intermediate state.

♻️ Alternative approach using event cancellation
 public void handleToggleSwim(EntityToggleSwimEvent event) {
     if (!isEnabled()) return;
     if (!(event.getEntity() instanceof Player)) return;
     Player player = (Player) event.getEntity();
     if (!event.isSwimming() && swimmingPlayers.contains(player.getUniqueId())) {
-        plugin.getServer().getScheduler().runTask(plugin, () -> {
-            if (swimmingPlayers.contains(player.getUniqueId()) && player.isOnline()) {
-                applySwimmingState(player, true);
-            }
-        });
+        event.setCancelled(true);
     }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/tuffactions/swimming/Swimming.java` around lines 79 -
90, The handleToggleSwim method presently lets EntityToggleSwimEvent run then
re-applies swimming on the next tick causing a flicker; change the logic so when
the event is coming from a Player and you detect that swimming should remain
true (i.e., !event.isSwimming() &&
swimmingPlayers.contains(player.getUniqueId())), call event.setCancelled(true)
on that EntityToggleSwimEvent to prevent the state flip and then ensure you call
applySwimmingState(player, true) (immediately or via the existing scheduler if
needed) so the player stays in the expected swimming state without the one-tick
false window; update references in handleToggleSwim and keep applySwimmingState
usage consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/java/tf/tuff/netty/ChunkHandler.java`:
- Around line 172-202: The leak is caused by double-retaining the ByteBuf:
resolveViaDataOnMainThread retains buf and then writeWithViaOnly retains it
again for the composite; remove the extra retain in writeWithViaOnly so the
composite takes the caller's ownership, and update resolveViaDataOnMainThread so
you do not release the retained buffer in the finally block when you call
writeWithViaOnly (ensure only the code path that does currentCtx.write(...)
releases the retained buffer). Touch the methods resolveViaDataOnMainThread and
writeWithViaOnly to transfer ownership consistently (caller retains once, callee
must not retain) and ensure all other paths still release appropriately.

In `@src/main/java/tf/tuff/tuffactions/swimming/Swimming.java`:
- Around line 40-46: The enable method's null check on swimStateTask can miss
cases where the Task object exists but the scheduled task was cancelled; update
the scheduling guard in enable(...) to schedule a new task when swimStateTask is
null OR swimStateTask.isCancelled() is true, using the same call to
plugin.getServer().getScheduler().runTaskTimer(...) to assign swimStateTask;
reference swimStateTask, enable, maintainSwimmingState and the scheduler
runTaskTimer call when making the change.

In `@src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java`:
- Around line 82-83: Reload currently clears the viaBlocksEnabledPlayers set in
onTuffXReload which causes online players to lose ViaBlocks-enabled state and
stop receiving chunk/block packets until they re-handshake; instead preserve or
rebuild enabled-player state across reloads by iterating online players after
reload and repopulating viaBlocksEnabledPlayers (or trigger a re-handshake) for
those where CustomBlockListener.hasViaBlocksPlayersInWorld(...) /
plugin.isPlayerEnabled(...) previously returned true; update onTuffXReload to
either restore the set from a persisted snapshot or call the same logic used in
handlePacket -> setPlayerEnabled(player, true) for each online player so
CustomBlockListener.handleChunkLoad continues to prepare caches without
requiring clients to resend viablocks:handshake.

---

Nitpick comments:
In `@src/main/java/tf/tuff/netty/ChunkHandler.java`:
- Line 160: Remove the redundant call to decodeSectionCoordX(chunkSectionPos) —
it returns a value that is discarded and the coordinates are already decoded
inside decodeMultiBlockChangePosition; delete the standalone call and any
comment that implies validation occurs there, leaving
decodeMultiBlockChangePosition to perform the decoding/validation for
chunkSectionPos.

In `@src/main/java/tf/tuff/tuffactions/swimming/Swimming.java`:
- Around line 127-142: The loop in maintainSwimmingState removes from
swimmingPlayers directly while iterating, which can skip elements; change it to
use an explicit Iterator over swimmingPlayers (Iterator<UUID> it =
swimmingPlayers.iterator()) and call it.remove() when you need to remove the
current UUID; keep the same logic calling Bukkit.getPlayer(playerId),
applySwimmingState(player, false/true) and broadcastSwimState(player, false) but
ensure removals use iterator.remove() so iteration is predictable.
- Around line 79-90: The handleToggleSwim method presently lets
EntityToggleSwimEvent run then re-applies swimming on the next tick causing a
flicker; change the logic so when the event is coming from a Player and you
detect that swimming should remain true (i.e., !event.isSwimming() &&
swimmingPlayers.contains(player.getUniqueId())), call event.setCancelled(true)
on that EntityToggleSwimEvent to prevent the state flip and then ensure you call
applySwimmingState(player, true) (immediately or via the existing scheduler if
needed) so the player stays in the expected swimming state without the one-tick
false window; update references in handleToggleSwim and keep applySwimmingState
usage consistent.

In `@src/main/java/tf/tuff/TuffX.java`:
- Around line 93-105: The onDisable method currently unregisters incoming plugin
channels but misses outgoing channels; update onDisable in TuffX.java (method
onDisable) to also call
getServer().getMessenger().unregisterOutgoingPluginChannel(this) (in addition to
the existing unregisterIncomingPluginChannel(this)) so any outgoing channels
registered by sub-plugins (e.g., ViaEntitiesPlugin) are cleaned up; ensure this
call is placed alongside the existing messenger cleanup and executed before
returning from onDisable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 80cf6668-62b1-44ac-b6d7-b087a2f792bc

📥 Commits

Reviewing files that changed from the base of the PR and between 65120c3 and 19197db.

⛔ Files ignored due to path filters (1)
  • builds/TuffXPlus-1.0.1-beta.jar is excluded by !**/*.jar
📒 Files selected for processing (8)
  • build.gradle
  • src/main/java/tf/tuff/TuffX.java
  • src/main/java/tf/tuff/netty/ChunkHandler.java
  • src/main/java/tf/tuff/tuffactions/swimming/Swimming.java
  • src/main/java/tf/tuff/viablocks/CustomBlockListener.java
  • src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java
  • src/main/java/tf/tuff/y0/Y0Plugin.java
  • src/test/java/tf/tuff/netty/ChunkHandlerTest.java

Comment on lines +172 to 202
private void resolveViaDataOnMainThread(ChannelHandlerContext ctx, ByteBuf buf, ChannelPromise promise,
java.util.concurrent.Callable<byte[]> supplier) {
ByteBuf retained = buf.retain();
Bukkit.getScheduler().runTask(viaBlocks.plugin.plugin, () -> {
byte[] data = null;
try {
if (player.isOnline() && isViaActive()) {
data = supplier.call();
}
} catch (Exception ignored) {
}
final byte[] resolvedData = data;

ChannelHandlerContext currentCtx = this.ctx != null ? this.ctx : ctx;
currentCtx.channel().eventLoop().execute(() -> {
try {
retained.resetReaderIndex();
if (resolvedData != null && resolvedData.length > 0) {
writeWithViaOnly(currentCtx, retained, promise, resolvedData);
} else {
currentCtx.write(retained, promise);
}
} catch (Exception ignored) {
} finally {
if (retained.refCnt() > 0) {
retained.release();
}
}
});
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Memory leak in writeWithViaOnly path.

When resolveViaDataOnMainThread takes the writeWithViaOnly path, there's one unreleased buffer reference:

  1. buf starts with refCnt=1
  2. Line 174: buf.retain()refCnt=2
  3. Line 314 (writeWithViaOnly): buf.retain()refCnt=3, composite takes ownership of this reference
  4. Line 197: retained.release()refCnt=2
  5. After ctx.write(composite) completes, composite releases its component → refCnt=1

The original reference is never released, causing a memory leak.

In writeWithViaOnly, you're retaining the buffer to add it to the composite, but the caller (resolveViaDataOnMainThread) also holds a retained reference. You should not retain again in writeWithViaOnly since the caller is passing ownership of its retained reference.

Proposed fix
 private void writeWithViaOnly(ChannelHandlerContext ctx, ByteBuf buf, ChannelPromise promise,
                               byte[] data) throws Exception {
     ByteBuf tail = ctx.alloc().buffer();
     tail.writeBytes(data);

     CompositeByteBuf composite = ctx.alloc().compositeBuffer();
-    composite.addComponents(true, buf.retain(), tail);
+    composite.addComponents(true, buf, tail);

     ctx.write(composite, promise);
 }

And adjust resolveViaDataOnMainThread to not release when writing via writeWithViaOnly (since composite takes ownership), or remove the finally release entirely and let the write operations handle it:

             try {
                 retained.resetReaderIndex();
                 if (resolvedData != null && resolvedData.length > 0) {
                     writeWithViaOnly(currentCtx, retained, promise, resolvedData);
                 } else {
                     currentCtx.write(retained, promise);
                 }
             } catch (Exception ignored) {
-            } finally {
-                if (retained.refCnt() > 0) {
-                    retained.release();
-                }
+            } catch (Exception e) {
+                if (retained.refCnt() > 0) {
+                    retained.release();
+                }
             }

Also applies to: 308-317

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/netty/ChunkHandler.java` around lines 172 - 202, The
leak is caused by double-retaining the ByteBuf: resolveViaDataOnMainThread
retains buf and then writeWithViaOnly retains it again for the composite; remove
the extra retain in writeWithViaOnly so the composite takes the caller's
ownership, and update resolveViaDataOnMainThread so you do not release the
retained buffer in the finally block when you call writeWithViaOnly (ensure only
the code path that does currentCtx.write(...) releases the retained buffer).
Touch the methods resolveViaDataOnMainThread and writeWithViaOnly to transfer
ownership consistently (caller retains once, callee must not retain) and ensure
all other paths still release appropriately.

Comment on lines +40 to +46
@Override
protected void enable(boolean wasEnabled) {
if (swimStateTask == null) {
swimStateTask = plugin.getServer().getScheduler().runTaskTimer(plugin, this::maintainSwimmingState, 1L, 1L);
}
super.enable(wasEnabled);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Consider checking isCancelled() for robustness.

The null check prevents duplicate tasks in normal scenarios, but if the task was cancelled externally (e.g., scheduler cleared), swimStateTask would remain non-null while the task isn't running. Using swimStateTask.isCancelled() provides a more robust check.

🛡️ Suggested fix
 `@Override`
 protected void enable(boolean wasEnabled) {
-    if (swimStateTask == null) {
+    if (swimStateTask == null || swimStateTask.isCancelled()) {
         swimStateTask = plugin.getServer().getScheduler().runTaskTimer(plugin, this::maintainSwimmingState, 1L, 1L);
     }
     super.enable(wasEnabled);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/tuffactions/swimming/Swimming.java` around lines 40 -
46, The enable method's null check on swimStateTask can miss cases where the
Task object exists but the scheduled task was cancelled; update the scheduling
guard in enable(...) to schedule a new task when swimStateTask is null OR
swimStateTask.isCancelled() is true, using the same call to
plugin.getServer().getScheduler().runTaskTimer(...) to assign swimStateTask;
reference swimStateTask, enable, maintainSwimmingState and the scheduler
runTaskTimer call when making the change.

Comment on lines +82 to 83
viaBlocksEnabledPlayers.clear();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for player state preservation patterns during reload

# Check for similar state management during reload/enable cycles
rg -nC3 'onTuffXReload|onReload' --type=java

# Look for player state preservation or migration patterns
rg -nC3 'viaBlocksEnabledPlayers' --type=java

Repository: TuffNetwork/TuffXPlus

Length of output: 7845


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect reload/disable/handshake-related code paths for viaBlocksEnabledPlayers
sed -n '1,220p' src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java | nl -ba | sed -n '40,120p'
sed -n '220,320p' src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java | nl -ba

# Inspect the cache/guard logic in CustomBlockListener
sed -n '600,760p' src/main/java/tf/tuff/viablocks/CustomBlockListener.java | nl -ba

# Find where handshake/data channel sets player enabled
rg -n "SERVERBOUND_CHANNEL|CLIENTBOUND_CHANNEL|handshake|setPlayerEnabled|viaBlocksEnabledPlayers\.add|viaBlocksEnabledPlayers\.remove" src/main/java/tf/tuff/viablocks -S

# Look for other player-state migration/preservation during reload/enable cycles (broader than onTuffXReload)
rg -nC3 "(onTuffXReload|onEnable|onDisable|reload)" src/main/java/tf/tuff -S --type=java

Repository: TuffNetwork/TuffXPlus

Length of output: 108


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== ViaBlocksPlugin.java: onTuffXReload / disable around viaBlocksEnabledPlayers.clear ==="
sed -n '1,220p' src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java | cat -n | sed -n '40,120p'
sed -n '1,220p' src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java | cat -n | sed -n '140,210p'
sed -n '1,220p' src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java | cat -n | sed -n '210,280p'

echo
echo "=== CustomBlockListener.java: hasViaBlocksPlayersInWorld and related guards ==="
sed -n '560,740p' src/main/java/tf/tuff/viablocks/CustomBlockListener.java | cat -n | sed -n '1,220p'

echo
echo "=== Handshake / setPlayerEnabled usage to see how online players get re-added ==="
rg -n "setPlayerEnabled\\(|viaBlocksEnabledPlayers\\.add\\(|viaBlocksEnabledPlayers\\.remove\\(|SERVERBOUND_CHANNEL|CLIENTBOUND_CHANNEL|handshake" src/main/java/tf/tuff/viablocks -S --type=java

echo
echo "=== Any other reload/enable patterns elsewhere (broader signal) ==="
rg -nC3 "(onTuffXReload|onEnable|onDisable|reload\\b)" src/main/java/tf/tuff -S --type=java

Repository: TuffNetwork/TuffXPlus

Length of output: 19721


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== ViaBlocksPlugin.java: incoming handshake handling around CLIENTBOUND/SERVERBOUND registration ==="
sed -n '100,190p' src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java | cat -n

echo
echo "=== Call sites: hasViaBlocksPlayersInWorld(...) to see what it guards ==="
rg -n "hasViaBlocksPlayersInWorld\\(" src/main/java/tf/tuff/viablocks/CustomBlockListener.java src/main/java/tf/tuff/viablocks/*.java

echo
echo "=== CustomBlockListener: chunk caching logic around those guards ==="
# Print a wider window where caching likely happens
sed -n '300,560p' src/main/java/tf/tuff/viablocks/CustomBlockListener.java | cat -n

Repository: TuffNetwork/TuffXPlus

Length of output: 16460


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== CustomBlockListener.java: context around hasViaBlocksPlayersInWorld call site ==="
sed -n '150,220p' src/main/java/tf/tuff/viablocks/CustomBlockListener.java | cat -n

Repository: TuffNetwork/TuffXPlus

Length of output: 3584


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "viaBlocksEnabledPlayers\\.(add|remove|clear)" src/main/java/tf/tuff/viablocks -S

Repository: TuffNetwork/TuffXPlus

Length of output: 621


Consider UX impact of clearing viaBlocksEnabledPlayers on reload
ViaBlocksPlugin.onTuffXReload() clears viaBlocksEnabledPlayers (and the CustomBlockListener cache). That set is then required to drive custom rendering: CustomBlockListener.handleChunkLoad only prepares chunk cache when hasViaBlocksPlayersInWorld(...) finds an online player in the set, and most update paths gate on plugin.isPlayerEnabled(player). As a result, online players won’t receive ViaBlocks chunk/block packets after a reload until they send the viablocks:handshake again (handled by handlePacket()setPlayerEnabled(player, true)). If this full state reset is intended, it should be explicitly accounted for; otherwise, preserve/rebuild the enabled-player state across reloads (or trigger re-handshake for connected clients).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/tf/tuff/viablocks/ViaBlocksPlugin.java` around lines 82 - 83,
Reload currently clears the viaBlocksEnabledPlayers set in onTuffXReload which
causes online players to lose ViaBlocks-enabled state and stop receiving
chunk/block packets until they re-handshake; instead preserve or rebuild
enabled-player state across reloads by iterating online players after reload and
repopulating viaBlocksEnabledPlayers (or trigger a re-handshake) for those where
CustomBlockListener.hasViaBlocksPlayersInWorld(...) /
plugin.isPlayerEnabled(...) previously returned true; update onTuffXReload to
either restore the set from a persisted snapshot or call the same logic used in
handlePacket -> setPlayerEnabled(player, true) for each online player so
CustomBlockListener.handleChunkLoad continues to prepare caches without
requiring clients to resend viablocks:handshake.

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.

Thread failed main thread check: Cannot read world asynchronously [Bug] Viablocks under y0 appear exploded by wind burst Viablocks Swimming

1 participant