Merge pull request #3489 from Multiverse/fix/entity-warning

Improve spawn category map logging

.github/workflows/generic.platform_uploads.yml

@@ -107,7 +107,7 @@ jobs:
         files: '["${{ github.workspace }}/${{ inputs.plugin_name }}-${{ steps.release-info.outputs.tag_name }}.jar"]'
         name: ${{ steps.release-info.outputs.tag_name }}
         changelog: ${{ steps.release-artifact.outputs.body }}
-        game_versions: 26.1.2, 26.1.1, 26.1, 1.21.11, 1.21.10, 1.21.9, 1.21.8, 1.21.7, 1.21.6, 1.21.5, 1.21.4, 1.21.3, 1.21.2, 1.21.1, 1.21, 1.20.6, 1.20.5, 1.20.4, 1.20.3, 1.20.2, 1.20.1, 1.20, 1.19.4, 1.19.3, 1.19.2, 1.19.1, 1.19, 1.18.2
+        game_versions: 26.2, 26.1.2, 26.1.1, 26.1, 1.21.11, 1.21.10, 1.21.9, 1.21.8, 1.21.7, 1.21.6, 1.21.5, 1.21.4, 1.21.3, 1.21.2, 1.21.1, 1.21, 1.20.6, 1.20.5, 1.20.4, 1.20.3, 1.20.2, 1.20.1, 1.20, 1.19.4, 1.19.3, 1.19.2, 1.19.1, 1.19, 1.18.2
         version_type: ${{ steps.parse-release-type.outputs.release_type }}
         loaders: bukkit, spigot, paper, purpur
         dependencies: ${{ inputs.modrinth_dependencies }}
@@ -121,7 +121,7 @@ jobs:
         changelog: ${{ steps.release-artifact.outputs.body }}
         changelog_type: markdown
         display_name: ${{ steps.release-info.outputs.tag_name }}
-        game_versions: 26.1.2, 26.1.1, 26.1, 1.21.11, 1.21.10, 1.21.9, 1.21.8, 1.21.7, 1.21.6, 1.21.5, 1.21.4, 1.21.3, 1.21.2, 1.21.1, 1.21, 1.20.6, 1.20.5, 1.20.4, 1.20.3, 1.20.2, 1.20.1, 1.20, 1.19.4, 1.19.3, 1.19.2, 1.19.1, 1.19, 1.18.2
+        game_versions: 26.2, 26.1.2, 26.1.1, 26.1, 1.21.11, 1.21.10, 1.21.9, 1.21.8, 1.21.7, 1.21.6, 1.21.5, 1.21.4, 1.21.3, 1.21.2, 1.21.1, 1.21, 1.20.6, 1.20.5, 1.20.4, 1.20.3, 1.20.2, 1.20.1, 1.20, 1.19.4, 1.19.3, 1.19.2, 1.19.1, 1.19, 1.18.2
         release_type: ${{ steps.parse-release-type.outputs.release_type }}
         project_relations: ${{ inputs.dbo_project_relations }}
         file_path: ${{ github.workspace }}/${{ inputs.plugin_name }}-${{ steps.release-info.outputs.tag_name }}.jar
@@ -136,5 +136,5 @@ jobs:
         channel: ${{ steps.parse-release-type.outputs.release_type }}
         files: '[{"path": "${{ github.workspace }}/${{ inputs.plugin_name }}-${{ steps.release-info.outputs.tag_name }}.jar", "platforms": ["PAPER"]}]'
         description: ${{ steps.release-artifact.outputs.body }}
-        platform_dependencies: '{"PAPER": ["1.18.2-26.1.2"]}'
+        platform_dependencies: '{"PAPER": ["1.18.2-26.2"]}'
         plugin_dependencies: ${{ inputs.hangar_plugin_dependencies }}

src/main/java/org/mvplugins/multiverse/core/MultiverseCore.java

@@ -30,6 +30,7 @@ import org.mvplugins.multiverse.core.inject.PluginServiceLocatorFactory;
 import org.mvplugins.multiverse.core.module.MultiverseModule;
 import org.mvplugins.multiverse.core.utils.StringFormatter;
 import org.mvplugins.multiverse.core.world.WorldManager;
