Skip to content

Commit

Permalink
Item frame optimization and block picking support (GeyserMC#2203)
Browse files Browse the repository at this point in the history 
Geyser now supports block picking for item frames. It checks to see if the item frame has an item in it - if so, it attempts the same block picking process with the item inside (NBT included). Otherwise, it attempts to pick for an item frame item.

This commit also improves item frames by having the internal map store the entity and not the ID - in many situations, this prevents two maps from having to be searched. Additionally, item frames are no longer despawned if an item is placed on them - rather, it waits until the server tells us to despawn the entity.
  • Loading branch information
@Camotoy
Camotoy authored May 9, 2021
1 parent 51aa96d commit 0691bb6
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 65 deletions.
31 changes: 16 additions & 15 deletions connector/src/main/java/org/geysermc/connector/entity/ItemFrameEntity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.nukkitx.protocol.bedrock.data.inventory.ItemData;
import com.nukkitx.protocol.bedrock.packet.BlockEntityDataPacket;
import com.nukkitx.protocol.bedrock.packet.UpdateBlockPacket;
import lombok.Getter;
import org.geysermc.connector.entity.type.EntityType;
import org.geysermc.connector.network.session.GeyserSession;
import org.geysermc.connector.network.translators.item.ItemEntry;
Expand Down Expand Up @@ -69,6 +70,11 @@ public class ItemFrameEntity extends Entity {
* Cached item frame's Bedrock compound tag.
*/
private NbtMap cachedTag;
/**
* The item currently in the item frame. Used for block picking.
*/
@Getter
private ItemStack heldItem = null;

public ItemFrameEntity(long entityId, long geyserId, EntityType entityType, Vector3f position, Vector3f motion, Vector3f rotation, HangingDirection direction) {
super(entityId, geyserId, entityType, position, motion, rotation);
Expand All @@ -87,7 +93,8 @@ public void spawnEntity(GeyserSession session) {
bedrockRuntimeId = session.getBlockTranslator().getItemFrame(blockBuilder.build());
bedrockPosition = Vector3i.from(position.getFloorX(), position.getFloorY(), position.getFloorZ());

session.getItemFrameCache().put(bedrockPosition, entityId);
session.getItemFrameCache().put(bedrockPosition, this);

// Delay is required, or else loading in frames on chunk load is sketchy at best
session.getConnector().getGeneralThreadPool().schedule(() -> {
updateBlock(session);
Expand All @@ -99,13 +106,14 @@ public void spawnEntity(GeyserSession session) {
@Override
public void updateBedrockMetadata(EntityMetadata entityMetadata, GeyserSession session) {
if (entityMetadata.getId() == 7 && entityMetadata.getValue() != null) {
ItemData itemData = ItemTranslator.translateToBedrock(session, (ItemStack) entityMetadata.getValue());
this.heldItem = (ItemStack) entityMetadata.getValue();
ItemData itemData = ItemTranslator.translateToBedrock(session, heldItem);
ItemEntry itemEntry = ItemRegistry.getItem((ItemStack) entityMetadata.getValue());
NbtMapBuilder builder = NbtMap.builder();

builder.putByte("Count", (byte) itemData.getCount());
if (itemData.getTag() != null) {
builder.put("tag", itemData.getTag().toBuilder().build());
builder.put("tag", itemData.getTag());
}
builder.putShort("Damage", (short) itemData.getDamage());
builder.putString("Name", itemEntry.getBedrockIdentifier());
Expand Down Expand Up @@ -146,7 +154,9 @@ public boolean despawnEntity(GeyserSession session) {
updateBlockPacket.getFlags().add(UpdateBlockPacket.Flag.NETWORK);
updateBlockPacket.getFlags().add(UpdateBlockPacket.Flag.NEIGHBORS);
session.sendUpstreamPacket(updateBlockPacket);
session.getItemFrameCache().remove(position, entityId);

session.getItemFrameCache().remove(bedrockPosition, this);

valid = false;
return true;
}
Expand Down Expand Up @@ -192,16 +202,7 @@ public void updateBlock(GeyserSession session) {
* @param session GeyserSession.
* @return Java entity ID or -1 if not found.
*/
public static long getItemFrameEntityId(GeyserSession session, Vector3i position) {
return session.getItemFrameCache().getOrDefault(position, -1);
}

/**
* Force-remove from the position-to-ID map so it doesn't cause conflicts.
* @param session GeyserSession.
* @param position position of the removed item frame.
*/
public static void removePosition(GeyserSession session, Vector3i position) {
session.getItemFrameCache().remove(position);
public static ItemFrameEntity getItemFrameEntity(GeyserSession session, Vector3i position) {
return session.getItemFrameCache().get(position);
}
}
10 changes: 4 additions & 6 deletions connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,7 @@
import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
import it.unimi.dsi.fastutil.longs.Long2ObjectMaps;
import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2LongMap;
import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap;
import it.unimi.dsi.fastutil.objects.ObjectIterator;
import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet;
import it.unimi.dsi.fastutil.objects.*;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NonNull;
Expand All @@ -81,6 +78,7 @@
import org.geysermc.connector.common.AuthType;
import org.geysermc.connector.configuration.EmoteOffhandWorkaroundOption;
import org.geysermc.connector.entity.Entity;
import org.geysermc.connector.entity.ItemFrameEntity;
import org.geysermc.connector.entity.Tickable;
import org.geysermc.connector.entity.attribute.Attribute;
import org.geysermc.connector.entity.attribute.AttributeType;
Expand Down Expand Up @@ -189,10 +187,10 @@ public class GeyserSession implements CommandSender {
private final Long2ObjectMap<ClientboundMapItemDataPacket> storedMaps = Long2ObjectMaps.synchronize(new Long2ObjectOpenHashMap<>());

/**
* A map of Vector3i positions to Java entity IDs.
* A map of Vector3i positions to Java entities.
* Used for translating Bedrock block actions to Java entity actions.
*/
private final Object2LongMap<Vector3i> itemFrameCache = new Object2LongOpenHashMap<>();
private final Map<Vector3i, ItemFrameEntity> itemFrameCache = new Object2ObjectOpenHashMap<>();

/**
* Stores a list of all lectern locations and their block entity tags.
Expand Down
13 changes: 13 additions & 0 deletions ...org/geysermc/connector/network/translators/bedrock/BedrockBlockPickRequestTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import com.nukkitx.math.vector.Vector3i;
import com.nukkitx.protocol.bedrock.packet.BlockPickRequestPacket;
import org.geysermc.connector.entity.ItemFrameEntity;
import org.geysermc.connector.network.session.GeyserSession;
import org.geysermc.connector.network.translators.PacketTranslator;
import org.geysermc.connector.network.translators.Translator;
Expand All @@ -43,6 +44,18 @@ public void translate(BlockPickRequestPacket packet, GeyserSession session) {

// Block is air - chunk caching is probably off
if (blockToPick == BlockTranslator.JAVA_AIR_ID) {
// Check for an item frame since the client thinks that's a block when it's an entity in Java
ItemFrameEntity entity = ItemFrameEntity.getItemFrameEntity(session, packet.getBlockPosition());
if (entity != null) {
// Check to see if the item frame has an item in it first
if (entity.getHeldItem() != null && entity.getHeldItem().getId() != 0) {
// Grab the item in the frame
InventoryUtils.findOrCreateItem(session, entity.getHeldItem());
} else {
// Grab the frame as the item
InventoryUtils.findOrCreateItem(session, "minecraft:item_frame");
}
}
return;
}

Expand Down
2 changes: 1 addition & 1 deletion ...rg/geysermc/connector/network/translators/bedrock/BedrockEntityPickRequestTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void translate(EntityPickRequestPacket packet, GeyserSession session) {
break;
case ARMOR_STAND:
case END_CRYSTAL:
case ITEM_FRAME:
//case ITEM_FRAME: Not an entity in Bedrock Edition
case MINECART:
case PAINTING:
// No spawn egg, just an item
Expand Down
30 changes: 17 additions & 13 deletions ...geysermc/connector/network/translators/bedrock/BedrockInventoryTransactionTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,19 @@ public void translate(InventoryTransactionPacket packet, GeyserSession session)
}

// Bedrock sends block interact code for a Java entity so we send entity code back to Java
if (session.getBlockTranslator().isItemFrame(packet.getBlockRuntimeId()) &&
session.getEntityCache().getEntityByJavaId(ItemFrameEntity.getItemFrameEntityId(session, packet.getBlockPosition())) != null) {
Vector3f vector = packet.getClickPosition();
ClientPlayerInteractEntityPacket interactPacket = new ClientPlayerInteractEntityPacket((int) ItemFrameEntity.getItemFrameEntityId(session, packet.getBlockPosition()),
InteractAction.INTERACT, Hand.MAIN_HAND, session.isSneaking());
ClientPlayerInteractEntityPacket interactAtPacket = new ClientPlayerInteractEntityPacket((int) ItemFrameEntity.getItemFrameEntityId(session, packet.getBlockPosition()),
InteractAction.INTERACT_AT, vector.getX(), vector.getY(), vector.getZ(), Hand.MAIN_HAND, session.isSneaking());
session.sendDownstreamPacket(interactPacket);
session.sendDownstreamPacket(interactAtPacket);
break;
if (session.getBlockTranslator().isItemFrame(packet.getBlockRuntimeId())) {
Entity itemFrameEntity = ItemFrameEntity.getItemFrameEntity(session, packet.getBlockPosition());
if (itemFrameEntity != null) {
int entityId = (int) itemFrameEntity.getEntityId();
Vector3f vector = packet.getClickPosition();
ClientPlayerInteractEntityPacket interactPacket = new ClientPlayerInteractEntityPacket(entityId,
InteractAction.INTERACT, Hand.MAIN_HAND, session.isSneaking());
ClientPlayerInteractEntityPacket interactAtPacket = new ClientPlayerInteractEntityPacket(entityId,
InteractAction.INTERACT_AT, vector.getX(), vector.getY(), vector.getZ(), Hand.MAIN_HAND, session.isSneaking());
session.sendDownstreamPacket(interactPacket);
session.sendDownstreamPacket(interactAtPacket);
break;
}
}

Vector3i blockPos = BlockUtils.getBlockPosition(packet.getBlockPosition(), packet.getBlockFace());
Expand Down Expand Up @@ -288,9 +291,10 @@ else if (packet.getItemInHand() != null && ItemRegistry.BUCKETS.contains(packet.
session.sendUpstreamPacket(blockBreakPacket);
session.setBreakingBlock(BlockTranslator.JAVA_AIR_ID);

long frameEntityId = ItemFrameEntity.getItemFrameEntityId(session, packet.getBlockPosition());
if (frameEntityId != -1 && session.getEntityCache().getEntityByJavaId(frameEntityId) != null) {
ClientPlayerInteractEntityPacket attackPacket = new ClientPlayerInteractEntityPacket((int) frameEntityId, InteractAction.ATTACK, session.isSneaking());
Entity itemFrameEntity = ItemFrameEntity.getItemFrameEntity(session, packet.getBlockPosition());
if (itemFrameEntity != null) {
ClientPlayerInteractEntityPacket attackPacket = new ClientPlayerInteractEntityPacket((int) itemFrameEntity.getEntityId(),
InteractAction.ATTACK, session.isSneaking());
session.sendDownstreamPacket(attackPacket);
break;
}
Expand Down
11 changes: 7 additions & 4 deletions ...rg/geysermc/connector/network/translators/bedrock/BedrockItemFrameDropItemTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.github.steveice10.mc.protocol.data.game.entity.player.InteractAction;
import com.github.steveice10.mc.protocol.packet.ingame.client.player.ClientPlayerInteractEntityPacket;
import com.nukkitx.protocol.bedrock.packet.ItemFrameDropItemPacket;
import org.geysermc.connector.entity.Entity;
import org.geysermc.connector.entity.ItemFrameEntity;
import org.geysermc.connector.network.session.GeyserSession;
import org.geysermc.connector.network.translators.PacketTranslator;
Expand All @@ -44,9 +45,11 @@ public class BedrockItemFrameDropItemTranslator extends PacketTranslator<ItemFra

@Override
public void translate(ItemFrameDropItemPacket packet, GeyserSession session) {
ClientPlayerInteractEntityPacket interactPacket = new ClientPlayerInteractEntityPacket((int) ItemFrameEntity.getItemFrameEntityId(session, packet.getBlockPosition()),
InteractAction.ATTACK, Hand.MAIN_HAND, session.isSneaking());
session.sendDownstreamPacket(interactPacket);
Entity entity = ItemFrameEntity.getItemFrameEntity(session, packet.getBlockPosition());
if (entity != null) {
ClientPlayerInteractEntityPacket interactPacket = new ClientPlayerInteractEntityPacket((int) entity.getEntityId(),
InteractAction.ATTACK, Hand.MAIN_HAND, session.isSneaking());
session.sendDownstreamPacket(interactPacket);
}
}

}
6 changes: 3 additions & 3 deletions ...geysermc/connector/network/translators/bedrock/entity/player/BedrockActionTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@ public void translate(PlayerActionPacket packet, GeyserSession session) {
if (session.getGameMode() != GameMode.CREATIVE) {
// As of 1.16.210: item frame items are taken out here.
// Survival also sends START_BREAK, but by attaching our process here adventure mode also works
long entityId = ItemFrameEntity.getItemFrameEntityId(session, packet.getBlockPosition());
if (entityId != -1) {
ClientPlayerInteractEntityPacket interactPacket = new ClientPlayerInteractEntityPacket((int) entityId,
Entity itemFrameEntity = ItemFrameEntity.getItemFrameEntity(session, packet.getBlockPosition());
if (itemFrameEntity != null) {
ClientPlayerInteractEntityPacket interactPacket = new ClientPlayerInteractEntityPacket((int) itemFrameEntity.getEntityId(),
InteractAction.ATTACK, Hand.MAIN_HAND, session.isSneaking());
session.sendDownstreamPacket(interactPacket);
break;
Expand Down
19 changes: 5 additions & 14 deletions connector/src/main/java/org/geysermc/connector/utils/ChunkUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import lombok.Data;
import lombok.experimental.UtilityClass;
import org.geysermc.connector.GeyserConnector;
import org.geysermc.connector.entity.Entity;
import org.geysermc.connector.entity.ItemFrameEntity;
import org.geysermc.connector.entity.player.SkullPlayerEntity;
import org.geysermc.connector.network.session.GeyserSession;
Expand Down Expand Up @@ -329,21 +328,13 @@ public static void updateBlock(GeyserSession session, int blockState, Position p
*/
public static void updateBlock(GeyserSession session, int blockState, Vector3i position) {
// Checks for item frames so they aren't tripped up and removed
long frameEntityId = ItemFrameEntity.getItemFrameEntityId(session, position);
if (frameEntityId != -1) {
// TODO: Very occasionally the item frame doesn't sync up when destroyed
Entity entity = session.getEntityCache().getEntityByJavaId(frameEntityId);
if (blockState == JAVA_AIR_ID && entity != null) { // Item frame is still present and no block overrides that; refresh it
((ItemFrameEntity) entity).updateBlock(session);
ItemFrameEntity itemFrameEntity = ItemFrameEntity.getItemFrameEntity(session, position);
if (itemFrameEntity != null) {
if (blockState == JAVA_AIR_ID) { // Item frame is still present and no block overrides that; refresh it
itemFrameEntity.updateBlock(session);
return;
}

// Otherwise the item frame is gone
if (entity != null) {
session.getEntityCache().removeEntity(entity, false);
} else {
ItemFrameEntity.removePosition(session, position);
}
// Otherwise, let's still store our reference to the item frame, but let the new block take precedence for now
}

SkullPlayerEntity skull = session.getSkullCache().get(position);
Expand Down
80 changes: 71 additions & 9 deletions connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,60 @@ public static ItemData createUnusableSpaceBlock(String description) {
.tag(root.build()).build();
}

/**
* See {@link #findOrCreateItem(GeyserSession, String)}. This is for finding a specified {@link ItemStack}.
*
* @param session the Bedrock client's session
* @param itemStack the item to try to find a match for. NBT will also be accounted for.
*/
public static void findOrCreateItem(GeyserSession session, ItemStack itemStack) {
PlayerInventory inventory = session.getPlayerInventory();

if (itemStack == null || itemStack.getId() == 0) {
return;
}

// Check hotbar for item
for (int i = 36; i < 45; i++) {
GeyserItemStack geyserItem = inventory.getItem(i);
if (geyserItem.isEmpty()) {
continue;
}
// If this is the item we're looking for
if (geyserItem.getJavaId() == itemStack.getId() && Objects.equals(geyserItem.getNbt(), itemStack.getNbt())) {
setHotbarItem(session, i);
// Don't check inventory if item was in hotbar
return;
}
}

// Check inventory for item
for (int i = 9; i < 36; i++) {
GeyserItemStack geyserItem = inventory.getItem(i);
if (geyserItem.isEmpty()) {
continue;
}
// If this is the item we're looking for
if (geyserItem.getJavaId() == itemStack.getId() && Objects.equals(geyserItem.getNbt(), itemStack.getNbt())) {
ClientMoveItemToHotbarPacket packetToSend = new ClientMoveItemToHotbarPacket(i); // https://wiki.vg/Protocol#Pick_Item
session.sendDownstreamPacket(packetToSend);
return;
}
}

// If we still have not found the item, and we're in creative, ask for the item from the server.
if (session.getGameMode() == GameMode.CREATIVE) {
int slot = findEmptyHotbarSlot(inventory);

ClientCreativeInventoryActionPacket actionPacket = new ClientCreativeInventoryActionPacket(slot,
itemStack);
if ((slot - 36) != inventory.getHeldItemSlot()) {
setHotbarItem(session, slot);
}
session.sendDownstreamPacket(actionPacket);
}
}

/**
* Attempt to find the specified item name in the session's inventory.
* If it is found and in the hotbar, set the user's held item to that slot.
Expand Down Expand Up @@ -223,15 +277,7 @@ public static void findOrCreateItem(GeyserSession session, String itemName) {

// If we still have not found the item, and we're in creative, ask for the item from the server.
if (session.getGameMode() == GameMode.CREATIVE) {
int slot = inventory.getHeldItemSlot() + 36;
if (!inventory.getItemInHand().isEmpty()) { // Otherwise we should just use the current slot
for (int i = 36; i < 45; i++) {
if (inventory.getItem(i).isEmpty()) {
slot = i;
break;
}
}
}
int slot = findEmptyHotbarSlot(inventory);

ItemEntry entry = ItemRegistry.getItemEntry(itemName);
if (entry != null) {
Expand All @@ -247,6 +293,22 @@ public static void findOrCreateItem(GeyserSession session, String itemName) {
}
}

/**
* @return the first empty slot found in this inventory, or else the player's currently held slot.
*/
private static int findEmptyHotbarSlot(PlayerInventory inventory) {
int slot = inventory.getHeldItemSlot() + 36;
if (!inventory.getItemInHand().isEmpty()) { // Otherwise we should just use the current slot
for (int i = 36; i < 45; i++) {
if (inventory.getItem(i).isEmpty()) {
slot = i;
break;
}
}
}
return slot;
}

/**
* Changes the held item slot to the specified slot
* @param session GeyserSession
Expand Down

0 comments on commit 0691bb6

Please sign in to comment.