Improved implementation of boundSkills

MMOCore-API/src/main/java/net/Indyuce/mmocore/api/MMOCoreAPI.java

@@ -65,7 +65,7 @@ public class MMOCoreAPI {
     public SkillResult cast(PlayerData playerData, ClassSkill skill) {
         PlayerMetadata casterMeta = playerData.getMMOPlayerData().getStatMap().cache(EquipmentSlot.MAIN_HAND);
         TriggerMetadata triggerMeta = new TriggerMetadata(casterMeta, null, null);
-        return new CastableSkill(skill, playerData.getSkillLevel(skill.getSkill())).cast(triggerMeta);
+        return new CastableSkill(skill, playerData).cast(triggerMeta);
     }
 
     /**

MMOCore-API/src/main/java/net/Indyuce/mmocore/api/player/PlayerData.java

@@ -23,7 +23,6 @@ import net.Indyuce.mmocore.skill.binding.BoundSkillInfo;
 import net.Indyuce.mmocore.api.player.social.FriendRequest;
 import net.Indyuce.mmocore.api.player.stats.PlayerStats;
 import net.Indyuce.mmocore.api.quest.PlayerQuests;
-import net.Indyuce.mmocore.api.quest.trigger.SkillModifierTrigger;
 import net.Indyuce.mmocore.api.quest.trigger.StatTrigger;
 import net.Indyuce.mmocore.api.util.Closable;
 import net.Indyuce.mmocore.api.util.MMOCoreUtils;
@@ -43,6 +42,7 @@ import net.Indyuce.mmocore.player.CombatHandler;
 import net.Indyuce.mmocore.player.Unlockable;
 import net.Indyuce.mmocore.skill.ClassSkill;
 import net.Indyuce.mmocore.skill.RegisteredSkill;
+import net.Indyuce.mmocore.skill.binding.SkillSlot;
 import net.Indyuce.mmocore.skill.cast.SkillCastingHandler;
 import net.Indyuce.mmocore.skilltree.IntegerCoordinates;
 import net.Indyuce.mmocore.skilltree.NodeStatus;
@@ -171,13 +171,20 @@ public class PlayerData extends OfflinePlayerData implements Closable, Experienc
             MMOCore.log(Level.SEVERE, "[Userdata] Could not find class " + getProfess().getId() + " while refreshing player data.");
         }
 
-        final Iterator<Integer> ite = boundSkills.keySet().iterator();
+        final Iterator<Map.Entry<Integer, BoundSkillInfo>> ite = boundSkills.entrySet().iterator();
         while (ite.hasNext())
             try {
-                final int slot = ite.next();
+                final Map.Entry<Integer, BoundSkillInfo> entry = ite.next();
-                boundSkills.put(slot, new BoundSkillInfo(boundSkills.get(slot)));
+                final @Nullable SkillSlot skillSlot = getProfess().getSkillSlot(entry.getKey());
+                final String skillId = entry.getValue().getClassSkill().getSkill().getHandler().getId();
+                final @Nullable ClassSkill classSkill = getProfess().getSkill(skillId);
+                Validate.notNull(skillSlot, "Could not find skill slot n" + entry.getKey());
+                Validate.notNull(skillSlot, "Could not find skill with ID '" + skillId + "'");
+
+                entry.getValue().close();
+                boundSkills.put(entry.getKey(), new BoundSkillInfo(skillSlot, classSkill, this));
             } catch (Exception exception) {
-                exception.printStackTrace();
+                MMOCore.plugin.getLogger().log(Level.WARNING, "Could not reload data of '" + getPlayer().getName() + "': " + exception.getMessage());
             }
 
         for (SkillTree skillTree : getProfess().getSkillTrees())
@@ -1070,16 +1077,10 @@ public class PlayerData extends OfflinePlayerData implements Closable, Experienc
 
     public void setSkillLevel(String skill, int level) {
         skills.put(skill, level);
-        refreshBoundSkill(skill);
     }
 
     public void resetSkillLevel(String skill) {
         skills.remove(skill);
-        refreshBoundSkill(skill);
-    }
-
-    public void refreshBoundSkill(String skill) {
-        boundSkills.values().stream().filter(skillInfo -> skillInfo.getClassSkill().getSkill().getHandler().getId().equals(skill)).forEach(BoundSkillInfo::refresh);
     }
 
     @Deprecated
@@ -1136,9 +1137,9 @@ public class PlayerData extends OfflinePlayerData implements Closable, Experienc
     public void setClass(@Nullable PlayerClass profess) {
         this.profess = profess;
 
-        // Clear old skills
+        // Clear bound skills
-        for (Iterator<BoundSkillInfo> iterator = boundSkills.values().iterator(); iterator.hasNext(); )
+        boundSkills.forEach((slot, info) -> info.close());
-            if (!getProfess().hasSkill(iterator.next().getClassSkill().getSkill())) iterator.remove();
+        boundSkills.clear();
 
         // Update stats
         if (isOnline()) getStats().updateStats();
@@ -1168,30 +1169,20 @@ public class PlayerData extends OfflinePlayerData implements Closable, Experienc
     public void bindSkill(int slot, ClassSkill skill) {
         Validate.notNull(skill, "Skill cannot be null");
 
-        // Unbinds the previous skill (Important for passive skills.
+        // Unbinds the previous skill (important for passive skills)
-        if (boundSkills.containsKey(slot)) boundSkills.get(slot).unbind();
+        unbindSkill(slot);
 
         if (slot >= 0) {
-
+            final SkillSlot skillSlot = getProfess().getSkillSlot(slot);
-            // We apply the skill buffs associated with the slot to the skill.
+            boundSkills.put(slot, new BoundSkillInfo(skillSlot, skill, this));
-            for (SkillModifierTrigger skillBuffTrigger : profess.getSkillSlot(slot).getSkillBuffTriggers())
-                if (skillBuffTrigger.getTargetSkills().contains(skill.getSkill().getHandler()))
-                    skillBuffTrigger.apply(this, skill.getSkill().getHandler());
-
-            boundSkills.put(slot, new BoundSkillInfo(skill, this));
         }
     }
 
     public void unbindSkill(int slot) {
-
+        final @Nullable BoundSkillInfo boundSkillInfo = boundSkills.remove(slot);
-        // We remove the skill buffs associated with the slot from the skill that is .
+        if (boundSkillInfo != null) boundSkillInfo.close();
-        profess.getSkillSlot(slot).getSkillBuffTriggers().forEach(skillBuffTrigger -> skillBuffTrigger.remove(this, boundSkills.get(slot).getClassSkill().getSkill().getHandler()));
-
-        BoundSkillInfo boundSkillInfo = boundSkills.remove(slot);
-        boundSkillInfo.unbind();
     }
 
-
     public List<ClassSkill> getBoundSkills() {
         return boundSkills.values().stream().map(BoundSkillInfo::getClassSkill).collect(Collectors.toList());
     }

MMOCore-API/src/main/java/net/Indyuce/mmocore/api/player/profess/PlayerClass.java

@@ -184,11 +184,14 @@ public class PlayerClass extends PostLoadObject implements ExperienceObject {
                 }
 
         // Skill slots
-        for (int i = 1; i < MMOCore.plugin.configManager.maxSkillSlots + 1; i++) {
+        for (int i = 1; i < MMOCore.plugin.configManager.maxSkillSlots + 1; i++)
+            try {
                 if (config.contains("skill-slots." + i))
                     skillSlots.put(i, new SkillSlot(config.getConfigurationSection("skill-slots." + i)));
                 else
                     skillSlots.put(i, new SkillSlot(i, 0, "true", "&eSkill Slot " + MMOCoreUtils.intToRoman(i), new ArrayList<>(), false, true, new ArrayList<>()));
+            } catch (RuntimeException exception) {
+                MMOCore.plugin.getLogger().log(Level.WARNING, "Could not load skill slot '" + String.valueOf(i) + "' from class '" + getId() + "': " + exception.getMessage());
             }
 
         // Class skills

MMOCore-API/src/main/java/net/Indyuce/mmocore/api/quest/trigger/SkillModifierTrigger.java

@@ -1,5 +1,6 @@
 package net.Indyuce.mmocore.api.quest.trigger;
 
+import io.lumine.mythic.lib.UtilityMethods;
 import io.lumine.mythic.lib.api.MMOLineConfig;
 import io.lumine.mythic.lib.player.modifier.ModifierType;
 import io.lumine.mythic.lib.player.skillmod.SkillModifier;
@@ -8,7 +9,6 @@ import net.Indyuce.mmocore.MMOCore;
 import net.Indyuce.mmocore.api.player.PlayerData;
 import net.Indyuce.mmocore.api.quest.trigger.api.Removable;
 import net.Indyuce.mmocore.skill.RegisteredSkill;
-import org.apache.commons.lang.Validate;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -24,18 +24,17 @@ public class SkillModifierTrigger extends Trigger implements Removable {
 
         config.validateKeys("modifier");
         config.validateKeys("amount");
-        config.validateKeys("type");
+
         amount = config.getDouble("amount");
-        String skillModifier = config.getString("modifier");
+        final String skillModifier = config.getString("modifier");
-        String formula = config.getString("formula", "true");
+        final String formula = config.getString("formula", "true");
-        String type = config.getString("type").toUpperCase();
+        final ModifierType type = config.contains("type") ? ModifierType.valueOf(UtilityMethods.enumName(config.getString("type"))) : ModifierType.FLAT;
-        Validate.isTrue(type.equals("FLAT") || type.equals("RELATIVE"));
         List<SkillHandler<?>> targetSkills = new ArrayList<>();
-        for (RegisteredSkill skill : MMOCore.plugin.skillManager.getAll()) {
+        for (RegisteredSkill skill : MMOCore.plugin.skillManager.getAll())
             if (skill.matchesFormula(formula))
                 targetSkills.add(skill.getHandler());
-        }
+
-        mod = new SkillModifier(buffKey, skillModifier, targetSkills, amount, ModifierType.valueOf(type));
+        mod = new SkillModifier(buffKey, skillModifier, targetSkills, amount, type);
     }
 
     public List<SkillHandler<?>> getTargetSkills() {
@@ -53,14 +52,16 @@ public class SkillModifierTrigger extends Trigger implements Removable {
     }
 
     /**
-     * Used by skill slots to apply a skillBuff to a specific skill dynamically chosen .
+     * Used by skill slots to apply a skillBuff
+     * to a dynamically chosen skill handler.
      */
     public void apply(PlayerData playerData, SkillHandler<?> skill) {
         mod.register(playerData.getMMOPlayerData(), skill);
     }
 
     /**
-     * Used by skill slots to remove a skillBuff from a specific skill dynamically chosen.
+     * Used by skill slots to remove a skillBuff
+     * from a dynamically chosen skill handler.
      */
     public void remove(PlayerData playerData, SkillHandler<?> skill) {
         mod.unregister(playerData.getMMOPlayerData(), skill);

MMOCore-API/src/main/java/net/Indyuce/mmocore/skill/CastableSkill.java

@@ -12,15 +12,30 @@ import net.Indyuce.mmocore.api.player.PlayerActivity;
 import net.Indyuce.mmocore.api.player.PlayerData;
 import net.Indyuce.mmocore.api.event.PlayerResourceUpdateEvent;
 
+import javax.inject.Provider;
+
 public class CastableSkill extends Skill {
     private final ClassSkill skill;
-    private final int skillLevel;
 
-    public CastableSkill(ClassSkill skill, int skillLevel) {
+    /**
+     * Now uses a provider since 1.12 because the skill level can CHANGE
+     * with time, and such instance of CastableSkill is
+     */
+    private final Provider<Integer> skillLevel;
+
+    @Deprecated
+    public CastableSkill(ClassSkill skill, int fixedLevel) {
         super(skill.getSkill().getTrigger());
 
         this.skill = skill;
-        this.skillLevel = skillLevel;
+        this.skillLevel = () -> fixedLevel;
+    }
+
+    public CastableSkill(ClassSkill skill, PlayerData playerData) {
+        super(skill.getSkill().getTrigger());
+
+        this.skill = skill;
+        this.skillLevel = () -> playerData.getSkillLevel(skill.getSkill());
     }
 
     public ClassSkill getSkill() {
@@ -97,6 +112,6 @@ public class CastableSkill extends Skill {
 
     @Override
     public double getModifier(String mod) {
-        return skill.getModifier(mod, skillLevel);
+        return skill.getModifier(mod, skillLevel.get());
     }
 }

MMOCore-API/src/main/java/net/Indyuce/mmocore/skill/ClassSkill.java

@@ -132,10 +132,9 @@ public class ClassSkill implements CooldownObject, Unlockable {
     }
 
     public CastableSkill toCastable(PlayerData caster) {
-        return new CastableSkill(this, caster.getSkillLevel(getSkill()));
+        return new CastableSkill(this, caster);
     }
 
-
     /**
      * Be careful, this method creates a new UUID each time it
      * is called. It needs to be saved somewhere when trying to

MMOCore-API/src/main/java/net/Indyuce/mmocore/skill/binding/BoundSkillInfo.java

@@ -2,47 +2,41 @@ package net.Indyuce.mmocore.skill.binding;
 
 import io.lumine.mythic.lib.player.skill.PassiveSkill;
 import net.Indyuce.mmocore.api.player.PlayerData;
+import net.Indyuce.mmocore.api.quest.trigger.SkillModifierTrigger;
+import net.Indyuce.mmocore.api.util.Closable;
 import net.Indyuce.mmocore.skill.ClassSkill;
-import org.bukkit.Bukkit;
+import org.apache.commons.lang.Validate;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.Objects;
+public class BoundSkillInfo implements Closable {
-import java.util.UUID;
+    private final SkillSlot skillSlot;
-
-public class BoundSkillInfo {
     private final PlayerData playerData;
     private final ClassSkill classSkill;
 
     /**
-     * Private skills must be registered inside of MythicLib when bound.
+     * PASSIVE skills must be registered inside of MythicLib when
-     * When set to null, the skill is NOT registered.
+     * when bound. When set to null, the skill is not registered.
      */
     @Nullable
-    private PassiveSkill registered;
+    private final PassiveSkill registered;
 
-    public BoundSkillInfo(ClassSkill classSkill, PlayerData playerData) {
+    private boolean open = true;
+
+    public BoundSkillInfo(@NotNull SkillSlot skillSlot, @NotNull ClassSkill classSkill, @NotNull PlayerData playerData) {
+        this.skillSlot = skillSlot;
         this.classSkill = classSkill;
         this.playerData = playerData;
 
+        // Apply skill buffs associated to the slot
+        for (SkillModifierTrigger skillBuffTrigger : skillSlot.getSkillBuffTriggers())
+            if (skillBuffTrigger.getTargetSkills().contains(classSkill.getSkill().getHandler()))
+                skillBuffTrigger.apply(playerData, classSkill.getSkill().getHandler());
+
         if (classSkill.getSkill().getTrigger().isPassive()) {
             registered = classSkill.toPassive(playerData);
             registered.register(playerData.getMMOPlayerData());
-        }
+        } else registered = null;
-    }
-
-    /**
-     * Used on update to refresh the classSkill & all references to old data.
-     */
-    public BoundSkillInfo(BoundSkillInfo info) {
-        this.playerData = info.getPlayerData();
-        this.classSkill = Objects.requireNonNull(playerData.getProfess().getSkill(info.getClassSkill().getSkill()));
-
-        if (classSkill.getSkill().getTrigger().isPassive()) {
-            info.unbind();
-            registered = classSkill.toPassive(playerData);
-            registered.register(playerData.getMMOPlayerData());
-        }
     }
 
     @NotNull
@@ -55,23 +49,24 @@ public class BoundSkillInfo {
         return playerData;
     }
 
+    @NotNull
+    public SkillSlot getSkillSlot() {
+        return skillSlot;
+    }
+
     public boolean isPassive() {
         return registered != null;
     }
 
-    /**
+    @Override
-     * This is used to refresh the PassiveSkill playerModifier
+    public void close() {
-     * so it is always associated to the right skill level.
+        Validate.isTrue(open, "BoundSkillInfo has already been closed");
-     */
+        open = false;
-    public void refresh() {
-        if (isPassive()) {
-            registered.unregister(playerData.getMMOPlayerData());
-            registered = classSkill.toPassive(playerData);
-            registered.register(playerData.getMMOPlayerData());
-        }
-    }
 
-    public void unbind() {
+        // Unregister skill if passive
         if (isPassive()) registered.unregister(playerData.getMMOPlayerData());
+
+        // Remove skill buffs associated to the slot
+        skillSlot.getSkillBuffTriggers().forEach(skillBuffTrigger -> skillBuffTrigger.remove(playerData, classSkill.getSkill().getHandler()));
     }
 }