Skip to content

Commit

Permalink
Fixed probability bug Fixes #5055 Fixes #5054
Browse files Browse the repository at this point in the history
  • Loading branch information
@nossr50
nossr50 committed Jul 27, 2024
1 parent 248116d commit e886a16
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 43 deletions.
7 changes: 7 additions & 0 deletions Changelog.txt
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/com/gmail/nossr50/commands/skills/AcrobaticsCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ protected List<String> statsDisplay(Player player, float skillValue, boolean has

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)) : ""));*/
}
}

Expand Down
48 changes: 13 additions & 35 deletions src/main/java/com/gmail/nossr50/commands/skills/SkillCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
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%");
Expand Down Expand Up @@ -163,8 +165,6 @@ private void sendSkillCommandHeader(String skillName, Player player, McMMOPlayer
/*
* CHILD SKILLS
*/


var parents = mcMMO.p.getSkillTools().getChildSkillParents(skill);

//TODO: Add JSON here
Expand All @@ -186,28 +186,7 @@ private void sendSkillCommandHeader(String skillName, Player player, McMMOPlayer
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<PrimarySkillType> 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
Expand All @@ -222,10 +201,6 @@ protected int calculateRank(float skillValue, int maxLevel, int rankChangeLevel)
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();
Expand All @@ -252,14 +227,19 @@ protected String getStatMessage(SubSkillType subSkillType, String... vars) {
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);
}
}
Expand All @@ -276,8 +256,6 @@ protected String getLimitBreakDescriptionParameter() {

protected abstract void permissionsCheck(Player player);

//protected abstract List<String> effectsDisplay();

protected abstract List<String> statsDisplay(Player player, float skillValue, boolean hasEndurance, boolean isLucky);

protected abstract List<Component> getTextComponents(Player player);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/gmail/nossr50/util/random/ProbabilityUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public static double chanceOfSuccessPercentage(@Nullable Player player,
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

Expand Down Expand Up @@ -437,7 +437,7 @@ public static Probability calculateCurrentSkillProbability(double skillLevel, do
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));
Expand Down
105 changes: 102 additions & 3 deletions src/test/java/com/gmail/nossr50/util/random/ProbabilityUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
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;
Expand Down Expand Up @@ -68,9 +70,106 @@ public void isSkillRNGSuccessfulShouldBehaveAsExpected() {
assertProbabilityExpectations(20, probability);
}

private static Stream<Arguments> 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 calculateCurrentSkillProbabilityShouldBeTwenty() {
final Probability probability = calculateCurrentSkillProbability(1000, 0, 20, 1000);
assertEquals(0.2D, probability.getValue());
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]);
}

}

0 comments on commit e886a16

Please sign in to comment.