Skip to content

Collection of misc improvements and fixes#6415

Open
rtm516 wants to merge 7 commits into
GeyserMC:masterfrom
rtm516:fix/misc
Open

Collection of misc improvements and fixes#6415
rtm516 wants to merge 7 commits into
GeyserMC:masterfrom
rtm516:fix/misc

Conversation

@rtm516
Copy link
Copy Markdown
Member

@rtm516 rtm516 commented May 17, 2026

These issues were flagged by Claude and manually fixed by me following some of its guidence.

Fix asset utils breaking on slow zip reading

  • If the zip was constructed abnormally or we are reading over the network and this function is called before there are any bytes it can get stuck.

Validate commands the same regardless of packet type

  • The handling on CommandRequestPacket and TextPacket were different for commands, these are now the same.

Clamp lectern page number

  • Due to how bedrock and java handle lecturns differently we have to send 2x the amount of packets as the page we are requesting.
  • Previous code allows upto page 255, which would result in 510 packets. This new code limits to page 100 as thats all the book can be.

@rtm516 rtm516 added the PR: Bugfix When a PR contains a bugfix label May 17, 2026
@rtm516 rtm516 marked this pull request as ready for review May 17, 2026 17:12
Copilot AI review requested due to automatic review settings May 17, 2026 17:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a few Bedrock->Java translation edge cases and a file IO inefficiency that could lead to high CPU usage, aiming to improve stability and consistency in Geyser’s core translators/utilities.

Changes:

  • Replace InputStream.available()-based writing with InputStream#transferTo to avoid potential busy-looping/CPU spin while saving assets.
  • Align CommandRequestPacket command handling with TextPacket (normalize/trim, ignore blank, require leading slash).
  • Clamp lectern page updates to reduce potential packet spam.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
core/src/main/java/org/geysermc/geyser/util/AssetUtils.java Simplifies file saving to avoid available() pitfalls and potential CPU spin.
core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockLecternUpdateTranslator.java Adds page clamping before translating Bedrock lectern page changes to Java click packets.
core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockCommandRequestTranslator.java Normalizes/validates incoming command request strings to match TextPacket handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings May 17, 2026 20:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI review requested due to automatic review settings May 17, 2026 20:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +53 to +70
// Books on java can only be 100 pages so clamp the page number to that
// This also forces requested pages to be sequential
// This is 25 for showing page 49/50, and 50 for 99/100
int currentPage = lecternContainer.getCurrentBedrockPage();
int requestedPage = packet.getPage();
int page = MathUtils.constrain(MathUtils.constrain(requestedPage, currentPage - 1, currentPage + 1), 0, 50);

if (currentPage == requestedPage) {
// The same page means Bedrock is closing the window
InventoryUtils.sendJavaContainerClose(holder);
InventoryUtils.closeInventory(session, holder, false);
} else if (currentPage == page) {
// The requested page was clamped back to the current page so do nothing
} else {
// Each "page" Bedrock gives to us actually represents two pages (think opening a book and seeing two pages)
// Each "page" on Java is just one page (think a spiral notebook folded back to only show one page)
int newJavaPage = (packet.getPage() * 2);
int currentJavaPage = (lecternContainer.getCurrentBedrockPage() * 2);
int newJavaPage = (page * 2);
int currentJavaPage = (currentPage * 2);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Bugfix When a PR contains a bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants