From 13ffbd642b094d25c9d55e3697dc68fddb4c94fc Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:18:35 +0200 Subject: [PATCH 1/2] fix: bind skills.yml spells to the rune handler, not a class copy skills.yml keys are stored lowercase while MythicLib ids are upper case, so the exact lookup always missed and the loop matched the first skill whose display name equals the key. For Restoration, Maelstrom and Frostveil that was the MMOCore class copy (CLASS_RESTORATION, ...), which then got the rune's alignment modifiers (+20% mana and cooldown at 0 alignment) while the rune got none. Look up the upper-cased id first, then handler ids, and fall back to display names last. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../magic/integration/SkillIdResolver.java | 25 +++++++++++-------- .../magic/SkillResolverEdgeTest.java | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/main/java/net/tfminecraft/magic/integration/SkillIdResolver.java b/src/main/java/net/tfminecraft/magic/integration/SkillIdResolver.java index 03ed7b8..ef30293 100644 --- a/src/main/java/net/tfminecraft/magic/integration/SkillIdResolver.java +++ b/src/main/java/net/tfminecraft/magic/integration/SkillIdResolver.java @@ -74,22 +74,27 @@ public static SkillHandler handlerForBinding(String skillId) { if (skillId == null || skillId.isBlank() || MMOCore.plugin == null) { return null; } - RegisteredSkill exact = MMOCore.plugin.skillManager.getSkill(skillId); - if (exact != null && exact.getHandler() != null) { - return exact.getHandler(); + // Binding keys are stored lowercase and MythicLib ids are uppercase, so the exact lookup + // needs the upper-cased id. + for (String id : new String[] {skillId, skillId.toUpperCase(Locale.ROOT)}) { + RegisteredSkill exact = MMOCore.plugin.skillManager.getSkill(id); + if (exact != null && exact.getHandler() != null) { + return exact.getHandler(); + } } + // Handler ids before display names: a class copy (CLASS_RESTORATION, shown as "Restoration") + // shares the rune's name, and a name match bound the rune's modifiers to the class skill. for (RegisteredSkill skill : MMOCore.plugin.skillManager.getAll()) { - if (skill == null) { - continue; - } - if (skill.getName() != null && skill.getName().equalsIgnoreCase(skillId)) { - return skill.getHandler(); - } - SkillHandler handler = skill.getHandler(); + SkillHandler handler = skill == null ? null : skill.getHandler(); if (handler != null && skillId.equalsIgnoreCase(handler.getLowerCaseId())) { return handler; } } + for (RegisteredSkill skill : MMOCore.plugin.skillManager.getAll()) { + if (skill != null && skill.getName() != null && skill.getName().equalsIgnoreCase(skillId)) { + return skill.getHandler(); + } + } return null; } } diff --git a/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java b/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java index b24a826..0c6dcd2 100644 --- a/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java +++ b/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java @@ -84,4 +84,29 @@ void bindingLookupSupportsNamesAndHandlersCaseInsensitively() throws Exception { assertSame(second, SkillIdResolver.handlerForBinding("ABILITY")); assertNull(SkillIdResolver.handlerForBinding("unknown")); } + + @Test + void bindingPrefersTheRuneHandlerOverAClassCopyWithTheSameName() throws Exception { + MMOCore.plugin = mock(MMOCore.class); + var manager = mock(SkillManager.class); + var field = MMOCore.class.getDeclaredField("skillManager"); + field.setAccessible(true); + field.set(MMOCore.plugin, manager); + // CLASS_RESTORATION is shown as "Restoration" and comes first; the rune RESTORATION has the handler id. + var classCopy = mock(RegisteredSkill.class); + var classHandler = mock(SkillHandler.class); + when(classCopy.getName()).thenReturn("Restoration"); + when(classCopy.getHandler()).thenReturn(classHandler); + when(classHandler.getLowerCaseId()).thenReturn("class_restoration"); + var rune = mock(RegisteredSkill.class); + var runeHandler = mock(SkillHandler.class); + when(rune.getName()).thenReturn("Restoration"); + when(rune.getHandler()).thenReturn(runeHandler); + when(runeHandler.getLowerCaseId()).thenReturn("restoration"); + when(manager.getAll()).thenReturn(List.of(classCopy, rune)); + assertSame(runeHandler, SkillIdResolver.handlerForBinding("restoration")); + // skills.yml keys are stored lowercase; the MythicLib id is upper case + when(manager.getSkill("RESTORATION")).thenReturn(rune); + assertSame(runeHandler, SkillIdResolver.handlerForBinding("restoration")); + } } From 3d2d32b00df11638fe793f2c43be5cc8e6650696 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:52:27 +0200 Subject: [PATCH 2/2] test: check the upper-cased exact lookup on its own Co-Authored-By: Claude Opus 5.5 (1M context) --- src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java b/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java index 0c6dcd2..0fc6fea 100644 --- a/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java +++ b/src/test/java/net/tfminecraft/magic/SkillResolverEdgeTest.java @@ -107,6 +107,7 @@ void bindingPrefersTheRuneHandlerOverAClassCopyWithTheSameName() throws Exceptio assertSame(runeHandler, SkillIdResolver.handlerForBinding("restoration")); // skills.yml keys are stored lowercase; the MythicLib id is upper case when(manager.getSkill("RESTORATION")).thenReturn(rune); + when(manager.getAll()).thenReturn(List.of()); // only the upper-cased exact lookup can find it now assertSame(runeHandler, SkillIdResolver.handlerForBinding("restoration")); } }