+import org.mvplugins.multiverse.core.world.entity.SpawnCategoryMapper;
 import org.mvplugins.multiverse.core.world.location.NullSpawnLocation;
 import org.mvplugins.multiverse.core.world.location.SpawnLocation;
 import org.mvplugins.multiverse.core.world.location.UnloadedWorldLocation;
@@ -97,6 +98,9 @@ public class MultiverseCore extends MultiverseModule {
         }
         Logging.setShowingConfig(shouldShowConfig());
 
+        // Build it here so our logger can work, and failure messages will be logged
+        SpawnCategoryMapper.buildSpawnCategoryMap();
+
         // Initialize the worlds
         worldManagerProvider.get().initAllWorlds().andThenTry(() -> {
             loadEconomist(); // Setup economy here so vault is loaded

src/main/java/org/mvplugins/multiverse/core/commands/TeleportCommand.java

@@ -95,7 +95,7 @@ final class TeleportCommand extends CoreCommand {
                                       ParsedCommandFlags parsedFlags) {
         if (!permissionsChecker.checkTeleportPermission(issuer.getIssuer(), player, destination)) {
             // TODO localize
-            issuer.sendMessage(player == issuer.getPlayer()
+            issuer.sendError(player == issuer.getPlayer()
                     ? "You do not have permission to teleport yourself!"
                     : "You do not have permission to teleport other players!");
             return;
@@ -135,7 +135,7 @@ final class TeleportCommand extends CoreCommand {
                                          ParsedCommandFlags parsedFlags) {
         if (!permissionsChecker.checkTeleportPermission(issuer.getIssuer(), Arrays.asList(players), destination)) {
             // TODO localize
-            issuer.sendMessage("You do not have permission to teleport all these players!");
+            issuer.sendError("You do not have permission to teleport all these players!");
             return;
         }
 

src/main/java/org/mvplugins/multiverse/core/world/WorldsConfigManager.java

@@ -227,7 +227,7 @@ final class WorldsConfigManager {
             worldsConfig = new YamlConfiguration();
             worldConfigMap.forEach((keyOrName, worldConfig) -> {
                 worldConfig.save().onFailure(e -> {
-                    throw new RuntimeException("Failed to save world %s in config: " + keyOrName, e);
+                    throw new RuntimeException("Failed to save world " + keyOrName + " in config: " + e.getMessage(), e);
                 });
                 worldsConfig.set(encodeConfigKey(keyOrName), worldConfig.getConfigurationSection());
             });

src/main/java/org/mvplugins/multiverse/core/world/entity/EntitySpawnConfig.java

@@ -7,6 +7,7 @@ import org.bukkit.entity.Entity;
 import org.bukkit.entity.EntityType;
 import org.bukkit.entity.SpawnCategory;
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
 import org.mvplugins.multiverse.core.config.CoreConfig;
 import org.mvplugins.multiverse.core.utils.StringFormatter;
 import org.mvplugins.multiverse.core.world.MultiverseWorld;
@@ -20,12 +21,13 @@ public final class EntitySpawnConfig {
     private final CoreConfig config;
     private final Map<SpawnCategory, SpawnCategoryConfig> spawnCategoriesConfig;
 
-    EntitySpawnConfig(CoreConfig config, Map<SpawnCategory, SpawnCategoryConfig> spawnCategoriesConfig) {
+    EntitySpawnConfig(@NotNull CoreConfig config,
+                      @NotNull Map<SpawnCategory, SpawnCategoryConfig> spawnCategoriesConfig) {
         this.config = config;
         this.spawnCategoriesConfig = spawnCategoriesConfig;
     }
 
-    public SpawnCategoryConfig getSpawnCategoryConfig(SpawnCategory spawnCategory) {
+    public @NotNull SpawnCategoryConfig getSpawnCategoryConfig(@NotNull SpawnCategory spawnCategory) {
         return spawnCategoriesConfig.computeIfAbsent(spawnCategory,
                 computeSpawnCategory -> new SpawnCategoryConfig(
                         config,
@@ -35,12 +37,16 @@ public final class EntitySpawnConfig {
     }
 
     @ApiStatus.AvailableSince("5.7")
-    public boolean shouldAllowSpawn(EntityType entityType) {
-        return getSpawnCategoryConfig(SpawnCategoryMapper.getSpawnCategory(entityType)).shouldAllowSpawn(entityType);
+    public boolean shouldAllowSpawn(@NotNull EntityType entityType) {
+        // Ensure it defaults to true if SpawnCategoryMapper fails to find a spawn category for the entity type,
+        // to avoid accidentally breaking mob spawns.
+        return SpawnCategoryMapper.getSpawnCategory(entityType)
+                .map(spawnCategory -> getSpawnCategoryConfig(spawnCategory).shouldAllowSpawn(entityType))
+                .getOrElse(true);
     }
 
-    public boolean shouldAllowSpawn(Entity entity) {
-        return getSpawnCategoryConfig(entity.getSpawnCategory()).shouldAllowSpawn(entity);
+    public boolean shouldAllowSpawn(@NotNull Entity entity) {
+        return shouldAllowSpawn(entity.getType());
     }
 
     public void applyConfigToWorld() {
@@ -57,16 +63,19 @@ public final class EntitySpawnConfig {
     }
 
     @ApiStatus.Internal
-    public ConfigurationSection toSection() {
+    public @NotNull ConfigurationSection toSection() {
         MemoryConfiguration section = new MemoryConfiguration();
         spawnCategoriesConfig.forEach((spawnCategory, spawnCategoryConfig) -> {
-            section.set(spawnCategory.toString().toLowerCase(Locale.ENGLISH), spawnCategoryConfig.saveSection());
+            section.set(String.valueOf(spawnCategory).toLowerCase(Locale.ENGLISH), spawnCategoryConfig.saveSection());
         });
         return section;
     }
 
     @ApiStatus.Internal
-    public static EntitySpawnConfig fromSection(CoreConfig config, ConfigurationSection section) {
+    public static @NotNull EntitySpawnConfig fromSection(
+            @NotNull CoreConfig config,
+            @NotNull ConfigurationSection section
+    ) {
         Map<SpawnCategory, SpawnCategoryConfig> spawnCategoriesConfig = new LinkedHashMap<>();
         Map<String, Object> existingCategories = section.getValues(false);
         for (SpawnCategory category : SpawnCategory.values()) {

src/main/java/org/mvplugins/multiverse/core/world/entity/SpawnCategoryConfig.java

@@ -8,6 +8,7 @@ import org.bukkit.configuration.ConfigurationSection;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.EntityType;
 import org.bukkit.entity.SpawnCategory;
+import org.jetbrains.annotations.NotNull;
 import org.mvplugins.multiverse.core.config.CoreConfig;
 import org.mvplugins.multiverse.core.config.handle.MemoryConfigurationHandle;
 import org.mvplugins.multiverse.core.config.handle.StringPropertyHandle;
@@ -32,7 +33,11 @@ public final class SpawnCategoryConfig {
 
     private MultiverseWorld world;
 
-    SpawnCategoryConfig(CoreConfig config, SpawnCategory spawnCategory, ConfigurationSection section) {
+    SpawnCategoryConfig(
+            @NotNull CoreConfig config,
+            @NotNull SpawnCategory spawnCategory,
+            @NotNull ConfigurationSection section
+    ) {
         this.config = config;
         this.spawnCategory = spawnCategory;
         this.nodes = new Nodes();
@@ -67,7 +72,7 @@ public final class SpawnCategoryConfig {
         });
     }
 
-    private void applyTickPerSpawns(World bukkitWorld) {
+    private void applyTickPerSpawns(@NotNull World bukkitWorld) {
         if (!config.getApplyEntitySpawnLimit()) {
             Logging.finer("World %s %s skipping setTicksPerSpawns due to core config", world.getName(), spawnCategory);
             return;
@@ -85,7 +90,7 @@ public final class SpawnCategoryConfig {
         bukkitWorld.setTicksPerSpawns(spawnCategory, getTickRate());
     }
 
-    private void applySpawnLimit(World bukkitWorld) {
+    private void applySpawnLimit(@NotNull World bukkitWorld) {
         if (!config.getApplyEntitySpawnLimit()) {
             Logging.finer("Skipping World %s %s setSpawnLimit due to core config", world.getName(), spawnCategory);
             return;
@@ -98,11 +103,11 @@ public final class SpawnCategoryConfig {
         bukkitWorld.setSpawnLimit(spawnCategory, getSpawnLimit());
     }
 
-    public StringPropertyHandle getStringPropertyHandle() {
+    public @NotNull StringPropertyHandle getStringPropertyHandle() {
         return stringPropertyHandle;
     }
 
-    public SpawnCategory getSpawnCategory() {
+    public @NotNull SpawnCategory getSpawnCategory() {
         return spawnCategory;
     }
 
@@ -110,7 +115,7 @@ public final class SpawnCategoryConfig {
         return handle.get(nodes.spawn);
     }
 
-    public Try<Void> setSpawn(boolean spawn) {
+    public @NotNull Try<Void> setSpawn(boolean spawn) {
         return handle.set(nodes.spawn, spawn);
     }
 
@@ -118,7 +123,7 @@ public final class SpawnCategoryConfig {
         return handle.get(nodes.tickRate);
     }
 
-    public Try<Void> setTickRate(int tickRate) {
+    public @NotNull Try<Void> setTickRate(int tickRate) {
         return handle.set(nodes.tickRate, tickRate);
     }
 
@@ -126,23 +131,23 @@ public final class SpawnCategoryConfig {
         return handle.get(nodes.spawnLimit);
     }
 
-    public Try<Void> setSpawnLimit(int spawnLimit) {
+    public @NotNull Try<Void> setSpawnLimit(int spawnLimit) {
         return handle.set(nodes.spawnLimit, spawnLimit);
     }
 
-    public List<EntityType> getExceptions() {
+    public @NotNull List<EntityType> getExceptions() {
         return handle.get(nodes.exceptions);
     }
 
-    public Try<Void> setExceptions(List<EntityType> exceptions) {
+    public @NotNull Try<Void> setExceptions(@NotNull List<EntityType> exceptions) {
         return handle.set(nodes.exceptions, exceptions);
     }
 
-    public boolean shouldAllowSpawn(Entity entity) {
+    public boolean shouldAllowSpawn(@NotNull Entity entity) {
         return shouldAllowSpawn(entity.getType());
     }
 
-    public boolean shouldAllowSpawn(EntityType entityType) {
+    public boolean shouldAllowSpawn(@NotNull EntityType entityType) {
         return isSpawn() != getExceptions().contains(entityType);
     }
 

src/main/java/org/mvplugins/multiverse/core/world/entity/SpawnCategoryMapper.java

@@ -1,11 +1,15 @@
 package org.mvplugins.multiverse.core.world.entity;
 
 import com.dumptruckman.minecraft.util.Logging;
+import io.vavr.control.Option;
 import io.vavr.control.Try;
 import org.bukkit.entity.EntityType;
 import org.bukkit.entity.SpawnCategory;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
 import org.mvplugins.multiverse.core.utils.ReflectHelper;
 
+import javax.annotation.Nullable;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -17,23 +21,30 @@ import java.util.Map;
 /**
  * Uses reflection into nms to map entity types to spawn categories.
  */
-final class SpawnCategoryMapper {
+@ApiStatus.Internal
+public final class SpawnCategoryMapper {
 
-    private static final Map<EntityType, SpawnCategory> entityTypeToSpawnCategoryMap;
-    private static final Map<SpawnCategory, List<EntityType>> spawnCategoryMap;
+    private static final @NotNull Map<EntityType, SpawnCategory> entityTypeToSpawnCategoryMap;
+    private static final @NotNull Map<SpawnCategory, List<EntityType>> spawnCategoryMap;
 
     static {
         entityTypeToSpawnCategoryMap = new HashMap<>();
         spawnCategoryMap = new HashMap<>();
-        buildSpawnCategoryMap();
     }
 
-    private static void buildSpawnCategoryMap() {
+    public static void buildSpawnCategoryMap() {
         Class<?> entityTypeClass = ReflectHelper.tryGetClass("net.minecraft.world.entity.EntityType").getOrNull();
         if (entityTypeClass == null) {
             Logging.warning("Failed to find EntityType class. SpawnCategoryMapper will not work.");
             return;
         }
+        // In 26.2, the list of static fields has been moved from EntityType to EntityTypes class.
+        Class<?> entityTypesListClass = ReflectHelper.tryGetClass("net.minecraft.world.entity.EntityTypes").getOrNull();
+        if (entityTypesListClass == null) {
+            // Assume it's the older mc versions that has list of static fields in EntityType class instead.
+            Logging.finer("Failed to find EntityTypes class. falling back to EntityType.");
+            entityTypesListClass = entityTypeClass;
+        }
         Method getCategoryMethod = ReflectHelper.tryGetMethod(entityTypeClass, "getCategory").getOrNull();
         if (getCategoryMethod == null) {
             Logging.warning("Failed to find getCategory method. SpawnCategoryMapper will not work.");
@@ -52,7 +63,10 @@ final class SpawnCategoryMapper {
             Logging.warning("Failed to find toBukkit method. SpawnCategoryMapper will not work.");
             return;
         }
-        Arrays.stream(entityTypeClass.getFields()).forEach(entityTypeField -> Try.of(() -> EntityType.valueOf(entityTypeField.getName()))
+
+        Logging.finer("Mapping entities to its spawn category...");
+
+        Arrays.stream(entityTypesListClass.getFields()).forEach(entityTypeField -> Try.of(() -> EntityType.valueOf(entityTypeField.getName()))
                 .peek(entityType -> ReflectHelper.tryGetStaticFieldValue(entityTypeField, entityTypeClass)
                         .flatMap(nmsEntityType -> ReflectHelper.tryInvokeMethod(nmsEntityType, getCategoryMethod))
                         .flatMap(nsmMobCategory -> ReflectHelper.tryInvokeStaticMethod(toBukkitMethod, nsmMobCategory))
@@ -61,7 +75,22 @@ final class SpawnCategoryMapper {
                         .peek(bukkitSpawnCategory -> entityTypeToSpawnCategoryMap.put(entityType, bukkitSpawnCategory))
                         .peek(bukkitSpawnCategory -> spawnCategoryMap
                                 .computeIfAbsent(bukkitSpawnCategory, ignore -> new ArrayList<>())
-                                .add(entityType))));
+                                .add(entityType))
+                        .onFailure(throwable -> Logging.warning("Failed to map entity type "
+                                + entityTypeField.getName() + " to its spawn category. " + throwable.getLocalizedMessage())))
+                .onFailure(throwable -> Logging.fine("Failed to map entity type "
+                        + entityTypeField.getName() + " to its spawn category. " + throwable.getLocalizedMessage())));
+
+        if (entityTypeToSpawnCategoryMap.isEmpty()) {
+            Logging.warning("entityTypeToSpawnCategoryMap map is empty after building... " +
+                    "SpawnCategoryMapper will not work.");
+        }
+        if (spawnCategoryMap.isEmpty()) {
+            Logging.warning("spawnCategoryMap map is empty after building... " +
+                    "SpawnCategoryMapper will not work.");
+        }
+
+        Logging.finer("Mapped " + entityTypeToSpawnCategoryMap.size() + " entities to its spawn category.");
     }
 
     /**
@@ -70,8 +99,8 @@ final class SpawnCategoryMapper {
      * @param entityType The entity type
      * @return The spawn category that the given entity type is in, or null if it cannot be determined
      */
-    static SpawnCategory getSpawnCategory(EntityType entityType) {
-        return entityTypeToSpawnCategoryMap.get(entityType);
+    static @NotNull Option<SpawnCategory> getSpawnCategory(@Nullable EntityType entityType) {
+        return Option.of(entityTypeToSpawnCategoryMap.get(entityType));
     }
 
     /**
@@ -80,10 +109,11 @@ final class SpawnCategoryMapper {
      * @param spawnCategory The spawn category
      * @return The entity types associated with the spawn category
      */
-    static List<EntityType> getEntityTypes(SpawnCategory spawnCategory) {
-        if (spawnCategoryMap == null) {
-            return Collections.emptyList();
-        }
-        return spawnCategoryMap.get(spawnCategory);
+    static @NotNull List<EntityType> getEntityTypes(@Nullable SpawnCategory spawnCategory) {
+        return spawnCategoryMap.getOrDefault(spawnCategory, Collections.emptyList());
+    }
+
+    private SpawnCategoryMapper() {
+        throw new UnsupportedOperationException("This is a utility class and cannot be instantiated");
     }
 }