Fix #5089 and don't auto-load Registries

core/src/main/java/org/geysermc/geyser/entity/EntityDefinition.java

@@ -37,7 +37,6 @@
 import org.geysermc.geyser.entity.type.Entity;
 import org.geysermc.geyser.registry.Registries;
 import org.geysermc.geyser.translator.entity.EntityMetadataTranslator;
-import org.geysermc.geyser.util.EnvironmentUtils;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.metadata.EntityMetadata;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.metadata.MetadataType;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.type.EntityType;
@@ -146,13 +145,8 @@ public Builder<T> addTranslator(EntityMetadataTranslator<T, ?, ?> translator) {
             return this;
         }
 
-        /**
-         * Build the given entity. If a testing environment has been discovered the entity is not registered,
-         * otherwise it is. This is to prevent all the registries from loading, which will fail (and should
-         * not be loaded) while testing
-         */
         public EntityDefinition<T> build() {
-            return build(!EnvironmentUtils.isUnitTesting);
+            return build(true);
         }
 
         /**

core/src/main/java/org/geysermc/geyser/entity/EntityDefinitions.java

@@ -145,7 +145,6 @@
 import org.geysermc.geyser.entity.type.player.PlayerEntity;
 import org.geysermc.geyser.registry.Registries;
 import org.geysermc.geyser.translator.text.MessageTranslator;
-import org.geysermc.geyser.util.EnvironmentUtils;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.metadata.MetadataType;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.metadata.type.BooleanEntityMetadata;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.metadata.type.FloatEntityMetadata;
@@ -1122,10 +1121,7 @@ public final class EntityDefinitions {
                 .identifier("minecraft:armor_stand") // Emulated
                 .build(false); // Never sent over the network
 
-        // causes the registries to load
-        if (!EnvironmentUtils.isUnitTesting) {
-            Registries.JAVA_ENTITY_IDENTIFIERS.get().put("minecraft:marker", null); // We don't need an entity definition for this as it is never sent over the network
-        }
+        Registries.JAVA_ENTITY_IDENTIFIERS.get().put("minecraft:marker", null); // We don't need an entity definition for this as it is never sent over the network
     }
 
     public static void init() {

core/src/main/java/org/geysermc/geyser/entity/GeyserDirtyMetadata.java

@@ -32,7 +32,7 @@
 import java.util.Map;
 
 /**
- * A write-only wrapper for temporarily storing entity metadata that will be sent to Bedrock.
+ * A wrapper for temporarily storing entity metadata that will be sent to Bedrock.
  */
 public final class GeyserDirtyMetadata {
     private final Map<EntityDataType<?>, Object> metadata = new Object2ObjectLinkedOpenHashMap<>();
@@ -53,6 +53,14 @@ public boolean hasEntries() {
         return !metadata.isEmpty();
     }
 
+    /**
+     * Intended for testing purposes only
+     */
+    public <T> T get(EntityDataType<T> entityData) {
+        //noinspection unchecked
+        return (T) metadata.get(entityData);
+    }
+
     @Override
     public String toString() {
         return metadata.toString();

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

@@ -27,6 +27,7 @@
 
 import lombok.Getter;
 import net.kyori.adventure.text.Component;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.cloudburstmc.math.vector.Vector3f;
 import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataType;
 import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataTypes;
@@ -36,6 +37,7 @@
 import org.geysermc.geyser.entity.EntityDefinitions;
 import org.geysermc.geyser.entity.type.LivingEntity;
 import org.geysermc.geyser.item.Items;
+import org.geysermc.geyser.scoreboard.Team;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.util.InteractionResult;
 import org.geysermc.geyser.util.MathUtils;
@@ -123,6 +125,12 @@ public void moveAbsolute(Vector3f position, float yaw, float pitch, float headYa
         this.position = position;
     }
 
+    @Override
+    public void updateNametag(@Nullable Team team) {
+        // unlike all other LivingEntities, armor stands are not affected by team nametag visibility
+        super.updateNametag(team, true);
+    }
+
     @Override
     public void setDisplayName(EntityMetadata<Optional<Component>, ?> entityMetadata) {
         super.setDisplayName(entityMetadata);

core/src/main/java/org/geysermc/geyser/entity/type/player/PlayerEntity.java

@@ -43,7 +43,6 @@
 import org.cloudburstmc.protocol.bedrock.data.GameType;
 import org.cloudburstmc.protocol.bedrock.data.PlayerPermission;
 import org.cloudburstmc.protocol.bedrock.data.command.CommandPermission;
-import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataMap;
 import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataTypes;
 import org.cloudburstmc.protocol.bedrock.data.entity.EntityFlag;
 import org.cloudburstmc.protocol.bedrock.data.entity.EntityLinkData;
@@ -112,20 +111,6 @@ public PlayerEntity(GeyserSession session, int entityId, long geyserId, UUID uui
         this.texturesProperty = texturesProperty;
     }
 
-    /**
-     * Do not use! For testing purposes only
-     */
-    public PlayerEntity(GeyserSession session, long geyserId, UUID uuid, String username) {
-        super(session, -1, geyserId, uuid, EntityDefinitions.PLAYER, Vector3f.ZERO, Vector3f.ZERO, 0, 0, 0);
-        this.username = username;
-        this.nametag = username;
-        this.texturesProperty = null;
-
-        // clear initial metadata
-        dirtyMetadata.apply(new EntityDataMap());
-        setFlagsDirty(false);
-    }
-
     @Override
     protected void initializeMetadata() {
         super.initializeMetadata();
@@ -193,11 +178,7 @@ public void sendPlayer() {
         if (session.getEntityCache().getPlayerEntity(uuid) == null)
             return;
 
-        if (session.getEntityCache().getEntityByGeyserId(geyserId) == null) {
-            session.getEntityCache().spawnEntity(this);
-        } else {
-            spawnEntity();
-        }
+        session.getEntityCache().spawnEntity(this);
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/registry/Registries.java

@@ -62,14 +62,16 @@
  * Holds all the common registries in Geyser.
  */
 public final class Registries {
+    private static boolean initialized = false;
+
     /**
      * A registry holding all the providers.
      * This has to be initialized first to allow extensions to access providers during other registry events.
      */
     public static final SimpleMappedRegistry<Class<?>, ProviderSupplier> PROVIDERS = SimpleMappedRegistry.create(new IdentityHashMap<>(), ProviderRegistryLoader::new);
 
     /**
-     * A registry holding a CompoundTag of the known entity identifiers.
+     * A registry holding a NbtMap of the known entity identifiers.
      */
     public static final SimpleRegistry<NbtMap> BEDROCK_ENTITY_IDENTIFIERS = SimpleRegistry.create("bedrock/entity_identifiers.dat", RegistryLoaders.NBT);
 
@@ -79,7 +81,7 @@ public final class Registries {
     public static final PacketTranslatorRegistry<BedrockPacket> BEDROCK_PACKET_TRANSLATORS = PacketTranslatorRegistry.create();
 
     /**
-     * A registry holding a CompoundTag of all the known biomes.
+     * A registry holding a NbtMap of all the known biomes.
      */
     public static final SimpleRegistry<NbtMap> BIOMES_NBT = SimpleRegistry.create("bedrock/biome_definitions.dat", RegistryLoaders.NBT);
 
@@ -118,6 +120,9 @@ public final class Registries {
      */
     public static final ListRegistry<Item> JAVA_ITEMS = ListRegistry.create(RegistryLoaders.empty(ArrayList::new));
 
+    /**
+     * A registry containing item identifiers.
+     */
     public static final SimpleMappedRegistry<String, Item> JAVA_ITEM_IDENTIFIERS = SimpleMappedRegistry.create(RegistryLoaders.empty(Object2ObjectOpenHashMap::new));
 
     /**
@@ -135,7 +140,7 @@ public final class Registries {
     /**
      * A registry holding all the potion mixes.
      */
-    public static final VersionedRegistry<Set<PotionMixData>> POTION_MIXES;
+    public static final VersionedRegistry<Set<PotionMixData>> POTION_MIXES = VersionedRegistry.create(PotionMixRegistryLoader::new);
 
     /**
      * A versioned registry holding all the recipes, with the net ID being the key, and {@link GeyserRecipe} as the value.
@@ -163,15 +168,35 @@ public final class Registries {
     public static final SimpleMappedRegistry<SoundTranslator, SoundInteractionTranslator<?>> SOUND_TRANSLATORS = SimpleMappedRegistry.create("org.geysermc.geyser.translator.sound.SoundTranslator", SoundTranslatorRegistryLoader::new);
 
     public static void init() {
-        // no-op
-    }
+        if (initialized) return;
+        initialized = true;
+
+        PROVIDERS.load();
+        BEDROCK_ENTITY_IDENTIFIERS.load();
+        BEDROCK_PACKET_TRANSLATORS.load();
+        BIOMES_NBT.load();
+        BIOME_IDENTIFIERS.load();
+        BLOCK_ENTITIES.load();
+        ENTITY_DEFINITIONS.load();
+        BEDROCK_ENTITY_PROPERTIES.load();
+        JAVA_ENTITY_IDENTIFIERS.load();
+        JAVA_PACKET_TRANSLATORS.load();
+        JAVA_ITEMS.load();
+        JAVA_ITEM_IDENTIFIERS.load();
+        ITEMS.load();
+        PARTICLES.load();
+        // load potion mixes later
+        RECIPES.load();
+        RESOURCE_PACKS.load();
+        SOUNDS.load();
+        SOUND_LEVEL_EVENTS.load();
+        SOUND_TRANSLATORS.load();
 
-    static {
         PacketRegistryPopulator.populate();
         ItemRegistryPopulator.populate();
 
-        // Create registries that require other registries to load first
-        POTION_MIXES = VersionedRegistry.create(PotionMixRegistryLoader::new);
+        // potion mixes depend on other registries
+        POTION_MIXES.load();
 
         // Remove unneeded client generation data from NbtMapBuilder
         NbtMapBuilder biomesNbt = NbtMap.builder();

core/src/main/java/org/geysermc/geyser/registry/Registry.java

@@ -25,9 +25,9 @@
 
 package org.geysermc.geyser.registry;
 
-import org.geysermc.geyser.registry.loader.RegistryLoader;
-
 import java.util.function.Consumer;
+import org.geysermc.geyser.registry.loader.RegistryLoader;
+import org.geysermc.geyser.registry.loader.RegistryLoaderHolder;
 
 /**
  * A wrapper around a value which is loaded based on the output from the provided
@@ -63,7 +63,9 @@
  *
  * @param <M> the value being held by the registry
  */
+@SuppressWarnings("rawtypes")
 public abstract class Registry<M> implements IRegistry<M> {
+    protected RegistryLoaderHolder loaderHolder;
     protected M mappings;
 
     /**
@@ -76,7 +78,17 @@ public abstract class Registry<M> implements IRegistry<M> {
      * @param <I> the input type
      */
     protected <I> Registry(I input, RegistryLoader<I, M> registryLoader) {
-        this.mappings = registryLoader.load(input);
+        this.loaderHolder = new RegistryLoaderHolder<>(input, registryLoader);
+    }
+
+    public void load() {
+        // don't load twice
+        if (this.mappings != null) return;
+
+        var holder = this.loaderHolder;
+        this.loaderHolder = null;
+        //noinspection unchecked
+        this.mappings = (M) holder.registryLoader().load(holder.input());
     }
 
     /**
@@ -111,4 +123,4 @@ public void set(M mappings) {
     public void register(Consumer<M> consumer) {
         consumer.accept(this.mappings);
     }
-}
+}

core/src/main/java/org/geysermc/geyser/registry/loader/PotionMixRegistryLoader.java

@@ -125,4 +125,4 @@ private static ItemMapping getNonNull(ItemMappings mappings, Item javaItem) {
 
         return itemMapping;
     }
-}
+}

core/src/main/java/org/geysermc/geyser/registry/loader/RegistryLoaderHolder.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2024 GeyserMC. http://geysermc.org
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @author GeyserMC
+ * @link https://github.com/GeyserMC/Geyser
+ */
+
+package org.geysermc.geyser.registry.loader;
+
+/**
+ * A holder of the constructor parameters to prevent them from automatically loading,
+ * and instead load them when the load method is called.
+ */
+public record RegistryLoaderHolder<I, M>(I input, RegistryLoader<I, M> registryLoader) {
+}

core/src/main/java/org/geysermc/geyser/scoreboard/Team.java

@@ -90,6 +90,8 @@ public void addEntities(String... names) {
                     // Remove old team from this map, and from the set of players of the old team.
                     // Java 1.19.3 Mojmap: Scoreboard#addPlayerToTeam calls #removePlayerFromTeam
                     oldTeam.entities.remove(player);
+                    // also remove the managed entity if there is one
+                    removeManagedEntity(player);
                 }
                 return this;
             });
@@ -282,6 +284,15 @@ private void removeRemovedEntities(Set<String> names) {
         }
     }
 
+    /**
+     * Used internally to remove a managed entity without causing an update.
+     * This is fine because its only used when the entity is added to another team,
+     * which will fire the correct nametag updates etc.
+     */
+    private void removeManagedEntity(String name) {
+        managedEntities.removeIf(entity -> name.equals(entity.teamIdentifier()));
+    }
+
     private void refreshAllEntities() {
         for (Entity entity : session().getEntityCache().getEntities().values()) {
             entity.updateNametag(scoreboard.getTeamFor(entity.teamIdentifier()));

core/src/main/java/org/geysermc/geyser/session/cache/EntityCache.java

@@ -69,12 +69,13 @@ public EntityCache(GeyserSession session) {
 
     public void spawnEntity(Entity entity) {
         if (cacheEntity(entity)) {
-            entity.spawnEntity();
-
-            // start tracking newly spawned entities.
-            // This is however not called for players, that's done in addPlayerEntity
+            // start tracking newly spawned entities. Doing this before the actual entity spawn can result in combining
+            // the otherwise sent metadata packet (in the case of team visibility, which sets the NAME metadata to
+            // empty) with the entity spawn packet (which also includes metadata). Resulting in 1 less packet sent.
             session.getWorldCache().getScoreboard().entityRegistered(entity);
 
+            entity.spawnEntity();
+
             if (entity instanceof Tickable) {
                 // Start ticking it
                 tickableEntities.add((Tickable) entity);
@@ -144,8 +145,6 @@ public void addPlayerEntity(PlayerEntity entity) {
         // notify scoreboard for new entity
         var scoreboard = session.getWorldCache().getScoreboard();
         scoreboard.playerRegistered(entity);
-        // spawnPlayer's entityRegistered is not called for players
-        scoreboard.entityRegistered(entity);
     }
 
     public PlayerEntity getPlayerEntity(UUID uuid) {

core/src/main/java/org/geysermc/geyser/translator/protocol/java/entity/player/JavaPlayerInfoUpdateTranslator.java

@@ -25,7 +25,6 @@
 
 package org.geysermc.geyser.translator.protocol.java.entity.player;
 
-import org.geysermc.mcprotocollib.auth.GameProfile;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.cloudburstmc.math.vector.Vector3f;
 import org.cloudburstmc.protocol.bedrock.packet.PlayerListPacket;
@@ -35,6 +34,7 @@
 import org.geysermc.geyser.skin.SkinManager;
 import org.geysermc.geyser.translator.protocol.PacketTranslator;
 import org.geysermc.geyser.translator.protocol.Translator;
+import org.geysermc.mcprotocollib.auth.GameProfile;
 import org.geysermc.mcprotocollib.protocol.data.game.PlayerListEntry;
 import org.geysermc.mcprotocollib.protocol.data.game.PlayerListEntryAction;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.clientbound.ClientboundPlayerInfoUpdatePacket;
@@ -95,8 +95,6 @@ public void translate(GeyserSession session, ClientboundPlayerInfoUpdatePacket p
                 if (self) {
                     SkinManager.requestAndHandleSkinAndCape(playerEntity, session, skinAndCape ->
                             GeyserImpl.getInstance().getLogger().debug("Loaded Local Bedrock Java Skin Data for " + session.getClientData().getUsername()));
-                } else {
-                    playerEntity.setValid(true);
                 }
             }
         }