Armor stand fixes (#3147)

Co-authored-by: David Choo <davchoo@users.noreply.github.com>
Co-authored-by: The Judge <53906078+thejudge156@users.noreply.github.com>

core/src/main/java/org/geysermc/geyser/entity/type/living/ArmorStandEntity.java

@@ -26,13 +26,13 @@
 package org.geysermc.geyser.entity.type.living;
 
 import com.github.steveice10.mc.protocol.data.game.entity.metadata.EntityMetadata;
+import com.github.steveice10.mc.protocol.data.game.entity.metadata.type.BooleanEntityMetadata;
 import com.github.steveice10.mc.protocol.data.game.entity.metadata.type.ByteEntityMetadata;
 import com.github.steveice10.mc.protocol.data.game.entity.player.Hand;
 import com.nukkitx.math.vector.Vector3f;
 import com.nukkitx.protocol.bedrock.data.entity.EntityData;
 import com.nukkitx.protocol.bedrock.data.entity.EntityFlag;
 import com.nukkitx.protocol.bedrock.data.inventory.ItemData;
-import com.nukkitx.protocol.bedrock.packet.MoveEntityAbsolutePacket;
 import lombok.Getter;
 import net.kyori.adventure.text.Component;
 import org.geysermc.geyser.entity.EntityDefinition;
@@ -54,6 +54,8 @@ public class ArmorStandEntity extends LivingEntity {
     @Getter
     private boolean isSmall = false;
 
+    private boolean isNameTagVisible = false;
+
     /**
      * On Java Edition, armor stands always show their name. Invisibility hides the name on Bedrock.
      * By having a second entity, we can allow an invisible entity with the name tag.
@@ -75,7 +77,6 @@ public class ArmorStandEntity extends LivingEntity {
      * - No armor, no name: false
      * - No armor, yes name: true
      */
-    @Getter
     private boolean positionRequiresOffset = false;
     /**
      * Whether we should update the position of this armor stand after metadata updates.
@@ -88,7 +89,11 @@ public class ArmorStandEntity extends LivingEntity {
 
     @Override
     public void spawnEntity() {
+        Vector3f javaPosition = position;
+        // Apply the offset if we're the second entity
+        position = position.up(getYOffset());
         super.spawnEntity();
+        position = javaPosition;
     }
 
     @Override
@@ -101,22 +106,18 @@ public class ArmorStandEntity extends LivingEntity {
 
     @Override
     public void moveRelative(double relX, double relY, double relZ, float yaw, float pitch, float headYaw, boolean isOnGround) {
-        if (secondEntity != null) {
+        moveAbsolute(position.add(relX, relY, relZ), yaw, pitch, headYaw, onGround, false);
-            secondEntity.moveRelative(relX, relY, relZ, yaw, pitch, headYaw, isOnGround);
-        }
-        super.moveRelative(relX, relY, relZ, yaw, pitch, headYaw, isOnGround);
     }
 
     @Override
     public void moveAbsolute(Vector3f position, float yaw, float pitch, float headYaw, boolean isOnGround, boolean teleported) {
         if (secondEntity != null) {
-            secondEntity.moveAbsolute(applyOffsetToPosition(position), yaw, pitch, headYaw, isOnGround, teleported);
+            secondEntity.moveAbsolute(position, yaw, pitch, headYaw, isOnGround, teleported);
-        } else if (positionRequiresOffset) {
-            // Fake the height to be above where it is so the nametag appears in the right location for invisible non-marker armour stands
-            position = applyOffsetToPosition(position);
         }
-
+        // Fake the height to be above where it is so the nametag appears in the right location
-        super.moveAbsolute(position, yaw, yaw, yaw, isOnGround, teleported);
+        float yOffset = getYOffset();
+        super.moveAbsolute(yOffset != 0 ? position.up(yOffset) : position , yaw, yaw, yaw, isOnGround, teleported);
+        this.position = position;
     }
 
     @Override
@@ -127,20 +128,14 @@ public class ArmorStandEntity extends LivingEntity {
 
     public void setArmorStandFlags(ByteEntityMetadata entityMetadata) {
         byte xd = entityMetadata.getPrimitiveValue();
-
+        boolean offsetChanged = false;
         // isSmall
         boolean newIsSmall = (xd & 0x01) == 0x01;
         if (newIsSmall != isSmall) {
-            if (positionRequiresOffset) {
-                // Fix new inconsistency with offset
-                this.position = fixOffsetForSize(position, newIsSmall);
-                positionUpdateRequired = true;
-            }
-
             isSmall = newIsSmall;
-            if (!isMarker && !isInvisible) { // Addition for isInvisible check caused by https://github.com/GeyserMC/Geyser/issues/2780
+            offsetChanged = true;
-                toggleSmallStatus();
+            // Update the passenger offset as the armor stand's height has changed
-            }
+            updatePassengerOffsets();
         }
 
         // setMarker
@@ -150,12 +145,21 @@ public class ArmorStandEntity extends LivingEntity {
             if (isMarker) {
                 setBoundingBoxWidth(0.0f);
                 setBoundingBoxHeight(0.0f);
-                dirtyMetadata.put(EntityData.SCALE, 0f);
             } else {
-                toggleSmallStatus();
+                setBoundingBoxWidth(definition.width());
+                setBoundingBoxHeight(definition.height());
             }
 
             updateMountOffset();
+            offsetChanged = true;
+        }
+
+        if (offsetChanged) {
+            if (positionRequiresOffset) {
+                positionUpdateRequired = true;
+            } else if (secondEntity != null) {
+                secondEntity.positionUpdateRequired = true;
+            }
             updateSecondEntityStatus(false);
         }
 
@@ -226,7 +230,7 @@ public class ArmorStandEntity extends LivingEntity {
         super.updateBedrockMetadata();
         if (positionUpdateRequired) {
             positionUpdateRequired = false;
-            updatePosition();
+            moveAbsolute(position, yaw, pitch, headYaw, onGround, true);
         }
     }
 
@@ -285,6 +289,13 @@ public class ArmorStandEntity extends LivingEntity {
         updateSecondEntityStatus(true);
     }
 
+    @Override
+    public void setDisplayNameVisible(BooleanEntityMetadata entityMetadata) {
+        super.setDisplayNameVisible(entityMetadata);
+        isNameTagVisible = entityMetadata.getPrimitiveValue();
+        updateSecondEntityStatus(false);
+    }
+
     /**
      * Determine if we need to load or unload the second entity.
      *
@@ -293,40 +304,44 @@ public class ArmorStandEntity extends LivingEntity {
     private void updateSecondEntityStatus(boolean sendMetadata) {
         // A secondary entity always has to have the offset applied, so it remains invisible and the nametag shows.
         if (!primaryEntity) return;
-        if (!isInvisible || isMarker) {
+        if (!isInvisible) {
-            // It is either impossible to show armor, or the armor stand isn't invisible. We good.
+            // The armor stand isn't invisible. We good.
             setFlag(EntityFlag.INVISIBLE, false);
+            dirtyMetadata.put(EntityData.SCALE, getScale());
             updateOffsetRequirement(false);
-            if (positionUpdateRequired) {
-                positionUpdateRequired = false;
-                updatePosition();
-            }
 
             if (secondEntity != null) {
                 secondEntity.despawnEntity();
                 secondEntity = null;
             }
+            if (sendMetadata) {
+                this.updateBedrockMetadata();
+            }
             return;
         }
         boolean isNametagEmpty = nametag.isEmpty();
         if (!isNametagEmpty && (!helmet.equals(ItemData.AIR) || !chestplate.equals(ItemData.AIR) || !leggings.equals(ItemData.AIR)
                 || !boots.equals(ItemData.AIR) || !hand.equals(ItemData.AIR) || !offHand.equals(ItemData.AIR))) {
-            // If the second entity exists, no need to recreate it.
+            // Reset scale of the proper armor stand
-            // We can't stuff this check above or else it'll fall into another else case and delete the second entity
+            this.dirtyMetadata.put(EntityData.SCALE, getScale());
-            if (secondEntity != null) return;
+            // Set the proper armor stand to invisible to show armor
+            setFlag(EntityFlag.INVISIBLE, true);
+            // Update the position of the armor stand
+            updateOffsetRequirement(false);
 
-            // Create the second entity. It doesn't need to worry about the items, but it does need to worry about
+            if (secondEntity == null) {
-            // the metadata as it will hold the name tag.
+                // Create the second entity. It doesn't need to worry about the items, but it does need to worry about
-            secondEntity = new ArmorStandEntity(session, 0, session.getEntityCache().getNextEntityId().incrementAndGet(), null,
+                // the metadata as it will hold the name tag.
-                    EntityDefinitions.ARMOR_STAND, position, motion, getYaw(), getPitch(), getHeadYaw());
+                secondEntity = new ArmorStandEntity(session, 0, session.getEntityCache().getNextEntityId().incrementAndGet(), null,
-            secondEntity.primaryEntity = false;
+                        EntityDefinitions.ARMOR_STAND, position, motion, getYaw(), getPitch(), getHeadYaw());
-            if (!this.positionRequiresOffset) {
+                secondEntity.primaryEntity = false;
-                // Ensure the offset is applied for the 0 scale
-                secondEntity.position = secondEntity.applyOffsetToPosition(secondEntity.position);
             }
             // Copy metadata
             secondEntity.isSmall = isSmall;
-            //secondEntity.getDirtyMetadata().putAll(dirtyMetadata); //TODO check
+            secondEntity.isMarker = isMarker;
+            secondEntity.positionRequiresOffset = true; // Offset should always be applied
+            secondEntity.getDirtyMetadata().put(EntityData.NAMETAG, nametag);
+            secondEntity.getDirtyMetadata().put(EntityData.NAMETAG_ALWAYS_SHOW, isNameTagVisible ? (byte) 1 : (byte) 0);
             secondEntity.flags.merge(this.flags);
             // Guarantee this copy is NOT invisible
             secondEntity.setFlag(EntityFlag.INVISIBLE, false);
@@ -335,18 +350,13 @@ public class ArmorStandEntity extends LivingEntity {
             // No bounding box as we don't want to interact with this entity
             secondEntity.getDirtyMetadata().put(EntityData.BOUNDING_BOX_WIDTH, 0.0f);
             secondEntity.getDirtyMetadata().put(EntityData.BOUNDING_BOX_HEIGHT, 0.0f);
-            secondEntity.spawnEntity();
+            if (!secondEntity.valid) { // Spawn the entity once
-
+                secondEntity.spawnEntity();
-            // Reset scale of the proper armor stand
+            }
-            this.dirtyMetadata.put(EntityData.SCALE, isSmall ? 0.55f : 1f);
-            // Set the proper armor stand to invisible to show armor
-            setFlag(EntityFlag.INVISIBLE, true);
-            // Update the position of the armor stand
-            updateOffsetRequirement(false);
         } else if (isNametagEmpty) {
             // We can just make an invisible entity
             // Reset scale of the proper armor stand
-            dirtyMetadata.put(EntityData.SCALE, isSmall ? 0.55f : 1f);
+            dirtyMetadata.put(EntityData.SCALE, getScale());
             // Set the proper armor stand to invisible to show armor
             setFlag(EntityFlag.INVISIBLE, true);
             // Update offset
@@ -362,7 +372,7 @@ public class ArmorStandEntity extends LivingEntity {
             setFlag(EntityFlag.INVISIBLE, false);
             dirtyMetadata.put(EntityData.SCALE, 0.0f);
             // As the above is applied, we need an offset
-            updateOffsetRequirement(true);
+            updateOffsetRequirement(!isMarker);
 
             if (secondEntity != null) {
                 secondEntity.despawnEntity();
@@ -374,35 +384,34 @@ public class ArmorStandEntity extends LivingEntity {
         }
     }
 
-    /**
+    @Override
-     * If this armor stand is not a marker, set its bounding box size and scale.
+    public float getBoundingBoxWidth() {
-     */
+        // For consistency with getBoundingBoxHeight()
-    private void toggleSmallStatus() {
+        return super.getBoundingBoxWidth() * getScale();
-        setBoundingBoxWidth(isSmall ? 0.25f : definition.width());
+    }
-        setBoundingBoxHeight(isSmall ? 0.9875f : definition.height());
+
-        dirtyMetadata.put(EntityData.SCALE, isSmall ? 0.55f : 1f);
+    @Override
+    public float getBoundingBoxHeight() {
+        // This is required so that EntityUtils#updateMountOffset() calculates the correct offset for small
+        // armor stands. The bounding box height is not changed as the SCALE entity data handles that for us.
+        return super.getBoundingBoxHeight() * getScale();
     }
 
     /**
-     * @return the selected position with the position offset applied.
+     * @return the y offset required to position the name tag correctly
      */
-    private Vector3f applyOffsetToPosition(Vector3f position) {
+    public float getYOffset() {
-        return position.add(0d, definition.height() * (isSmall ? 0.55d : 1d), 0d);
+        if (!positionRequiresOffset || isMarker || secondEntity != null) {
+            return 0;
+        }
+        return definition.height() * getScale();
     }
 
     /**
-     * @return an adjusted offset for the new small status.
+     * @return the scale according to Java
      */
-    private Vector3f fixOffsetForSize(Vector3f position, boolean isNowSmall) {
+    private float getScale() {
-        position = removeOffsetFromPosition(position);
+        return isSmall ? 0.5f : 1f;
-        return position.add(0d, definition.height() * (isNowSmall ? 0.55d : 1d), 0d);
-    }
-
-    /**
-     * @return the selected position with the position offset removed.
-     */
-    private Vector3f removeOffsetFromPosition(Vector3f position) {
-        return position.sub(0d, definition.height() * (isSmall ? 0.55d : 1d), 0d);
     }
 
     /**
@@ -411,30 +420,12 @@ public class ArmorStandEntity extends LivingEntity {
     private void updateOffsetRequirement(boolean newValue) {
         if (newValue != positionRequiresOffset) {
             this.positionRequiresOffset = newValue;
-            if (positionRequiresOffset) {
+            this.positionUpdateRequired = true;
-                this.position = applyOffsetToPosition(position);
+            // Update the passenger offset as the armor stand's y offset has changed
-                // Update the passenger offset as armorstand is moving up by roughly 2 blocks
+            updatePassengerOffsets();
-                updatePassengerOffsets();
-            } else {
-                this.position = removeOffsetFromPosition(position);
-            }
-            positionUpdateRequired = true;
         }
     }
 
-    /**
-     * Updates position without calling movement code.
-     */
-    private void updatePosition() {
-        MoveEntityAbsolutePacket moveEntityPacket = new MoveEntityAbsolutePacket();
-        moveEntityPacket.setRuntimeEntityId(geyserId);
-        moveEntityPacket.setPosition(position);
-        moveEntityPacket.setRotation(getBedrockRotation());
-        moveEntityPacket.setOnGround(isOnGround());
-        moveEntityPacket.setTeleported(false);
-        session.sendUpstreamPacket(moveEntityPacket);
-    }
-
     @Override
     public Vector3f getBedrockRotation() {
         return Vector3f.from(getYaw(), getYaw(), getYaw());

core/src/main/java/org/geysermc/geyser/util/EntityUtils.java

@@ -190,11 +190,8 @@ public final class EntityUtils {
             if (passenger.getDefinition().entityType() == EntityType.FALLING_BLOCK) {
                 yOffset += 0.5f;
             }
-            if (mount.getDefinition().entityType() == EntityType.ARMOR_STAND) {
+            if (mount instanceof ArmorStandEntity armorStand) {
-                ArmorStandEntity armorStand = (ArmorStandEntity) mount;
+                yOffset -= armorStand.getYOffset();
-                if (armorStand.isPositionRequiresOffset()) {
-                    yOffset -= EntityDefinitions.ARMOR_STAND.height() * (armorStand.isSmall() ? 0.55d : 1d);
-                }
             }
             Vector3f offset = Vector3f.from(xOffset, yOffset, zOffset);
             passenger.setRiderSeatPosition(offset);