From 8ff047d5a1a77f0f58c391ac41e9920f0e2b7199 Mon Sep 17 00:00:00 2001 From: nossr50 Date: Mon, 25 May 2026 10:06:19 -0700 Subject: [PATCH] Fix KnockOnWood XP orbs never spawning on nether tree cap blocks during Tree Feller Nether_Wart_Block and Warped_Wart_Block both have woodcutting XP in experience.yml AND are in the treeFellerDestructibleWhiteList. The drop routing in dropTreeFellerLootFromBlocks used an if/else-if structure, so blocks with woodcutting XP always took the first branch and never reached the else-if where the KnockOnWood orb logic lived. Fix: extract the orb spawning block into a standalone if-check that runs after the drop routing, so it fires for any isNonWoodPartOfTree block regardless of whether it also has woodcutting XP. Fixes #5288 --- Changelog.txt | 1 + .../woodcutting/WoodcuttingManager.java | 23 +-- .../skills/woodcutting/WoodcuttingTest.java | 163 ++++++++++++++++++ 3 files changed, 177 insertions(+), 10 deletions(-) diff --git a/Changelog.txt b/Changelog.txt index 052a0df79..ff412be0a 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,4 +1,5 @@ Version 2.2.053 + Fixed KnockOnWood XP orbs never spawning on nether/warped tree cap blocks (nether_wart_block, warped_wart_block) during Tree Feller — those blocks grant woodcutting XP and are also tree-feller destructible, but the orb logic was unreachable for them due to an if/else-if routing bug (Codebase) Removed 27 dead JSON.* locale keys from all locale files — JSON.Rank, JSON.JWrapper.Header, JSON.JWrapper.Target.{Type,Block,Player}, JSON.Hover.{SuperAbility,Mystery2}, JSON.Notification.SuperAbility, JSON.Acrobatics.Roll.Interaction.Activated, and all JSON. skill-name keys were never loaded in Java source; skill names use .SkillName instead Version 2.2.052 diff --git a/src/main/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingManager.java b/src/main/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingManager.java index 588349625..50d732cd5 100644 --- a/src/main/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingManager.java +++ b/src/main/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingManager.java @@ -400,17 +400,20 @@ public class WoodcuttingManager extends SkillManager { player ); } + } - //Drop displaced non-woodcutting XP blocks - if (hasUnlockedSubskill(player, SubSkillType.WOODCUTTING_KNOCK_ON_WOOD)) { - if (RankUtils.hasReachedRank(2, player, - SubSkillType.WOODCUTTING_KNOCK_ON_WOOD)) { - if (mcMMO.p.getAdvancedConfig().isKnockOnWoodXPOrbEnabled()) { - if (ProbabilityUtil.isStaticSkillRNGSuccessful( - PrimarySkillType.WOODCUTTING, mmoPlayer, 10)) { - int randOrbCount = Math.max(1, Misc.getRandom().nextInt(100)); - Misc.spawnExperienceOrb(block.getLocation(), randOrbCount); - } + // KnockOnWood XP orbs apply to any non-log tree component, including blocks that + // also grant woodcutting XP (e.g. nether/warped wart blocks). Previously this was + // nested inside the else-if above, which prevented orbs from spawning on nether tree + // caps because they have woodcutting XP and never reached the else-if branch. + if (BlockUtils.isNonWoodPartOfTree(block) + && hasUnlockedSubskill(player, SubSkillType.WOODCUTTING_KNOCK_ON_WOOD)) { + if (RankUtils.hasReachedRank(2, player, SubSkillType.WOODCUTTING_KNOCK_ON_WOOD)) { + if (mcMMO.p.getAdvancedConfig().isKnockOnWoodXPOrbEnabled()) { + if (ProbabilityUtil.isStaticSkillRNGSuccessful( + PrimarySkillType.WOODCUTTING, mmoPlayer, 10)) { + int randOrbCount = Math.max(1, Misc.getRandom().nextInt(100)); + Misc.spawnExperienceOrb(block.getLocation(), randOrbCount); } } } diff --git a/src/test/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingTest.java b/src/test/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingTest.java index ddcf5857f..55df53e05 100644 --- a/src/test/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingTest.java +++ b/src/test/java/com/gmail/nossr50/skills/woodcutting/WoodcuttingTest.java @@ -17,21 +17,28 @@ import static org.mockito.Mockito.mockStatic; import com.gmail.nossr50.MMOTestEnvironment; import com.gmail.nossr50.api.exceptions.InvalidSkillException; import com.gmail.nossr50.config.experience.ExperienceConfig; +import com.gmail.nossr50.datatypes.player.McMMOPlayer; import com.gmail.nossr50.datatypes.skills.PrimarySkillType; import com.gmail.nossr50.datatypes.skills.SubSkillType; import com.gmail.nossr50.datatypes.skills.SuperAbilityType; import com.gmail.nossr50.mcMMO; import com.gmail.nossr50.util.BlockUtils; +import com.gmail.nossr50.util.EventUtils; import com.gmail.nossr50.util.MetadataConstants; +import com.gmail.nossr50.util.Misc; +import com.gmail.nossr50.util.random.ProbabilityUtil; import com.gmail.nossr50.util.skills.RankUtils; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Random; import java.util.Set; import java.util.concurrent.ThreadLocalRandom; import java.util.logging.Logger; +import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.block.Block; import org.bukkit.block.BlockFace; @@ -43,6 +50,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; import org.mockito.Mockito; +import static org.mockito.ArgumentMatchers.anyDouble; class WoodcuttingTest extends MMOTestEnvironment { private static final Logger logger = getLogger(WoodcuttingTest.class.getName()); @@ -317,4 +325,159 @@ class WoodcuttingTest extends MMOTestEnvironment { } } + @Nested + class KnockOnWoodXpOrbSpawning { + // Covers the bug where KnockOnWood XP orbs never spawned for nether tree cap + // blocks (nether_wart_block, warped_wart_block). Those blocks appear in both + // experience.yml (hasWoodcuttingXP=true) and the treeFellerDestructibleWhiteList + // (isNonWoodPartOfTree=true). The old code placed the XP orb logic inside the + // else-if branch for isNonWoodPartOfTree, so it was unreachable for any block + // that also had woodcutting XP. The fix extracts the orb logic into a standalone + // if-block that runs after the if/else-if drop routing. + + @Test + void netherWartBlock_spawnXpOrb_whenKnockOnWoodRank2Unlocked() { + // Given: player has KnockOnWood at rank 2 and XP orbs are enabled + mmoPlayer.modifySkill(PrimarySkillType.WOODCUTTING, 1000); + Mockito.when(RankUtils.hasUnlockedSubskill(any(Player.class), + eq(SubSkillType.WOODCUTTING_KNOCK_ON_WOOD))).thenReturn(true); + Mockito.when(RankUtils.hasReachedRank(eq(2), any(Player.class), + eq(SubSkillType.WOODCUTTING_KNOCK_ON_WOOD))).thenReturn(true); + Mockito.when(advancedConfig.isKnockOnWoodXPOrbEnabled()).thenReturn(true); + + // Given: block that satisfies BOTH hasWoodcuttingXP AND isNonWoodPartOfTree + // (matches nether_wart_block / warped_wart_block — the previously broken case) + final Block netherWartBlock = mock(Block.class, "netherWartBlock"); + final Location blockLocation = new Location(world, 10, 64, 10); + Mockito.when(netherWartBlock.getDrops(any())).thenReturn(Collections.emptyList()); + Mockito.when(netherWartBlock.getLocation()).thenReturn(blockLocation); + + // Stub processBonusDropCheck so it does not exercise unrelated paths + Mockito.doNothing().when(woodcuttingManager).processBonusDropCheck(any(Block.class)); + + try (MockedStatic mockedBlockUtils = mockStatic(BlockUtils.class); + MockedStatic localMockedEventUtils = mockStatic(EventUtils.class); + MockedStatic mockedProbabilityUtil = + mockStatic(ProbabilityUtil.class)) { + + mockedBlockUtils.when(() -> BlockUtils.hasWoodcuttingXP(any(Block.class))) + .thenReturn(true); + mockedBlockUtils.when(() -> BlockUtils.isNonWoodPartOfTree(any(Block.class))) + .thenReturn(true); + localMockedEventUtils.when(() -> EventUtils.simulateBlockBreak( + any(Block.class), any(Player.class), any())).thenReturn(true); + // Force the 10% RNG check to always succeed so the orb always spawns + mockedProbabilityUtil.when(() -> ProbabilityUtil.isStaticSkillRNGSuccessful( + any(PrimarySkillType.class), any(McMMOPlayer.class), anyDouble())) + .thenReturn(true); + + // Wire Misc.getRandom() to a predictable stub so the orb count is deterministic + final Random stubRandom = mock(Random.class); + Mockito.when(Misc.getRandom()).thenReturn(stubRandom); + Mockito.when(stubRandom.nextInt(anyInt())).thenReturn(50); + + // When: Tree Feller loot is processed for this block + invokeDropTreeFellerLootFromBlocks(Set.of(netherWartBlock)); + + // Then: an XP orb was spawned — this was the bug; this call was unreachable + // before the fix because the orb code was inside the else-if for isNonWoodPartOfTree + mockedMisc.verify(() -> Misc.spawnExperienceOrb(eq(blockLocation), anyInt())); + } + } + + @Test + void regularLeaf_spawnXpOrb_whenKnockOnWoodRank2Unlocked() { + // Given: a regular leaf block — only isNonWoodPartOfTree=true, no woodcutting XP. + // Regression check: existing leaf behaviour continues to work after the fix. + mmoPlayer.modifySkill(PrimarySkillType.WOODCUTTING, 1000); + Mockito.when(RankUtils.hasUnlockedSubskill(any(Player.class), + eq(SubSkillType.WOODCUTTING_KNOCK_ON_WOOD))).thenReturn(true); + Mockito.when(RankUtils.hasReachedRank(eq(2), any(Player.class), + eq(SubSkillType.WOODCUTTING_KNOCK_ON_WOOD))).thenReturn(true); + Mockito.when(advancedConfig.isKnockOnWoodXPOrbEnabled()).thenReturn(true); + + final Block leafBlock = mock(Block.class, "leafBlock"); + final Location blockLocation = new Location(world, 10, 64, 10); + Mockito.when(leafBlock.getDrops(any())).thenReturn(Collections.emptyList()); + Mockito.when(leafBlock.getLocation()).thenReturn(blockLocation); + + try (MockedStatic mockedBlockUtils = mockStatic(BlockUtils.class); + MockedStatic localMockedEventUtils = mockStatic(EventUtils.class); + MockedStatic mockedProbabilityUtil = + mockStatic(ProbabilityUtil.class)) { + + mockedBlockUtils.when(() -> BlockUtils.hasWoodcuttingXP(any(Block.class))) + .thenReturn(false); + mockedBlockUtils.when(() -> BlockUtils.isNonWoodPartOfTree(any(Block.class))) + .thenReturn(true); + localMockedEventUtils.when(() -> EventUtils.simulateBlockBreak( + any(Block.class), any(Player.class), any())).thenReturn(true); + mockedProbabilityUtil.when(() -> ProbabilityUtil.isStaticSkillRNGSuccessful( + any(PrimarySkillType.class), any(McMMOPlayer.class), anyDouble())) + .thenReturn(true); + + final Random stubRandom = mock(Random.class); + Mockito.when(Misc.getRandom()).thenReturn(stubRandom); + Mockito.when(stubRandom.nextInt(anyInt())).thenReturn(50); + + // When + invokeDropTreeFellerLootFromBlocks(Set.of(leafBlock)); + + // Then: orb still spawns for leaves (unchanged behaviour) + mockedMisc.verify(() -> Misc.spawnExperienceOrb(eq(blockLocation), anyInt())); + } + } + + @Test + void regularLog_doesNotSpawnXpOrb() { + // Given: a regular log block (hasWoodcuttingXP=true, isNonWoodPartOfTree=false). + // XP orbs must NEVER fire for plain logs — only for non-log tree components. + mmoPlayer.modifySkill(PrimarySkillType.WOODCUTTING, 1000); + Mockito.when(RankUtils.hasUnlockedSubskill(any(Player.class), + eq(SubSkillType.WOODCUTTING_KNOCK_ON_WOOD))).thenReturn(true); + Mockito.when(RankUtils.hasReachedRank(eq(2), any(Player.class), + eq(SubSkillType.WOODCUTTING_KNOCK_ON_WOOD))).thenReturn(true); + Mockito.when(advancedConfig.isKnockOnWoodXPOrbEnabled()).thenReturn(true); + + final Block logBlock = mock(Block.class, "logBlock"); + Mockito.when(logBlock.getDrops(any())).thenReturn(Collections.emptyList()); + + // Stub processBonusDropCheck so it does not exercise unrelated paths + Mockito.doNothing().when(woodcuttingManager).processBonusDropCheck(any(Block.class)); + + try (MockedStatic mockedBlockUtils = mockStatic(BlockUtils.class); + MockedStatic localMockedEventUtils = mockStatic(EventUtils.class); + MockedStatic mockedProbabilityUtil = + mockStatic(ProbabilityUtil.class)) { + + mockedBlockUtils.when(() -> BlockUtils.hasWoodcuttingXP(any(Block.class))) + .thenReturn(true); + mockedBlockUtils.when(() -> BlockUtils.isNonWoodPartOfTree(any(Block.class))) + .thenReturn(false); + localMockedEventUtils.when(() -> EventUtils.simulateBlockBreak( + any(Block.class), any(Player.class), any())).thenReturn(true); + mockedProbabilityUtil.when(() -> ProbabilityUtil.isStaticSkillRNGSuccessful( + any(PrimarySkillType.class), any(McMMOPlayer.class), anyDouble())) + .thenReturn(true); + + // When + invokeDropTreeFellerLootFromBlocks(Set.of(logBlock)); + + // Then: no XP orb spawned for a plain log + mockedMisc.verify(() -> Misc.spawnExperienceOrb(any(), anyInt()), never()); + } + } + + private void invokeDropTreeFellerLootFromBlocks(final Set blocks) { + try { + final Method method = WoodcuttingManager.class.getDeclaredMethod( + "dropTreeFellerLootFromBlocks", Set.class); + method.setAccessible(true); + method.invoke(woodcuttingManager, blocks); + } catch (Exception exception) { + throw new RuntimeException(exception); + } + } + } + }