Repository navigation
Conversation
ChunkBatchSizeCalculator.onBatchFinished starts with `if (count > 0)`; mineflayer dropped that guard, so a chunk_batch_finished with batchSize 0 does 0/0 and Math.min/Math.max propagate NaN into weightedAverage. NaN is absorbing, so every later sample keeps it and chunk_batch_received reports chunksPerTick: NaN for the rest of the session. Fixes PrismarineJS#4171
This branch has not been deployed
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.
Fixes #%d
What
lib/plugins/blocks.jshandleschunk_batch_finishedwithout the guard the vanilla client has. Vanilla'sChunkBatchSizeCalculator.onBatchFinished(int count)opens withif (count > 0) { ... }; mineflayer omits it, so a batch withbatchSize: 0computes(now - start) / 0.Because
chunk_batch_startand an emptychunk_batch_finishedare written back to back andDate.now()has millisecond resolution, the elapsed time is usually 0 as well:0 / 0isNaN,Math.min(Math.max(NaN, lo), hi)isNaN, andweightedAveragebecomes NaN. NaN is absorbing, so it stays NaN for every later sample andchunk_batch_receivedreportschunksPerTick: NaNfor the rest of the session.Fix
Skip the average update for empty batches, exactly like vanilla; still answer
chunk_batch_receivedwith the current average (vanilla does not send a reply at all, but keeping the reply preserves the existing contract with the server's batching).Verification
Reproduced by arithmetic, no server needed: run the handler twice over the same packet sequence (one empty batch, then five normal batches).
The default value comes from
weightedAverage = 2, sochunksPerTick: 7 / 2 = 3.5on the first reply.Not verified
I could not run the mocha suite here (no npm install in this environment), so this is the arithmetic reproduction only, not an integration test against a server. Happy to adjust if you would rather drop the reply for empty batches entirely.