fix: attribute issues (#3193)

* fix: NPEs from attribute related stuff

* chore: clarify comment

* fix: issues when using attribute key as argument

Changes:
Added argument parsing backward compatibility for /npc attribute command but also introduces more time cost (although it might be not so long, so we can assume it as zero)
Make OptionalAttributeCompletions return attribute keys for now as old enum names are too legacy.

* fix: rewrite attribute trait load logic

main/src/main/java/net/citizensnpcs/commands/NPCCommands.java

@@ -30,6 +30,7 @@ import org.bukkit.Registry;
 import org.bukkit.Rotation;
 import org.bukkit.Sound;
 import org.bukkit.World;
+import org.bukkit.attribute.Attribute;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
 import org.bukkit.command.CommandSender;
@@ -422,13 +423,18 @@ public class NPCCommands {
     public void attribute(CommandContext args, CommandSender sender, NPC npc,
             @Arg(value = 1, completionsProvider = OptionalAttributeCompletions.class) String attribute,
             @Arg(2) Double value) {
-
+        final Attribute attr = Util.getAttribute(attribute);
+        if (attr == null) {
+            // todo an translation key is necessary here
+            sender.sendMessage("Attribute not found");
+            return;
+        }
         AttributeTrait trait = npc.getOrAddTrait(AttributeTrait.class);
         if (value == null) {
-            trait.setDefaultAttribute(Registry.ATTRIBUTE.get(SpigotUtil.getKey(attribute)));
+            trait.setDefaultAttribute(attr);
             Messaging.sendTr(sender, Messages.ATTRIBUTE_RESET, attribute);
         } else {
-            trait.setAttributeValue(Registry.ATTRIBUTE.get(SpigotUtil.getKey(attribute)), value);
+            trait.setAttributeValue(attr, value);
             Messaging.sendTr(sender, Messages.ATTRIBUTE_SET, attribute, value);
         }
     }
@@ -3755,10 +3761,10 @@ public class NPCCommands {
                 trait.isTamed(), trait.getCollarColor().name());
     }
 
-    public static class OptionalAttributeCompletions extends OptionalEnumCompletions {
+    public static class OptionalAttributeCompletions implements Arg.CompletionsProvider {
         @Override
-        public String getEnumClassName() {
-            return "org.bukkit.attribute.Attribute";
+        public Collection<String> getCompletions(CommandContext args, CommandSender sender, NPC npc) {
+            return Arrays.stream(Attribute.values()).map(attr -> attr.getKey().toString()).collect(Collectors.toList());
         }
     }
 

main/src/main/java/net/citizensnpcs/trait/AttributeTrait.java

@@ -2,6 +2,9 @@ package net.citizensnpcs.trait;
 
 import java.util.Map;
 
+import net.citizensnpcs.api.exception.NPCLoadException;
+import net.citizensnpcs.api.util.DataKey;
+import net.citizensnpcs.util.Util;
 import org.bukkit.attribute.Attribute;
 import org.bukkit.attribute.AttributeInstance;
 import org.bukkit.entity.LivingEntity;
@@ -29,13 +32,32 @@ public class AttributeTrait extends Trait {
         return attributes.containsKey(attribute);
     }
 
+    @Override
+    public void load(DataKey key) throws NPCLoadException {
+        for (Map.Entry<String, Object> entry : key.getValuesDeep().entrySet()) {
+            final String rawAttributeName = entry.getKey();
+            final Attribute attribute = Util.getAttribute(rawAttributeName);
+            if (attribute != null) {
+                final Object rawValue = entry.getValue();
+                if (rawValue instanceof Double) {
+                    attributes.put(attribute, (Double) rawValue);
+                }
+            }
+        }
+    }
+
     @Override
     public void onSpawn() {
         if (!(npc.getEntity() instanceof LivingEntity))
             return;
         LivingEntity le = (LivingEntity) npc.getEntity();
         for (Map.Entry<Attribute, Double> entry : attributes.entrySet()) {
-            le.getAttribute(entry.getKey()).setBaseValue(entry.getValue());
+            final Attribute key = entry.getKey();
+            final AttributeInstance attributeInstance = le.getAttribute(key);
+            if (attributeInstance == null) { // not applicable anymore so ignore
+                continue;
+            }
+            attributeInstance.setBaseValue(entry.getValue());
         }
     }
 

main/src/main/java/net/citizensnpcs/util/Util.java

@@ -24,6 +24,7 @@ import org.bukkit.Material;
 import org.bukkit.NamespacedKey;
 import org.bukkit.Registry;
 import org.bukkit.World;
+import org.bukkit.attribute.Attribute;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
 import org.bukkit.entity.Entity;
@@ -266,6 +267,20 @@ public class Util {
         return null;
     }
 
+    public static Attribute getAttribute(String... keyCandidates) {
+        for (String keyCandidate : keyCandidates) {
+            boolean isFullUpperCase = keyCandidate.toUpperCase(Locale.ENGLISH).equals(keyCandidate);
+            if (isFullUpperCase) { // we assume it is an enum key
+                try {
+                    // Just imagine we're still on older API (1.21.3-, exclusive)
+                    // noinspection deprecation
+                    return Attribute.valueOf(keyCandidate);
+                } catch (IllegalArgumentException ignored) {} // huh, not?
+            }
+        }
+        return getRegistryValue(Registry.ATTRIBUTE, keyCandidates);
+    }
+
     public static String getTeamName(UUID id) {
         return "CIT-" + id.toString().replace("-", "").substring(0, 12);
     }