1
0
Fork 0
mirror of https://github.com/GeyserMC/Geyser.git synced 2025-04-17 19:12:14 +02:00

Remove session variable used to force-close the player inventory, use confirm variable instead. Also: fix rare issues where server-side requested lectern closing didn't work for real lectern blocks

This commit is contained in:
onebeastchris 2025-04-12 13:45:45 +02:00
parent 49b9c68647
commit 176d552995
13 changed files with 35 additions and 44 deletions

View file

@ -107,19 +107,13 @@ public final class InventoryHolder<T extends Inventory> {
}
public void openInventory() {
if (session.getInventoryHolder() != this) {
throw new IllegalStateException("Inventory is not open!");
}
this.translator.openInventory(session, inventory);
this.pending = false;
this.inventory.setDisplayed(true);
}
public void closeInventory() {
if (session.getInventoryHolder() != this) {
throw new IllegalStateException("Inventory is not open!");
}
this.translator.closeInventory(session, inventory);
public void closeInventory(boolean force) {
this.translator.closeInventory(session, inventory, force);
if (session.getContainerOutputFuture() != null) {
session.getContainerOutputFuture().cancel(true);
}

View file

@ -549,14 +549,6 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
@Setter
private boolean placedBucket;
/**
* Stores whether the Java server requested the player inventory to be closed.
* Used to prevent our hacky player inventory closing workaround in {@link PlayerInventoryTranslator#closeInventory(GeyserSession, PlayerInventory)}
* to run when the closing is initated by the Bedrock client.
*/
@Setter
private boolean serverRequestedClosePlayerInventory;
/**
* Counts how many ticks have occurred since an arm animation started.
* -1 means there is no active arm swing

View file

@ -104,7 +104,7 @@ public abstract class AbstractBlockInventoryTranslator<Type extends Container> e
}
@Override
public void closeInventory(GeyserSession session, Type container) {
public void closeInventory(GeyserSession session, Type container, boolean force) {
holder.closeInventory(session, container, closeContainerType(container));
}

View file

@ -174,7 +174,7 @@ public abstract class InventoryTranslator<Type extends Inventory> {
/**
* Closes the inventory, and if necessary, cleans up the prepared inventory.
*/
public abstract void closeInventory(GeyserSession session, Type inventory);
public abstract void closeInventory(GeyserSession session, Type inventory, boolean force);
/**
* Updates a property in the inventory.

View file

@ -47,6 +47,8 @@ import org.geysermc.mcprotocollib.protocol.data.game.item.component.WritableBook
import org.geysermc.mcprotocollib.protocol.data.game.item.component.WrittenBookContent;
import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.inventory.ServerboundContainerButtonClickPacket;
import java.util.concurrent.TimeUnit;
public class LecternInventoryTranslator extends AbstractBlockInventoryTranslator<LecternContainer> {
/**
@ -83,8 +85,14 @@ public class LecternInventoryTranslator extends AbstractBlockInventoryTranslator
}
}
// Lecterns don't require a delay before opening.
@Override
public void closeInventory(GeyserSession session, LecternContainer container) {
public boolean requiresOpeningDelay(GeyserSession session, LecternContainer container) {
return false;
}
@Override
public void closeInventory(GeyserSession session, LecternContainer container, boolean force) {
// Of course, sending a simple ContainerClosePacket, or even breaking the block doesn't work to close a lectern.
// Heck, the latter crashes the client xd
// BDS just sends an empty base lectern tag... that kicks out the client. Fine. Let's do that!
@ -95,13 +103,21 @@ public class LecternInventoryTranslator extends AbstractBlockInventoryTranslator
// Closing lecterns isn't followed up by a ContainerClosePacket, so this wouldn't ever be reset.
session.setPendingOrCurrentBedrockInventoryId(-1);
super.closeInventory(session, container); // Removes the fake blocks if need be
super.closeInventory(session, container, force); // Removes the fake blocks if need be
// Now: Restore the lectern, if it actually exists
if (container.isUsingRealBlock()) {
boolean hasBook = session.getGeyser().getWorldManager().blockAt(session, position).getValue(Properties.HAS_BOOK, false);
NbtMap map = LecternBlock.getBaseLecternTag(position, hasBook);
BlockEntityUtils.updateBlockEntity(session, map, position);
Runnable closeLecternRunnable = () -> {
boolean hasBook = session.getGeyser().getWorldManager().blockAt(session, position).getValue(Properties.HAS_BOOK, false);
NbtMap map = LecternBlock.getBaseLecternTag(position, hasBook);
BlockEntityUtils.updateBlockEntity(session, map, position);
};
if (force) {
// Without a delay, an inventory close request can *occasionally* be ignored as we're restoring the book too quickly
session.scheduleInEventLoop(closeLecternRunnable, 100, TimeUnit.MILLISECONDS);
} else {
closeLecternRunnable.run();
}
}
}

View file

@ -127,7 +127,7 @@ public class MerchantInventoryTranslator extends BaseInventoryTranslator<Merchan
}
@Override
public void closeInventory(GeyserSession session, MerchantContainer container) {
public void closeInventory(GeyserSession session, MerchantContainer container, boolean force) {
if (container.getVillager() != null) {
container.getVillager().despawnEntity();
}

View file

@ -595,9 +595,8 @@ public class PlayerInventoryTranslator extends InventoryTranslator<PlayerInvento
}
@Override
public void closeInventory(GeyserSession session, PlayerInventory inventory) {
if (session.isServerRequestedClosePlayerInventory()) {
session.setServerRequestedClosePlayerInventory(false);
public void closeInventory(GeyserSession session, PlayerInventory inventory, boolean force) {
if (force) {
Vector3i pos = session.getPlayerEntity().getPosition().toInt();
UpdateBlockPacket packet = new UpdateBlockPacket();

View file

@ -164,7 +164,7 @@ public class DoubleChestInventoryTranslator extends ChestInventoryTranslator<Con
}
@Override
public void closeInventory(GeyserSession session, Container container) {
public void closeInventory(GeyserSession session, Container container, boolean force) {
// No need to reset a block since we didn't change any blocks
// But send a container close packet because we aren't destroying the original.
if (container.isDisplayed()) {

View file

@ -67,7 +67,7 @@ public class SingleChestInventoryTranslator extends ChestInventoryTranslator<Gen
}
@Override
public void closeInventory(GeyserSession session, Generic9X3Container container) {
public void closeInventory(GeyserSession session, Generic9X3Container container, boolean force) {
holder.closeInventory(session, container, ContainerType.CONTAINER);
}

View file

@ -49,7 +49,7 @@ public abstract class AbstractHorseInventoryTranslator extends BaseInventoryTran
}
@Override
public void closeInventory(GeyserSession session, Container container) {
public void closeInventory(GeyserSession session, Container container, boolean force) {
// TODO find a way to implement
// Can cause inventory de-sync if the Java server requests an inventory close
}

View file

@ -28,7 +28,6 @@ package org.geysermc.geyser.translator.protocol.java.inventory;
import org.geysermc.geyser.inventory.Inventory;
import org.geysermc.geyser.inventory.InventoryHolder;
import org.geysermc.geyser.inventory.LecternContainer;
import org.geysermc.geyser.inventory.PlayerInventory;
import org.geysermc.geyser.session.GeyserSession;
import org.geysermc.geyser.translator.protocol.PacketTranslator;
import org.geysermc.geyser.translator.protocol.Translator;
@ -46,13 +45,8 @@ public class JavaContainerCloseTranslator extends PacketTranslator<ClientboundCo
boolean confirm = false;
if (holder != null) {
if (packet.getContainerId() == 0) {
Inventory inventory = holder.inventory();
if (inventory instanceof PlayerInventory) {
session.setServerRequestedClosePlayerInventory(true);
}
if (inventory instanceof LecternContainer container && container.isBookInPlayerInventory()) {
InventoryUtils.closeInventory(session, holder, false);
if (holder.inventory() instanceof LecternContainer container && container.isBookInPlayerInventory()) {
InventoryUtils.closeInventory(session, holder, true);
return;
}
}

View file

@ -61,18 +61,14 @@ import java.util.concurrent.TimeUnit;
@Translator(packet = ClientboundContainerSetSlotPacket.class)
public class JavaContainerSetSlotTranslator extends PacketTranslator<ClientboundContainerSetSlotPacket> {
// TODO
@Override
public void translate(GeyserSession session, ClientboundContainerSetSlotPacket packet) {
//TODO: support window id -2, should update player inventory
//TODO: ^ I think this is outdated.
InventoryHolder<?> holder = InventoryUtils.getInventory(session, packet.getContainerId());
if (holder == null) {
return;
}
Inventory inventory = holder.inventory();
int slot = packet.getSlot();
if (slot >= inventory.getSize()) {
GeyserLogger logger = session.getGeyser().getLogger();

View file

@ -184,7 +184,7 @@ public class InventoryUtils {
updateCursor(session);
if (holder != null) {
holder.closeInventory();
holder.closeInventory(confirm);
if (holder.shouldConfirmClose(confirm)) {
session.setClosingInventory(true);
}