refactor: clean up 1.21.1 compatibility issues

Close #368

bukkit/src/main/java/net/william278/husksync/data/BukkitData.java

@@ -25,9 +25,7 @@ import com.google.gson.annotations.SerializedName;
 import de.tr7zw.changeme.nbtapi.NBTCompound;
 import de.tr7zw.changeme.nbtapi.NBTPersistentDataContainer;
 import lombok.*;
-import net.kyori.adventure.util.TriState;
 import net.william278.desertwell.util.ThrowingConsumer;
-import net.william278.desertwell.util.Version;
 import net.william278.husksync.BukkitHuskSync;
 import net.william278.husksync.HuskSync;
 import net.william278.husksync.adapter.Adaptable;
@@ -39,7 +37,7 @@ import org.bukkit.attribute.AttributeInstance;
 import org.bukkit.attribute.AttributeModifier;
 import org.bukkit.entity.Player;
 import org.bukkit.event.inventory.InventoryType;
-import org.bukkit.inventory.EquipmentSlot;
+import org.bukkit.inventory.EquipmentSlotGroup;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.persistence.PersistentDataContainer;
 import org.bukkit.potion.PotionEffect;
@@ -49,7 +47,6 @@ import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.Range;
 import org.jetbrains.annotations.Unmodifiable;
 
-import java.lang.reflect.Constructor;
 import java.util.*;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
@@ -280,7 +277,7 @@ public abstract class BukkitData implements Data {
         public List<Effect> getActiveEffects() {
             return effects.stream()
                     .map(potionEffect -> new Effect(
-                            potionEffect.getType().getName().toLowerCase(Locale.ENGLISH),
+                            potionEffect.getType().getKey().toString(),
                             potionEffect.getAmplifier(),
                             potionEffect.getDuration(),
                             potionEffect.isAmbient(),
@@ -563,13 +560,9 @@ public abstract class BukkitData implements Data {
     @Getter
     @AllArgsConstructor(access = AccessLevel.PRIVATE)
     @NoArgsConstructor(access = AccessLevel.PRIVATE)
+    @SuppressWarnings("UnstableApiUsage")
     public static class Attributes extends BukkitData implements Data.Attributes, Adaptable {
 
-        private static final String EQUIPMENT_SLOT_GROUP = "org.bukkit.inventory.EquipmentSlotGroup";
-        private static final String EQUIPMENT_SLOT_GROUP$ANY = "ANY";
-        private static final String EQUIPMENT_SLOT$getGroup = "getGroup";
-        private static TriState USE_KEYED_MODIFIERS = TriState.NOT_SET;
-
         private List<Attribute> attributes;
 
         @NotNull
@@ -607,6 +600,7 @@ public abstract class BukkitData implements Data {
                     instance.getBaseValue(),
                     instance.getModifiers().stream()
                             .filter(modifier -> !settings.isIgnoredModifier(modifier.getName()))
+                            .filter(modifier -> modifier.getSlotGroup() != EquipmentSlotGroup.ANY)
                             .map(BukkitData.Attributes::adapt).collect(Collectors.toSet())
             );
         }
@@ -614,34 +608,14 @@ public abstract class BukkitData implements Data {
         @NotNull
         private static Modifier adapt(@NotNull AttributeModifier modifier) {
             return new Modifier(
-                    getModifierId(modifier),
+                    modifier.getKey().toString(),
-                    modifier.getName(),
                     modifier.getAmount(),
                     modifier.getOperation().ordinal(),
-                    modifier.getSlot() != null ? modifier.getSlot().ordinal() : -1
+                    modifier.getSlotGroup().toString()
             );
         }
 
-        @Nullable
+        private static void applyAttribute(@Nullable AttributeInstance instance, @Nullable Attribute attribute) {
-        private static UUID getModifierId(@NotNull AttributeModifier modifier) {
-            try {
-                return modifier.getUniqueId();
-            } catch (Throwable e) {
-                return null;
-            }
-        }
-
-        private static boolean useKeyedModifiers(@NotNull HuskSync plugin) {
-            if (USE_KEYED_MODIFIERS == TriState.NOT_SET) {
-                boolean is1_21 = plugin.getMinecraftVersion().compareTo(Version.fromString("1.21")) >= 0;
-                USE_KEYED_MODIFIERS = TriState.byBoolean(is1_21);
-                return is1_21;
-            }
-            return Boolean.TRUE.equals(USE_KEYED_MODIFIERS.toBoolean());
-        }
-
-        private static void applyAttribute(@Nullable AttributeInstance instance, @Nullable Attribute attribute,
-                                           @NotNull HuskSync plugin) {
             if (instance == null) {
                 return;
             }
@@ -651,51 +625,25 @@ public abstract class BukkitData implements Data {
                 attribute.modifiers().stream()
                         .filter(mod -> instance.getModifiers().stream().map(AttributeModifier::getName)
                                 .noneMatch(n -> n.equals(mod.name())))
-                        .distinct()
+                        .distinct().filter(mod -> !mod.hasUuid())
-                        .filter(mod -> useKeyedModifiers(plugin) == !mod.hasUuid())
+                        .forEach(mod -> instance.addModifier(adapt(mod)));
-                        .forEach(mod -> instance.addModifier(adapt(mod, plugin)));
             }
         }
 
-        @SuppressWarnings("JavaReflectionMemberAccess")
         @NotNull
-        private static AttributeModifier adapt(@NotNull Modifier modifier, @NotNull HuskSync plugin) {
+        private static AttributeModifier adapt(@NotNull Modifier modifier) {
-            final int slotId = modifier.equipmentSlot();
-            if (useKeyedModifiers(plugin)) {
-                try {
-                    // Reflexively create a modern keyed attribute modifier instance. Remove in favor of API long-term.
-                    final EquipmentSlot slot = slotId != -1 ? EquipmentSlot.values()[slotId] : null;
-                    final Class<?> slotGroup = Class.forName(EQUIPMENT_SLOT_GROUP);
-                    final String modifierName = modifier.name() == null ? modifier.uuid().toString() : modifier.name();
-                    final NamespacedKey modifierKey = Objects.requireNonNull(NamespacedKey.fromString(modifierName),
-                            "Modifier key returned null");
-                    final Constructor<AttributeModifier> constructor = AttributeModifier.class.getDeclaredConstructor(
-                            NamespacedKey.class, double.class, AttributeModifier.Operation.class, slotGroup);
-                    return constructor.newInstance(
-                            modifierKey,
-                            modifier.amount(),
-                            AttributeModifier.Operation.values()[modifier.operationType()],
-                            slot == null ? slotGroup.getField(EQUIPMENT_SLOT_GROUP$ANY).get(null)
-                                    : EquipmentSlot.class.getDeclaredMethod(EQUIPMENT_SLOT$getGroup).invoke(slot)
-                    );
-                } catch (Throwable e) {
-                    plugin.log(Level.WARNING, "Error reflectively creating keyed attribute modifier", e);
-                    USE_KEYED_MODIFIERS = TriState.FALSE;
-                }
-            }
             return new AttributeModifier(
-                    modifier.uuid(),
+                    Objects.requireNonNull(NamespacedKey.fromString(modifier.name())),
-                    modifier.name(),
                     modifier.amount(),
-                    AttributeModifier.Operation.values()[modifier.operationType()],
+                    AttributeModifier.Operation.values()[modifier.operation()],
-                    slotId != -1 ? EquipmentSlot.values()[slotId] : null
+                    modifier.equipmentSlotGroup().map(EquipmentSlotGroup::getByName).orElse(EquipmentSlotGroup.ANY)
             );
         }
 
         @Override
         public void apply(@NotNull BukkitUser user, @NotNull BukkitHuskSync plugin) throws IllegalStateException {
             Registry.ATTRIBUTE.forEach(id -> applyAttribute(
-                    user.getPlayer().getAttribute(id), getAttribute(id).orElse(null), plugin
+                    user.getPlayer().getAttribute(id), getAttribute(id).orElse(null)
             ));
         }
 

bukkit/src/main/java/net/william278/husksync/util/BukkitKeyedAdapter.java

@@ -51,7 +51,7 @@ public final class BukkitKeyedAdapter {
 
     @Nullable
     public static PotionEffectType matchEffectType(@NotNull String key) {
-        return PotionEffectType.getByName(key); // No registry for this in 1.17 API
+        return getRegistryValue(Registry.EFFECT, key);
     }
 
     private static <T extends Keyed> T getRegistryValue(@NotNull Registry<T> registry, @NotNull String keyString) {

common/src/main/java/net/william278/husksync/data/Data.java

@@ -22,9 +22,9 @@ package net.william278.husksync.data;
 import com.google.common.collect.Sets;
 import com.google.gson.annotations.SerializedName;
 import lombok.AccessLevel;
-import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
+import lombok.RequiredArgsConstructor;
 import lombok.experimental.Accessors;
 import net.kyori.adventure.key.Key;
 import net.william278.husksync.HuskSync;
@@ -132,7 +132,7 @@ public interface Data {
         /**
          * Represents a potion effect
          *
-         * @param type          the type of potion effect
+         * @param type          the key of potion effect
          * @param amplifier     the amplifier of the potion effect
          * @param duration      the duration of the potion effect
          * @param isAmbient     whether the potion effect is ambient
@@ -341,42 +341,72 @@ public interface Data {
 
         @Getter
         @Accessors(fluent = true)
-        @AllArgsConstructor
+        @RequiredArgsConstructor
         @NoArgsConstructor
         final class Modifier {
+            final static String ANY_EQUIPMENT_SLOT_GROUP = "any";
+
             @Getter(AccessLevel.NONE)
             @Nullable
             @SerializedName("uuid")
+            @Deprecated(since = "3.7")
             private UUID uuid;
+
+            // Since 1.21.1: Name, amount, operation, slotGroup
             @SerializedName("name")
             private String name;
+
             @SerializedName("amount")
             private double amount;
+
             @SerializedName("operation")
-            private int operationType;
+            private int operation;
+
             @SerializedName("equipment_slot")
+            @Deprecated(since = "3.7")
             private int equipmentSlot;
 
-            public Modifier(@NotNull String name, double amount, int operationType, int equipmentSlot) {
+            @Getter(AccessLevel.NONE)
+            @SerializedName("equipment_slot_group")
+            @Nullable
+            private String slotGroup;
+
+            public Modifier(@NotNull String name, double amount, int operation, @NotNull String slotGroup) {
                 this.name = name;
                 this.amount = amount;
-                this.operationType = operationType;
+                this.operation = operation;
+                this.slotGroup = slotGroup;
+            }
+
+            @Deprecated(since = "3.7")
+            public Modifier(@NotNull String name, double amount, int operation, int equipmentSlot) {
+                this.name = name;
+                this.amount = amount;
+                this.operation = operation;
+                this.equipmentSlot = equipmentSlot;
+            }
+
+            @Deprecated(since = "3.7")
+            public Modifier(@NotNull UUID uuid, @NotNull String name, double amount, int operation, int equipmentSlot) {
+                this.name = name;
+                this.amount = amount;
+                this.operation = operation;
                 this.equipmentSlot = equipmentSlot;
             }
 
             @Override
             public boolean equals(Object obj) {
                 if (obj instanceof Modifier other) {
-                    if (uuid == null || other.uuid == null) {
+                    if (uuid != null && other.uuid != null) {
-                        return name.equals(other.name);
-                    }
                         return uuid.equals(other.uuid);
                     }
+                    return name.equals(other.name);
+                }
                 return super.equals(obj);
             }
 
             public double modify(double value) {
-                return switch (operationType) {
+                return switch (operation) {
                     case 0 -> value + amount;
                     case 1 -> value * amount;
                     case 2 -> value * (1 + amount);
@@ -393,6 +423,10 @@ public interface Data {
                 return uuid != null ? uuid : UUID.nameUUIDFromBytes(name.getBytes());
             }
 
+            Optional<String> equipmentSlotGroup() {
+                return Optional.ofNullable(slotGroup);
+            }
+
         }
 
         default Optional<Attribute> getAttribute(@NotNull Key key) {

fabric/src/main/java/net/william278/husksync/data/FabricData.java

@@ -578,11 +578,10 @@ public abstract class FabricData implements Data {
                 }
                 final Set<Modifier> modifiers = Sets.newHashSet();
                 instance.getModifiers().forEach(modifier -> modifiers.add(new Modifier(
-                        UUID.nameUUIDFromBytes(modifier.id().toString().getBytes()),
+                        modifier.id().toString(),
-                        modifier.id().examinableName(),
                         modifier.value(),
                         modifier.operation().getId(),
-                        -1
+                        Modifier.ANY_EQUIPMENT_SLOT_GROUP
                 )));
                 attributes.add(new Attribute(
                         key.toString(),
@@ -624,10 +623,10 @@ public abstract class FabricData implements Data {
             instance.setBaseValue(attribute == null ? instance.getAttribute().value().getDefaultValue() : attribute.baseValue());
             instance.getModifiers().forEach(instance::removeModifier);
             if (attribute != null) {
-                attribute.modifiers().forEach(modifier -> instance.addPersistentModifier(new EntityAttributeModifier(
+                attribute.modifiers().forEach(modifier -> instance.addTemporaryModifier(new EntityAttributeModifier(
                         Identifier.of(modifier.uuid().toString()),
                         modifier.amount(),
-                        EntityAttributeModifier.Operation.ID_TO_VALUE.apply(modifier.operationType())
+                        EntityAttributeModifier.Operation.ID_TO_VALUE.apply(modifier.operation())
                 )));
             }
         }