Probability factory should live within the interface

src/main/java/com/gmail/nossr50/commands/skills/FishingCommand.java

@@ -10,7 +10,6 @@ import com.gmail.nossr50.skills.fishing.FishingManager;
 import com.gmail.nossr50.util.Permissions;
 import com.gmail.nossr50.util.player.UserManager;
 import com.gmail.nossr50.util.random.Probability;
-import com.gmail.nossr50.util.random.ProbabilityFactory;
 import com.gmail.nossr50.util.skills.RankUtils;
 import com.gmail.nossr50.util.skills.SkillUtils;
 import com.gmail.nossr50.util.text.StringUtils;
@@ -82,7 +81,7 @@ public class FishingCommand extends SkillCommand {
 
         // FISHING_SHAKE
         if (canShake) {
-            Probability shakeProbability = ProbabilityFactory.ofPercentageValue(fishingManager.getShakeChance());
+            Probability shakeProbability = Probability.ofPercentageValue(fishingManager.getShakeChance());
             String[] shakeStrings = SkillUtils.getRNGDisplayValues(shakeProbability);
             shakeChance = shakeStrings[0];
             shakeChanceLucky = shakeStrings[1];

src/main/java/com/gmail/nossr50/datatypes/skills/subskills/acrobatics/Roll.java

@@ -136,9 +136,8 @@ public class Roll extends AcrobaticsSubSkill {
         /*
          * Graceful is double the odds of a normal roll
          */
-        //TODO: Yeah I know, ...I'm tired I'll clean it up later
         Probability probability = getRollProbability(player);
-        Probability gracefulProbability = new ProbabilityImpl(probability.getValue() * 2);
+        Probability gracefulProbability = Probability.ofPercentageValue(probability.getValue() * 2);
         String[] gracefulRollStrings = SkillUtils.getRNGDisplayValues(gracefulProbability);
         gracefulRollChance = gracefulRollStrings[0];
         gracefulRollChanceLucky = gracefulRollStrings[1];
@@ -249,7 +248,7 @@ public class Roll extends AcrobaticsSubSkill {
         double modifiedDamage = calculateModifiedRollDamage(damage, mcMMO.p.getAdvancedConfig().getRollDamageThreshold() * 2);
 
         double gracefulOdds = SkillUtils.getSubSkillProbability(subSkillType, player).getValue() * 2;
-        Probability gracefulProbability = new ProbabilityImpl(gracefulOdds);
+        Probability gracefulProbability = Probability.ofPercentageValue(gracefulOdds);
 
         if (!isFatal(player, modifiedDamage)
                 //TODO: Graceful isn't sending out an event

src/main/java/com/gmail/nossr50/datatypes/treasure/Treasure.java

@@ -1,8 +1,6 @@
 package com.gmail.nossr50.datatypes.treasure;
 
-import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.util.random.Probability;
-import com.gmail.nossr50.util.random.ProbabilityFactory;
 import com.gmail.nossr50.util.random.ProbabilityImpl;
 import com.google.common.base.Objects;
 import org.bukkit.inventory.ItemStack;
@@ -19,7 +17,7 @@ public abstract class Treasure {
         this.drop = drop;
         this.xp = xp;
         this.dropChance = dropChance;
-        this.dropProbability = new ProbabilityImpl(ProbabilityFactory.probabilityFromPercent(dropChance));
+        this.dropProbability = Probability.ofPercentageValue(dropChance / 100);
         this.dropLevel = dropLevel;
     }
 
@@ -49,7 +47,7 @@ public abstract class Treasure {
 
     public void setDropChance(double dropChance) {
         this.dropChance = dropChance;
-        this.dropProbability = new ProbabilityImpl(ProbabilityFactory.probabilityFromPercent(dropChance));
+        this.dropProbability = Probability.ofPercentageValue(dropChance / 100);
     }
 
     public int getDropLevel() {

src/main/java/com/gmail/nossr50/util/random/Probability.java

@@ -1,5 +1,13 @@
 package com.gmail.nossr50.util.random;
 
+import com.gmail.nossr50.datatypes.player.McMMOPlayer;
+import com.gmail.nossr50.datatypes.skills.SubSkillType;
+import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.util.player.UserManager;
+import org.bukkit.entity.Player;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
 public interface Probability {
     /**
      * The value of this Probability
@@ -11,4 +19,53 @@ public interface Probability {
      * @return the value of probability
      */
     double getValue();
+
+    static @NotNull Probability ofSubSkill(@Nullable Player player,
+                                           @NotNull SubSkillType subSkillType,
+                                           @NotNull SkillProbabilityType skillProbabilityType) {
+
+        switch (skillProbabilityType) {
+            case DYNAMIC_CONFIGURABLE:
+                double probabilityCeiling;
+                double xCeiling;
+                double xPos;
+
+                if (player != null) {
+                    McMMOPlayer mmoPlayer = UserManager.getPlayer(player);
+                    if(mmoPlayer != null)
+                        xPos = mmoPlayer.getSkillLevel(subSkillType.getParentSkill());
+                    else
+                        xPos = 0;
+                } else {
+                    xPos = 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);
+            case STATIC_CONFIGURABLE:
+                try {
+                    return ofPercentageValue(getStaticRandomChance(subSkillType));
+                } catch (InvalidStaticChance invalidStaticChance) {
+                    invalidStaticChance.printStackTrace();
+                }
+            default:
+                throw new RuntimeException("No case in switch statement for Skill Probability Type!");
+        }
+    }
+
+    static @NotNull Probability ofPercentageValue(double percentageValue) {
+        return new ProbabilityImpl(percentageValue / 100);
+    }
+
+    static double getStaticRandomChance(@NotNull SubSkillType subSkillType) throws InvalidStaticChance {
+        return switch (subSkillType) {
+            case AXES_ARMOR_IMPACT -> mcMMO.p.getAdvancedConfig().getImpactChance();
+            case AXES_GREATER_IMPACT -> mcMMO.p.getAdvancedConfig().getGreaterImpactChance();
+            case TAMING_FAST_FOOD_SERVICE -> mcMMO.p.getAdvancedConfig().getFastFoodChance();
+            default -> throw new InvalidStaticChance();
+        };
+    }
 }

src/main/java/com/gmail/nossr50/util/random/ProbabilityFactory.java

@@ -1,82 +0,0 @@
-package com.gmail.nossr50.util.random;
-
-import com.gmail.nossr50.config.AdvancedConfig;
-import com.gmail.nossr50.datatypes.player.McMMOPlayer;
-import com.gmail.nossr50.datatypes.skills.SubSkillType;
-import com.gmail.nossr50.mcMMO;
-import com.gmail.nossr50.util.player.UserManager;
-import org.bukkit.entity.Player;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
-
-public class ProbabilityFactory {
-
-    public static @NotNull Probability ofPercentageValue(double percentageValue) {
-        return new ProbabilityImpl(probabilityFromPercent(percentageValue));
-    }
-
-    public static @NotNull Probability ofSubSkill(@Nullable Player player,
-                                                  @NotNull SubSkillType subSkillType,
-                                                  @NotNull SkillProbabilityType skillProbabilityType) {
-
-        switch (skillProbabilityType) {
-            case DYNAMIC_CONFIGURABLE:
-                double probabilityCeiling;
-                double xCeiling;
-                double xPos;
-
-                if (player != null) {
-                    McMMOPlayer mmoPlayer = UserManager.getPlayer(player);
-                    if(mmoPlayer != null)
-                        xPos = mmoPlayer.getSkillLevel(subSkillType.getParentSkill());
-                    else
-                        xPos = 0;
-                } else {
-                    xPos = 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);
-            case STATIC_CONFIGURABLE:
-                try {
-                    return ofPercentageValue(getStaticRandomChance(subSkillType));
-                } catch (InvalidStaticChance invalidStaticChance) {
-                    invalidStaticChance.printStackTrace();
-                }
-            default:
-                throw new RuntimeException("No case in switch statement for Skill Probability Type!");
-        }
-    }
-
-    /**
-     * Convert a probability from a percentage
-     * @param percentage value to convert
-     * @return 0 -> 1 inclusive representation of probability
-     */
-    public static double probabilityFromPercent(double percentage) {
-        return percentage / 100;
-    }
-
-    /**
-     * Grabs static activation rolls for Secondary Abilities
-     *
-     * @param subSkillType The secondary ability to grab properties of
-     * @return The static activation roll involved in the RNG calculation
-     * @throws InvalidStaticChance if the skill has no defined static chance this exception will be thrown and you should know you're a naughty boy
-     */
-    private static double getStaticRandomChance(@NotNull SubSkillType subSkillType) throws InvalidStaticChance {
-        switch (subSkillType) {
-            case AXES_ARMOR_IMPACT:
-                return mcMMO.p.getAdvancedConfig().getImpactChance();
-            case AXES_GREATER_IMPACT:
-                return mcMMO.p.getAdvancedConfig().getGreaterImpactChance();
-            case TAMING_FAST_FOOD_SERVICE:
-                return mcMMO.p.getAdvancedConfig().getFastFoodChance();
-            default:
-                throw new InvalidStaticChance();
-        }
-    }
-}

src/main/java/com/gmail/nossr50/util/random/ProbabilityImpl.java

@@ -12,7 +12,7 @@ public class ProbabilityImpl implements Probability {
      *
      * @param staticProbability the value to assign to this probability
      */
-    public ProbabilityImpl(double staticProbability) throws ValueOutOfBoundsException {
+    ProbabilityImpl(double staticProbability) throws ValueOutOfBoundsException {
         if (staticProbability < 0) {
             throw new ValueOutOfBoundsException("Value should never be negative for Probability! This suggests a coding mistake, contact the devs!");
         }

src/main/java/com/gmail/nossr50/util/skills/SkillUtils.java

@@ -391,7 +391,7 @@ public final class SkillUtils {
 
         //Mutate probability
         if(resultModifier != 1.0D)
-            probability = ProbabilityFactory.ofPercentageValue(probability.getValue() * resultModifier);
+            probability = Probability.ofPercentageValue(probability.getValue() * resultModifier);
 
         //Luck
         boolean isLucky = Permissions.lucky(player, subSkillType.getParentSkill());
@@ -414,7 +414,7 @@ public final class SkillUtils {
      */
     public static boolean isStaticSkillRNGSuccessful(@NotNull PrimarySkillType primarySkillType, @Nullable Player player, double probabilityPercentage) {
         //Grab a probability converted from a "percentage" value
-        Probability probability = ProbabilityFactory.ofPercentageValue(probabilityPercentage);
+        Probability probability = Probability.ofPercentageValue(probabilityPercentage);
 
         return isStaticSkillRNGSuccessful(primarySkillType, player, probability);
     }
@@ -463,7 +463,7 @@ public final class SkillUtils {
         if(subSkillType == SubSkillType.TAMING_FAST_FOOD_SERVICE || subSkillType == SubSkillType.AXES_ARMOR_IMPACT || subSkillType == SubSkillType.AXES_GREATER_IMPACT)
             skillProbabilityType = SkillProbabilityType.STATIC_CONFIGURABLE;
 
-        return ProbabilityFactory.ofSubSkill(player, subSkillType, skillProbabilityType);
+        return Probability.ofSubSkill(player, subSkillType, skillProbabilityType);
     }
 
     public static @NotNull String[] getRNGDisplayValues(@NotNull Player player, @NotNull SubSkillType subSkill) {

src/test/java/com/gmail/nossr50/util/random/RandomChanceUtilTest.java → src/test/java/com/gmail/nossr50/util/random/ProbabilityFactoryTest.java

@@ -10,7 +10,7 @@ import java.util.stream.Stream;
 import static com.gmail.nossr50.util.random.RandomChanceUtil.processProbability;
 import static org.junit.jupiter.api.Assertions.*;
 
-class RandomChanceUtilTest {
+class ProbabilityFactoryTest {
 
     private static Stream<Arguments> provideProbabilitiesForWithinExpectations() {
         return Stream.of(
@@ -48,6 +48,15 @@ class RandomChanceUtilTest {
         }
     }
 
+    @Test
+    void testIsSuccessfulRollFailsOfPercentage() {
+        Probability probability = Probability.ofPercentageValue(100);
+
+        for (int i = 0; i < 100000; i++) {
+            assertFalse(processProbability(probability));
+        }
+    }
+
     @ParameterizedTest
     @MethodSource("provideProbabilitiesForWithinExpectations")
     void testProcessProbabilityWithinExpectations(Probability probability, double expectedWinPercent) {
@@ -67,10 +76,10 @@ class RandomChanceUtilTest {
     }
 
     @Test
-    void chanceOfSuccessPercentage() {
+    void ofPercentageValue() {
     }
 
     @Test
-    void testChanceOfSuccessPercentage() {
+    void ofSubSkill() {
     }
 }

src/test/java/com/gmail/nossr50/util/random/ProbabilityTest.java

@@ -0,0 +1,24 @@
+package com.gmail.nossr50.util.random;
+
+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.stream.Stream;
+
+import static com.gmail.nossr50.util.random.RandomChanceUtil.processProbability;
+import static org.junit.jupiter.api.Assertions.*;
+
+class ProbabilityTest {
+
+
+
+    @Test
+    void chanceOfSuccessPercentage() {
+    }
+
+    @Test
+    void testChanceOfSuccessPercentage() {
+    }
+}