From b6e512b09e415605b40eb2d24fababc33a0bdcc8 Mon Sep 17 00:00:00 2001 From: nossr50 Date: Sat, 6 Apr 2024 12:44:56 -0700 Subject: [PATCH] fix probability being unbounded --- Changelog.txt | 7 ++ .../nossr50/database/SQLDatabaseManager.java | 6 +- .../nossr50/util/random/Probability.java | 48 ++++++++-- .../nossr50/util/random/ProbabilityImpl.java | 33 +++---- .../nossr50/util/random/ProbabilityUtil.java | 88 ++++++++++++++----- .../nossr50/util/random/ProbabilityTest.java | 26 +++--- .../util/random/ProbabilityTestUtils.java | 22 +++++ .../util/random/ProbabilityUtilTest.java | 55 +++++++----- 8 files changed, 194 insertions(+), 91 deletions(-) create mode 100644 src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java diff --git a/Changelog.txt b/Changelog.txt index fa5a2e9c9..961605906 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,3 +1,10 @@ +Version 2.2.005 + Fixed a bug where certain skills such as Dodge/Arrow Deflect had no skill cap and would continue improving forever + Reduced messages on startup for SQL DB + (API) Constructor for ProbabilityImpl now takes a raw value between 0 and 1 instead of an inflated percentage + (API) Added some convenience methods to Probability, and ProbabilityUtil classes + (Codebase) Added more unit tests revolving around Probability/RNG + Version 2.2.004 Fixed bug where values from Experience_Formula.Skill_Multiplier were not functioning diff --git a/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java b/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java index 3b5954f62..892a43721 100644 --- a/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java +++ b/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java @@ -1037,13 +1037,13 @@ public final class SQLDatabaseManager implements DatabaseManager { try (Connection connection = getConnection(PoolIdentifier.MISC)) { if (!columnExists(connection, mcMMO.p.getGeneralConfig().getMySQLDatabaseName(), tablePrefix+tableName, columnName)) { try (Statement createStatement = connection.createStatement()) { - logger.info("[SQLDB Check] Adding column '" + columnName + "' to table '" + tablePrefix + tableName + "'..."); + // logger.info("[SQLDB Check] Adding column '" + columnName + "' to table '" + tablePrefix + tableName + "'..."); String startingLevel = "'" + mcMMO.p.getAdvancedConfig().getStartingLevel() + "'"; createStatement.executeUpdate("ALTER TABLE `" + tablePrefix + tableName + "` " + "ADD COLUMN `" + columnName + "` int(" + columnSize + ") unsigned NOT NULL DEFAULT " + startingLevel); } } else { - logger.info("[SQLDB Check] Column '" + columnName + "' already exists in table '" + tablePrefix + tableName + "', looks good!"); + // logger.info("[SQLDB Check] Column '" + columnName + "' already exists in table '" + tablePrefix + tableName + "', looks good!"); } } catch (SQLException e) { e.printStackTrace(); // Consider more robust logging @@ -1052,7 +1052,7 @@ public final class SQLDatabaseManager implements DatabaseManager { } private boolean columnExists(Connection connection, String database, String tableName, String columnName) throws SQLException { - logger.info("[SQLDB Check] Checking if column '" + columnName + "' exists in table '" + tableName + "'"); + // logger.info("[SQLDB Check] Checking if column '" + columnName + "' exists in table '" + tableName + "'"); try (Statement createStatement = connection.createStatement()) { String sql = "SELECT `COLUMN_NAME`\n" + "FROM `INFORMATION_SCHEMA`.`COLUMNS`\n" + diff --git a/src/main/java/com/gmail/nossr50/util/random/Probability.java b/src/main/java/com/gmail/nossr50/util/random/Probability.java index 4bb9dacc9..3b9225b97 100644 --- a/src/main/java/com/gmail/nossr50/util/random/Probability.java +++ b/src/main/java/com/gmail/nossr50/util/random/Probability.java @@ -1,10 +1,21 @@ package com.gmail.nossr50.util.random; +import com.gmail.nossr50.api.exceptions.ValueOutOfBoundsException; import org.jetbrains.annotations.NotNull; import java.util.concurrent.ThreadLocalRandom; public interface Probability { + /** + * A Probability that always fails. + */ + Probability ALWAYS_FAILS = () -> 0; + + /** + * A Probability that always succeeds. + */ + Probability ALWAYS_SUCCEEDS = () -> 1; + /** * The value of this Probability * Should return a result between 0 and 1 (inclusive) @@ -17,19 +28,40 @@ public interface Probability { double getValue(); /** - * Create a new Probability with the given value - * A value of 100 would represent 100% chance of success - * A value of 50 would represent 50% chance of success - * A value of 0 would represent 0% chance of success - * A value of 1 would represent 1% chance of success - * A value of 0.5 would represent 0.5% chance of success - * A value of 0.01 would represent 0.01% chance of success + * Create a new Probability of a percentage. + * This method takes a percentage and creates a Probability of equivalent odds. + * + * A value of 100 would represent 100% chance of success, + * A value of 50 would represent 50% chance of success, + * A value of 0 would represent 0% chance of success, + * A value of 1 would represent 1% chance of success, + * A value of 0.5 would represent 0.5% chance of success, + * A value of 0.01 would represent 0.01% chance of success. * * @param percentage the value of the probability * @return a new Probability with the given value */ static @NotNull Probability ofPercent(double percentage) { - return new ProbabilityImpl(percentage); + if (percentage < 0) { + throw new ValueOutOfBoundsException("Value should never be negative for Probability! This suggests a coding mistake, contact the devs!"); + } + + // Convert to a 0-1 floating point representation + double probabilityValue = percentage / 100.0D; + return new ProbabilityImpl(probabilityValue); + } + + /** + * Create a new Probability of a value. + * This method takes a value between 0 and 1 and creates a Probability of equivalent odds. + * A value of 1 or greater represents something that will always succeed. + * A value of around 0.5 represents something that succeeds around half the time. + * A value of 0 represents something that will always fail. + * @param value the value of the probability + * @return a new Probability with the given value + */ + static @NotNull Probability ofValue(double value) { + return new ProbabilityImpl(value); } /** diff --git a/src/main/java/com/gmail/nossr50/util/random/ProbabilityImpl.java b/src/main/java/com/gmail/nossr50/util/random/ProbabilityImpl.java index e240f2f07..b62110567 100644 --- a/src/main/java/com/gmail/nossr50/util/random/ProbabilityImpl.java +++ b/src/main/java/com/gmail/nossr50/util/random/ProbabilityImpl.java @@ -8,31 +8,22 @@ public class ProbabilityImpl implements Probability { private final double probabilityValue; /** - * Create a probability with a static value + * Create a probability from a static value. + * A value of 0 represents a 0% chance of success, + * A value of 1 represents a 100% chance of success. + * A value of 0.5 represents a 50% chance of success. + * A value of 0.01 represents a 1% chance of success. + * And so on. * - * @param percentage the percentage value of the probability + * @param value the value of the probability between 0 and 100 */ - ProbabilityImpl(double percentage) throws ValueOutOfBoundsException { - if (percentage < 0) { - throw new ValueOutOfBoundsException("Value should never be negative for Probability! This suggests a coding mistake, contact the devs!"); + public ProbabilityImpl(double value) throws ValueOutOfBoundsException { + if (value < 0) { + throw new ValueOutOfBoundsException("Value should never be negative for Probability!" + + " This suggests a coding mistake, contact the devs!"); } - // Convert to a 0-1 floating point representation - probabilityValue = percentage / 100.0D; - } - - ProbabilityImpl(double xPos, double xCeiling, double probabilityCeiling) throws ValueOutOfBoundsException { - if(probabilityCeiling > 100) { - throw new ValueOutOfBoundsException("Probability Ceiling should never be above 100!"); - } else if (probabilityCeiling < 0) { - throw new ValueOutOfBoundsException("Probability Ceiling should never be below 0!"); - } - - //Get the percent success, this will be from 0-100 - double probabilityPercent = (probabilityCeiling * (xPos / xCeiling)); - - //Convert to a 0-1 floating point representation - this.probabilityValue = probabilityPercent / 100.0D; + probabilityValue = value; } @Override 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 bc757506c..4836efedc 100644 --- a/src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java +++ b/src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java @@ -79,24 +79,24 @@ public class ProbabilityUtil { switch (getProbabilityType(subSkillType)) { case DYNAMIC_CONFIGURABLE: double probabilityCeiling; - double xCeiling; - double xPos; + double skillLevel; + double maxBonusLevel; // If a skill level is equal to the cap, it has the full probability if (player != null) { McMMOPlayer mmoPlayer = UserManager.getPlayer(player); if (mmoPlayer == null) { return Probability.ofPercent(0); } - xPos = mmoPlayer.getSkillLevel(subSkillType.getParentSkill()); + skillLevel = mmoPlayer.getSkillLevel(subSkillType.getParentSkill()); } else { - xPos = 0; + skillLevel = 0; } //Probability ceiling is configurable in this type probabilityCeiling = mcMMO.p.getAdvancedConfig().getMaximumProbability(subSkillType); //The xCeiling is configurable in this type - xCeiling = mcMMO.p.getAdvancedConfig().getMaxBonusLevel(subSkillType); - return new ProbabilityImpl(xPos, xCeiling, probabilityCeiling); + maxBonusLevel = mcMMO.p.getAdvancedConfig().getMaxBonusLevel(subSkillType); + return calculateCurrentSkillProbability(skillLevel, 0, probabilityCeiling, maxBonusLevel); case STATIC_CONFIGURABLE: try { return getStaticRandomChance(subSkillType); @@ -127,22 +127,7 @@ public class ProbabilityUtil { * @return true if the Skill RNG succeeds, false if it fails */ public static boolean isSkillRNGSuccessful(@NotNull SubSkillType subSkillType, @NotNull Player player) { - //Process probability - Probability probability = getSubSkillProbability(subSkillType, player); - - //Send out event - SubSkillEvent subSkillEvent = EventUtils.callSubSkillEvent(player, subSkillType); - - if(subSkillEvent.isCancelled()) { - return false; //Event got cancelled so this doesn't succeed - } - - //Result modifier - double resultModifier = subSkillEvent.getResultModifier(); - - //Mutate probability - if(resultModifier != 1.0D) - probability = Probability.ofPercent(probability.getValue() * resultModifier); + final Probability probability = getSkillProbability(subSkillType, player); //Luck boolean isLucky = Permissions.lucky(player, subSkillType.getParentSkill()); @@ -154,12 +139,42 @@ public class ProbabilityUtil { } } + /** + * Returns the {@link Probability} for a specific {@link SubSkillType} for a specific {@link Player}. + * This does not take into account perks such as lucky for the player. + * This is affected by other plugins who can listen to the {@link SubSkillEvent} and cancel it or mutate it. + * + * @param subSkillType the target subskill + * @param player the target player + * @return the probability for this skill + */ + public static Probability getSkillProbability(@NotNull SubSkillType subSkillType, @NotNull Player player) { + //Process probability + Probability probability = getSubSkillProbability(subSkillType, player); + + //Send out event + SubSkillEvent subSkillEvent = EventUtils.callSubSkillEvent(player, subSkillType); + + if(subSkillEvent.isCancelled()) { + return Probability.ALWAYS_FAILS; + } + + //Result modifier + double resultModifier = subSkillEvent.getResultModifier(); + + //Mutate probability + if(resultModifier != 1.0D) + probability = Probability.ofPercent(probability.getValue() * resultModifier); + + return probability; + } + /** * This is one of several Skill RNG check methods * This helper method is specific to static value RNG, which can be influenced by a player's Luck * * @param primarySkillType the related primary skill - * @param player the target player, can be null (null players have the worst odds) + * @param player the target player can be null (null players have the worst odds) * @param probabilityPercentage the probability of this player succeeding in "percentage" format (0-100 inclusive) * @return true if the RNG succeeds, false if it fails */ @@ -223,4 +238,31 @@ public class ProbabilityUtil { return new String[]{percent.format(firstValue), percent.format(secondValue)}; } + + /** + * Helper function to calculate what probability a given skill has at a certain level + * @param skillLevel the skill level currently between the floor and the ceiling + * @param floor the minimum odds this skill can have + * @param ceiling the maximum odds this skill can have + * @param maxBonusLevel the maximum level this skill can have to reach the ceiling + * + * @return the probability of success for this skill at this level + */ + public static Probability calculateCurrentSkillProbability(double skillLevel, double floor, + double ceiling, double maxBonusLevel) { + // The odds of success are between the value of the floor and the value of the ceiling. + // If the skill has a maxBonusLevel of 500 on this skill, then at skill level 500 you would have the full odds, + // at skill level 250 it would be half odds. + + if (skillLevel >= maxBonusLevel || maxBonusLevel <= 0) { + // Avoid divide by zero bugs + // Max benefit has been reached, should always succeed + return Probability.ofPercent(ceiling); + } + + double odds = (skillLevel / maxBonusLevel) * 100D; + + // 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/ProbabilityTest.java b/src/test/java/com/gmail/nossr50/util/random/ProbabilityTest.java index a2b6cc416..e114313ba 100644 --- a/src/test/java/com/gmail/nossr50/util/random/ProbabilityTest.java +++ b/src/test/java/com/gmail/nossr50/util/random/ProbabilityTest.java @@ -14,19 +14,19 @@ class ProbabilityTest { private static Stream provideProbabilitiesForWithinExpectations() { return Stream.of( // static probability, % of time for success - Arguments.of(new ProbabilityImpl(5), 5), - Arguments.of(new ProbabilityImpl(10), 10), - Arguments.of(new ProbabilityImpl(15), 15), - Arguments.of(new ProbabilityImpl(20), 20), - Arguments.of(new ProbabilityImpl(25), 25), - Arguments.of(new ProbabilityImpl(50), 50), - Arguments.of(new ProbabilityImpl(75), 75), - Arguments.of(new ProbabilityImpl(90), 90), - Arguments.of(new ProbabilityImpl(99.9), 99.9), - Arguments.of(new ProbabilityImpl(0.05), 0.05), - Arguments.of(new ProbabilityImpl(0.1), 0.1), - Arguments.of(new ProbabilityImpl(500), 100), - Arguments.of(new ProbabilityImpl(1000), 100) + Arguments.of(new ProbabilityImpl(.05), 5), + Arguments.of(new ProbabilityImpl(.10), 10), + Arguments.of(new ProbabilityImpl(.15), 15), + Arguments.of(new ProbabilityImpl(.20), 20), + Arguments.of(new ProbabilityImpl(.25), 25), + Arguments.of(new ProbabilityImpl(.50), 50), + Arguments.of(new ProbabilityImpl(.75), 75), + Arguments.of(new ProbabilityImpl(.90), 90), + Arguments.of(new ProbabilityImpl(.999), 99.9), + Arguments.of(new ProbabilityImpl(0.0005), 0.05), + Arguments.of(new ProbabilityImpl(0.001), 0.1), + Arguments.of(new ProbabilityImpl(50.0), 100), + Arguments.of(new ProbabilityImpl(100.0), 100) ); } diff --git a/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java b/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java new file mode 100644 index 000000000..e97119be6 --- /dev/null +++ b/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java @@ -0,0 +1,22 @@ +package com.gmail.nossr50.util.random; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class ProbabilityTestUtils { + public static void assertProbabilityExpectations(double expectedWinPercent, Probability probability) { + double iterations = 2.0e7; //20 million + double winCount = 0; + for (int i = 0; i < iterations; i++) { + if(probability.evaluate()) { + winCount++; + } + } + + double successPercent = (winCount / iterations) * 100; + System.out.println("Wins: " + winCount); + System.out.println("Fails: " + (iterations - winCount)); + System.out.println("Percentage succeeded: " + successPercent + ", Expected: " + expectedWinPercent); + assertEquals(expectedWinPercent, successPercent, 0.025D); + System.out.println("Variance is within tolerance levels!"); + } +} 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 35f134da5..4994f8053 100644 --- a/src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java +++ b/src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java @@ -1,39 +1,44 @@ package com.gmail.nossr50.util.random; -import com.gmail.nossr50.config.AdvancedConfig; +import com.gmail.nossr50.MMOTestEnvironment; import com.gmail.nossr50.datatypes.skills.SubSkillType; -import com.gmail.nossr50.mcMMO; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import java.util.logging.Logger; import java.util.stream.Stream; 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; +import static java.util.logging.Logger.getLogger; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -class ProbabilityUtilTest { - mcMMO mmoInstance; - AdvancedConfig advancedConfig; +class ProbabilityUtilTest extends MMOTestEnvironment { + private static final Logger logger = getLogger(ProbabilityUtilTest.class.getName()); final static double impactChance = 11D; final static double greaterImpactChance = 0.007D; final static double fastFoodChance = 45.5D; @BeforeEach - public void setupMocks() throws NoSuchFieldException, IllegalAccessException { - this.mmoInstance = mock(mcMMO.class); - mcMMO.class.getField("p").set(null, mmoInstance); - this.advancedConfig = mock(AdvancedConfig.class); - when(mmoInstance.getAdvancedConfig()).thenReturn(advancedConfig); + public void setupMocks() { + mockBaseEnvironment(logger); when(advancedConfig.getImpactChance()).thenReturn(impactChance); when(advancedConfig.getGreaterImpactChance()).thenReturn(greaterImpactChance); when(advancedConfig.getFastFoodChance()).thenReturn(fastFoodChance); } + @AfterEach + public void tearDown() { + cleanupBaseEnvironment(); + } + private static Stream staticChanceSkills() { return Stream.of( // static probability, % of time for success @@ -45,22 +50,26 @@ class ProbabilityUtilTest { @ParameterizedTest @MethodSource("staticChanceSkills") - void testStaticChanceSkills(SubSkillType subSkillType, double expectedWinPercent) throws InvalidStaticChance { + void staticChanceSkillsShouldSucceedAsExpected(SubSkillType subSkillType, double expectedWinPercent) + throws InvalidStaticChance { Probability staticRandomChance = ProbabilityUtil.getStaticRandomChance(subSkillType); assertProbabilityExpectations(expectedWinPercent, staticRandomChance); } - private static void assertProbabilityExpectations(double expectedWinPercent, Probability probability) { - double iterations = 2.0e7; - double winCount = 0; - for (int i = 0; i < iterations; i++) { - if(probability.evaluate()) { - winCount++; - } - } + @Test + public void isSkillRNGSuccessfulShouldBehaveAsExpected() { + // Given + when(advancedConfig.getMaximumProbability(UNARMED_ARROW_DEFLECT)).thenReturn(20D); + when(advancedConfig.getMaxBonusLevel(UNARMED_ARROW_DEFLECT)).thenReturn(0); - double successPercent = (winCount / iterations) * 100; - System.out.println(successPercent + ", " + expectedWinPercent); - assertEquals(expectedWinPercent, successPercent, 0.05D); + final Probability probability = ProbabilityUtil.getSkillProbability(UNARMED_ARROW_DEFLECT, player); + assertEquals(0.2D, probability.getValue()); + assertProbabilityExpectations(20, probability); + } + + @Test + public void calculateCurrentSkillProbabilityShouldBeTwenty() { + final Probability probability = calculateCurrentSkillProbability(1000, 0, 20, 1000); + assertEquals(0.2D, probability.getValue()); } }