diff --git a/Changelog.txt b/Changelog.txt index 6457c279a..a2b6e2591 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,3 +1,10 @@ +Version 2.2.018 + Fixed a probability bug where certain skills would max out in chance to succeed well before they were supposed to (such as Dodge) + (Codebase) Added more unit tests for Probability and RNG + + NOTES: + This probability bug was a big oopsie and showed a gap in unit test coverage, I've added that coverage and a bug like this in theory shouldn't happen again. + Version 2.2.017 Fixed a bug with default Mace permissions (thanks SrBedrock) Fixed Blast Mining being almost completely broken diff --git a/src/main/java/com/gmail/nossr50/commands/skills/AcrobaticsCommand.java b/src/main/java/com/gmail/nossr50/commands/skills/AcrobaticsCommand.java index d04d3d4d4..94751a95b 100644 --- a/src/main/java/com/gmail/nossr50/commands/skills/AcrobaticsCommand.java +++ b/src/main/java/com/gmail/nossr50/commands/skills/AcrobaticsCommand.java @@ -59,9 +59,6 @@ public class AcrobaticsCommand extends SkillCommand { messages.add(getStatMessage(SubSkillType.ACROBATICS_ROLL, rollStrings[0]) + (isLucky ? LocaleLoader.getString("Perks.Lucky.Bonus", rollStrings[1]) : "")); - - /*messages.add(getStatMessage(true, false, SubSkillType.ACROBATICS_ROLL, String.valueOf(graceChance)) - + (isLucky ? LocaleLoader.getString("Perks.Lucky.Bonus", String.valueOf(graceChanceLucky)) : ""));*/ } } diff --git a/src/main/java/com/gmail/nossr50/commands/skills/SkillCommand.java b/src/main/java/com/gmail/nossr50/commands/skills/SkillCommand.java index 4e20a761b..e3f9b95d0 100644 --- a/src/main/java/com/gmail/nossr50/commands/skills/SkillCommand.java +++ b/src/main/java/com/gmail/nossr50/commands/skills/SkillCommand.java @@ -30,6 +30,8 @@ import java.util.List; import java.util.Locale; public abstract class SkillCommand implements TabExecutor { + public static final String ABILITY_GENERIC_TEMPLATE_CUSTOM = "Ability.Generic.Template.Custom"; + public static final String ABILITY_GENERIC_TEMPLATE = "Ability.Generic.Template"; protected PrimarySkillType skill; protected DecimalFormat percent = new DecimalFormat("##0.00%"); @@ -163,8 +165,6 @@ public abstract class SkillCommand implements TabExecutor { /* * CHILD SKILLS */ - - var parents = mcMMO.p.getSkillTools().getChildSkillParents(skill); //TODO: Add JSON here @@ -186,28 +186,7 @@ public abstract class SkillCommand implements TabExecutor { player.sendMessage(LocaleLoader.getString("Commands.XPGain.Overhaul", LocaleLoader.getString("Commands.XPGain.Child"))); player.sendMessage(LocaleLoader.getString("Effects.Child.Overhaul", skillValue, parentMessage.toString())); - //LEVEL - //player.sendMessage(LocaleLoader.getString("Effects.Child.Overhaul", skillValue, skillValue)); - } - /* - if (!SkillTools.isChildSkill(skill)) { - player.sendMessage(LocaleLoader.getString("Skills.Header", skillName)); - player.sendMessage(LocaleLoader.getString("Commands.XPGain", LocaleLoader.getString("Commands.XPGain." + StringUtils.getCapitalized(skill.toString())))); - player.sendMessage(LocaleLoader.getString("Effects.Level", skillValue, mcMMOPlayer.getSkillXpLevel(skill), mcMMOPlayer.getXpToLevel(skill))); - } else { - player.sendMessage(LocaleLoader.getString("Skills.Header", skillName + " " + LocaleLoader.getString("Skills.Child"))); - player.sendMessage(LocaleLoader.getString("Commands.XPGain", LocaleLoader.getString("Commands.XPGain.Child"))); - player.sendMessage(LocaleLoader.getString("Effects.Child", skillValue)); - - player.sendMessage(LocaleLoader.getString("Skills.Header", LocaleLoader.getString("Skills.Parents"))); - Set parents = FamilyTree.getParents(skill); - - for (PrimarySkillType parent : parents) { - player.sendMessage(parent.getName() + " - " + LocaleLoader.getString("Effects.Level", mcMMOPlayer.getSkillLevel(parent), mcMMOPlayer.getSkillXpLevel(parent), mcMMOPlayer.getXpToLevel(parent))); - } - } - */ } @Override @@ -222,10 +201,6 @@ public abstract class SkillCommand implements TabExecutor { return Math.min((int) skillValue, maxLevel) / rankChangeLevel; } -// protected String[] getAbilityDisplayValues(SkillActivationType skillActivationType, Player player, SubSkillType subSkill) { -// return RandomChanceUtil.calculateAbilityDisplayValues(skillActivationType, player, subSkill); -// } - protected String[] calculateLengthDisplayValues(Player player, float skillValue) { int maxLength = mcMMO.p.getSkillTools().getSuperAbilityMaxLength(mcMMO.p.getSkillTools().getSuperAbility(skill)); int abilityLengthVar = mcMMO.p.getAdvancedConfig().getAbilityLength(); @@ -252,14 +227,19 @@ public abstract class SkillCommand implements TabExecutor { return getStatMessage(false, false, subSkillType, vars); } - protected String getStatMessage(boolean isExtra, boolean isCustom, SubSkillType subSkillType, String... vars) { - String templateKey = isCustom ? "Ability.Generic.Template.Custom" : "Ability.Generic.Template"; - String statDescriptionKey = !isExtra ? subSkillType.getLocaleKeyStatDescription() : subSkillType.getLocaleKeyStatExtraDescription(); + protected String getStatMessage(boolean isExtra, boolean isCustom, + @NotNull SubSkillType subSkillType, String... vars) { + final String templateKey = isCustom ? ABILITY_GENERIC_TEMPLATE_CUSTOM : ABILITY_GENERIC_TEMPLATE; + final String statDescriptionKey = !isExtra + ? subSkillType.getLocaleKeyStatDescription() + : subSkillType.getLocaleKeyStatExtraDescription(); - if (isCustom) + if (isCustom) { return LocaleLoader.getString(templateKey, LocaleLoader.getString(statDescriptionKey, vars)); - else { - String[] mergedList = NotificationManager.addItemToFirstPositionOfArray(LocaleLoader.getString(statDescriptionKey), vars); + } else { + final String[] mergedList + = NotificationManager.addItemToFirstPositionOfArray( + LocaleLoader.getString(statDescriptionKey), vars); return LocaleLoader.getString(templateKey, mergedList); } } @@ -276,8 +256,6 @@ public abstract class SkillCommand implements TabExecutor { protected abstract void permissionsCheck(Player player); - //protected abstract List effectsDisplay(); - protected abstract List statsDisplay(Player player, float skillValue, boolean hasEndurance, boolean isLucky); protected abstract List getTextComponents(Player player); diff --git a/src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java b/src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java index 715673496..47059023d 100644 --- a/src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java +++ b/src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java @@ -41,7 +41,7 @@ public class ProbabilityUtil { public static double chanceOfSuccessPercentage(@Nullable McMMOPlayer mmoPlayer, @NotNull SubSkillType subSkillType, boolean isLucky) { - Probability probability = getSubSkillProbability(subSkillType, mmoPlayer); + final Probability probability = getSubSkillProbability(subSkillType, mmoPlayer); //Probability values are on a 0-1 scale and need to be "transformed" into a 1-100 scale double percentageValue = probability.getValue(); //Doesn't need to be scaled @@ -437,7 +437,7 @@ public class ProbabilityUtil { return Probability.ofPercent(ceiling); } - double odds = (skillLevel / maxBonusLevel) * 100D; + double odds = ((skillLevel / maxBonusLevel) * ceiling); // make sure the odds aren't lower or higher than the floor or ceiling return Probability.ofPercent(Math.min(Math.max(floor, odds), ceiling)); diff --git a/src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java b/src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java index 44420a093..905a46b4d 100644 --- a/src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java +++ b/src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java @@ -12,6 +12,8 @@ import org.junit.jupiter.params.provider.MethodSource; import java.util.logging.Logger; import java.util.stream.Stream; +import static com.gmail.nossr50.datatypes.skills.PrimarySkillType.ACROBATICS; +import static com.gmail.nossr50.datatypes.skills.PrimarySkillType.MINING; import static com.gmail.nossr50.datatypes.skills.SubSkillType.*; import static com.gmail.nossr50.util.random.ProbabilityTestUtils.assertProbabilityExpectations; import static com.gmail.nossr50.util.random.ProbabilityUtil.calculateCurrentSkillProbability; @@ -68,9 +70,106 @@ class ProbabilityUtilTest extends MMOTestEnvironment { assertProbabilityExpectations(20, probability); } - @Test - public void calculateCurrentSkillProbabilityShouldBeTwenty() { - final Probability probability = calculateCurrentSkillProbability(1000, 0, 20, 1000); - assertEquals(0.2D, probability.getValue()); + private static Stream provideSkillProbabilityTestData() { + return Stream.of( + // skillLevel, floor, ceiling, maxBonusLevel, expectedValue + + // 20% chance at skill level 1000 + Arguments.of(1000, 0, 20, 1000, 0.2), + // 10% chance at skill level 500 + Arguments.of(500, 0, 20, 1000, 0.1), + // 5% chance at skill level 250 + Arguments.of(250, 0, 20, 1000, 0.05), + // 0% chance at skill level 0 + Arguments.of(0, 0, 20, 1000, 0.0), + // 0% chance at skill level 1000 + Arguments.of(1000, 0, 0, 1000, 0.0), + // 1% chance at skill level 1000 + Arguments.of(1000, 0, 1, 1000, 0.01) + ); } + + @ParameterizedTest + @MethodSource("provideSkillProbabilityTestData") + void testCalculateCurrentSkillProbability(double skillLevel, double floor, double ceiling, double maxBonusLevel, + double expectedValue) { + // When + final Probability probability = calculateCurrentSkillProbability(skillLevel, floor, ceiling, maxBonusLevel); + + // Then + assertEquals(expectedValue, probability.getValue()); + } + + @Test + public void getRNGDisplayValuesShouldReturn10PercentForDodge() { + // Given + when(advancedConfig.getMaximumProbability(ACROBATICS_DODGE)).thenReturn(20D); + when(advancedConfig.getMaxBonusLevel(ACROBATICS_DODGE)).thenReturn(1000); + mmoPlayer.modifySkill(ACROBATICS, 500); + + // When & Then + final String[] rngDisplayValues = ProbabilityUtil.getRNGDisplayValues(mmoPlayer, ACROBATICS_DODGE); + assertEquals("10.00%", rngDisplayValues[0]); + } + + @Test + public void getRNGDisplayValuesShouldReturn20PercentForDodge() { + // Given + when(advancedConfig.getMaximumProbability(ACROBATICS_DODGE)).thenReturn(20D); + when(advancedConfig.getMaxBonusLevel(ACROBATICS_DODGE)).thenReturn(1000); + mmoPlayer.modifySkill(ACROBATICS, 1000); + + // When & then + final String[] rngDisplayValues = ProbabilityUtil.getRNGDisplayValues(mmoPlayer, ACROBATICS_DODGE); + assertEquals("20.00%", rngDisplayValues[0]); + } + + @Test + public void getRNGDisplayValuesShouldReturn0PercentForDodge() { + // Given + when(advancedConfig.getMaximumProbability(ACROBATICS_DODGE)).thenReturn(20D); + when(advancedConfig.getMaxBonusLevel(ACROBATICS_DODGE)).thenReturn(1000); + mmoPlayer.modifySkill(ACROBATICS, 0); + + // When & then + final String[] rngDisplayValues = ProbabilityUtil.getRNGDisplayValues(mmoPlayer, ACROBATICS_DODGE); + assertEquals("0.00%", rngDisplayValues[0]); + } + + @Test + public void getRNGDisplayValuesShouldReturn10PercentForDoubleDrops() { + // Given + when(advancedConfig.getMaximumProbability(MINING_DOUBLE_DROPS)).thenReturn(100D); + when(advancedConfig.getMaxBonusLevel(MINING_DOUBLE_DROPS)).thenReturn(1000); + mmoPlayer.modifySkill(MINING, 100); + + // When & Then + final String[] rngDisplayValues = ProbabilityUtil.getRNGDisplayValues(mmoPlayer, MINING_DOUBLE_DROPS); + assertEquals("10.00%", rngDisplayValues[0]); + } + + @Test + public void getRNGDisplayValuesShouldReturn50PercentForDoubleDrops() { + // Given + when(advancedConfig.getMaximumProbability(MINING_DOUBLE_DROPS)).thenReturn(100D); + when(advancedConfig.getMaxBonusLevel(MINING_DOUBLE_DROPS)).thenReturn(1000); + mmoPlayer.modifySkill(MINING, 500); + + // When & Then + final String[] rngDisplayValues = ProbabilityUtil.getRNGDisplayValues(mmoPlayer, MINING_DOUBLE_DROPS); + assertEquals("50.00%", rngDisplayValues[0]); + } + + @Test + public void getRNGDisplayValuesShouldReturn100PercentForDoubleDrops() { + // Given + when(advancedConfig.getMaximumProbability(MINING_DOUBLE_DROPS)).thenReturn(100D); + when(advancedConfig.getMaxBonusLevel(MINING_DOUBLE_DROPS)).thenReturn(1000); + mmoPlayer.modifySkill(MINING, 1000); + + // When & Then + final String[] rngDisplayValues = ProbabilityUtil.getRNGDisplayValues(mmoPlayer, MINING_DOUBLE_DROPS); + assertEquals("100.00%", rngDisplayValues[0]); + } + }