From bf003438ea1a7014e9fbf7ffe09fc84a93e8e5ad Mon Sep 17 00:00:00 2001 From: Pablo Herrera Date: Mon, 16 Sep 2024 02:03:44 +0200 Subject: [PATCH] Add flexibility to voted pools (#1398) Signed-off-by: Pablo Herrera --- core/src/main/java/tc/oc/pgm/PGMPlugin.java | 6 +- .../main/java/tc/oc/pgm/api/map/MapOrder.java | 2 +- .../tc/oc/pgm/rotation/pools/VotingPool.java | 82 +++++++--- .../oc/pgm/rotation/vote/MapVotePicker.java | 84 +++++----- core/src/main/resources/config.yml | 4 +- core/src/main/resources/map-pools.yml | 18 +++ .../java/tc/oc/pgm/util/math/Formula.java | 152 +++++++++--------- .../util/math/VariableExpressionContext.java | 19 --- 8 files changed, 201 insertions(+), 166 deletions(-) delete mode 100644 util/src/main/java/tc/oc/pgm/util/math/VariableExpressionContext.java diff --git a/core/src/main/java/tc/oc/pgm/PGMPlugin.java b/core/src/main/java/tc/oc/pgm/PGMPlugin.java index 428840d8eb..9335ade09e 100644 --- a/core/src/main/java/tc/oc/pgm/PGMPlugin.java +++ b/core/src/main/java/tc/oc/pgm/PGMPlugin.java @@ -185,7 +185,11 @@ public void onEnable() { if (config.getMapPoolFile() != null) { MapPoolManager manager = new MapPoolManager(logger, config.getMapPoolFile().toFile(), datastore); - if (manager.getActiveMapPool() != null) mapOrder = manager; + var pool = manager.getActiveMapPool(); + if (pool != null) { + if (!pool.getMaps().isEmpty()) mapOrder = manager; + else logger.severe("Active pool has no maps. Falling back to a random pool."); + } } if (mapOrder == null) mapOrder = new RandomMapOrder(Lists.newArrayList(mapLibrary.getMaps())); diff --git a/core/src/main/java/tc/oc/pgm/api/map/MapOrder.java b/core/src/main/java/tc/oc/pgm/api/map/MapOrder.java index 9addbf71c3..e42e271304 100644 --- a/core/src/main/java/tc/oc/pgm/api/map/MapOrder.java +++ b/core/src/main/java/tc/oc/pgm/api/map/MapOrder.java @@ -34,7 +34,7 @@ public interface MapOrder { void setNextMap(MapInfo map); /** - * Returns the duration used for cycles in {@link CycleMatchModule}. + * Returns the duration used for cycles in {@link tc.oc.pgm.cycle.CycleMatchModule}. * * @return The cycle duration */ diff --git a/core/src/main/java/tc/oc/pgm/rotation/pools/VotingPool.java b/core/src/main/java/tc/oc/pgm/rotation/pools/VotingPool.java index 02be76fd52..cc453dadb9 100644 --- a/core/src/main/java/tc/oc/pgm/rotation/pools/VotingPool.java +++ b/core/src/main/java/tc/oc/pgm/rotation/pools/VotingPool.java @@ -8,7 +8,9 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; +import net.objecthunter.exp4j.ExpressionContext; import org.bukkit.configuration.ConfigurationSection; +import org.bukkit.configuration.MemoryConfiguration; import tc.oc.pgm.api.map.MapInfo; import tc.oc.pgm.api.match.Match; import tc.oc.pgm.api.match.MatchScope; @@ -16,14 +18,14 @@ import tc.oc.pgm.rotation.MapPoolManager; import tc.oc.pgm.rotation.vote.MapPoll; import tc.oc.pgm.rotation.vote.MapVotePicker; +import tc.oc.pgm.util.math.Formula; public class VotingPool extends MapPool { // Arbitrary default of 1 in 5 players liking each map public static final double DEFAULT_SCORE = 0.2; - // How much score to add/remove on a map every cycle - public final double ADJUST_FACTOR; + public final VoteConstants constants; // The algorithm used to pick the maps for next vote. public final MapVotePicker mapPicker; @@ -35,11 +37,10 @@ public class VotingPool extends MapPool { public VotingPool( MapPoolType type, String name, MapPoolManager manager, ConfigurationSection section) { super(type, name, manager, section); + this.constants = new VoteConstants(section, maps.size()); - this.ADJUST_FACTOR = DEFAULT_SCORE / maps.size(); - - this.mapPicker = MapVotePicker.of(manager, section); - for (MapInfo map : maps) mapScores.put(map, DEFAULT_SCORE); + this.mapPicker = MapVotePicker.of(manager, constants, section); + for (MapInfo map : maps) mapScores.put(map, constants.defaultScore()); } public VotingPool( @@ -52,9 +53,9 @@ public VotingPool( Duration cycleTime, List maps) { super(type, name, manager, enabled, players, dynamic, cycleTime, maps); - this.ADJUST_FACTOR = DEFAULT_SCORE / maps.size(); - this.mapPicker = MapVotePicker.of(manager, null); - for (MapInfo map : maps) mapScores.put(map, DEFAULT_SCORE); + this.constants = new VoteConstants(new MemoryConfiguration(), maps.size()); + this.mapPicker = MapVotePicker.of(manager, constants, null); + for (MapInfo map : maps) mapScores.put(map, constants.defaultScore()); } public MapPoll getCurrentPoll() { @@ -66,23 +67,22 @@ public double getMapScore(MapInfo map) { } /** Ticks scores for all maps, making them go slowly towards DEFAULT_WEIGHT. */ - private void tickScores(MapInfo currentMap) { + private void tickScores(Match match) { // If the current map isn't from this pool, ignore ticking - if (!mapScores.containsKey(currentMap)) return; - mapScores.replaceAll( - (mapScores, value) -> - value > DEFAULT_SCORE - ? Math.max(value - ADJUST_FACTOR, DEFAULT_SCORE) - : Math.min(value + ADJUST_FACTOR, DEFAULT_SCORE)); - mapScores.put(currentMap, 0d); + if (!mapScores.containsKey(match.getMap())) return; + mapScores.replaceAll((mapScores, value) -> value > DEFAULT_SCORE + ? Math.max(value - constants.scoreDecay(), DEFAULT_SCORE) + : Math.min(value + constants.scoreRise(), DEFAULT_SCORE)); + mapScores.put( + match.getMap(), constants.scoreAfterPlay().applyAsDouble(new Context(match.getDuration()))); } private void updateScores(Map> votes) { - double voters = votes.values().stream().flatMap(Collection::stream).distinct().count(); + double voters = + votes.values().stream().flatMap(Collection::stream).distinct().count(); if (voters == 0) return; // Literally no one voted - votes.forEach( - (m, v) -> - mapScores.computeIfPresent(m, (a, b) -> Math.max(v.size() / voters, Double.MIN_VALUE))); + votes.forEach((m, v) -> + mapScores.computeIfPresent(m, (a, b) -> constants.afterVoteScore(v.size() / voters))); } @Override @@ -111,12 +111,12 @@ public void setNextMap(MapInfo map) { @Override public void unloadPool(Match match) { - tickScores(match.getMap()); + tickScores(match); } @Override public void matchEnded(Match match) { - tickScores(match.getMap()); + tickScores(match); match .getExecutor(MatchScope.LOADED) .schedule( @@ -132,4 +132,40 @@ public void matchEnded(Match match) { 5, TimeUnit.SECONDS); } + + public record VoteConstants( + int voteOptions, + double defaultScore, + double scoreDecay, + double scoreRise, + double scoreAfterVoteMin, + double scoreAfterVoteMax, + double scoreMinToVote, + Formula scoreAfterPlay) { + private VoteConstants(ConfigurationSection section, int mapAmount) { + this( + section.getInt("vote-options", MapVotePicker.MAX_VOTE_OPTIONS), // Show 5 maps + section.getDouble("score.default", DEFAULT_SCORE), // Start at 20% each + section.getDouble("score.decay", DEFAULT_SCORE / mapAmount), // Proportional to # of maps + section.getDouble("score.rise", DEFAULT_SCORE / mapAmount), // Proportional to # of maps + section.getDouble("score.min-after-vote", 0.01), // min = 1%, never fully discard the map + section.getDouble("score.max-after-vote", 1), // max = 100% + section.getDouble("score.min-for-vote", 0.01), // To even be voted, need at least 1% + Formula.of(section.getString("score.after-playing"), Context.variables(), c -> 0)); + } + + public double afterVoteScore(double score) { + return Math.max(Math.min(score, scoreAfterVoteMax), scoreAfterVoteMin); + } + } + + private static final class Context extends ExpressionContext.Impl { + public Context(Duration length) { + super(Map.of("play_minutes", length.toMillis() / 60_000d), null); + } + + static Set variables() { + return new Context(Duration.ZERO).getVariables(); + } + } } diff --git a/core/src/main/java/tc/oc/pgm/rotation/vote/MapVotePicker.java b/core/src/main/java/tc/oc/pgm/rotation/vote/MapVotePicker.java index ff0d22c376..e532bbb162 100644 --- a/core/src/main/java/tc/oc/pgm/rotation/vote/MapVotePicker.java +++ b/core/src/main/java/tc/oc/pgm/rotation/vote/MapVotePicker.java @@ -1,7 +1,7 @@ package tc.oc.pgm.rotation.vote; -import com.google.common.collect.ImmutableMap; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -9,17 +9,17 @@ import java.util.Set; import java.util.TreeMap; import java.util.logging.Level; -import java.util.stream.Collectors; +import net.objecthunter.exp4j.ExpressionContext; import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.MemoryConfiguration; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import tc.oc.pgm.api.PGM; +import tc.oc.pgm.api.map.Gamemode; import tc.oc.pgm.api.map.MapInfo; -import tc.oc.pgm.api.map.MapTag; import tc.oc.pgm.rotation.MapPoolManager; +import tc.oc.pgm.rotation.pools.VotingPool; import tc.oc.pgm.util.math.Formula; -import tc.oc.pgm.util.math.VariableExpressionContext; /** * Responsible for picking the set of maps that will be on the vote. It's able to apply any @@ -33,35 +33,39 @@ public class MapVotePicker { // A 0 that prevents arbitrarily low values with tons of precision, which cause issues when mixed // with larger numbers. - private static final double MINIMUM_WEIGHT = 0.00000001; + private static final double MINIMUM_WEIGHT = 0.000001; - private static final Formula DEFAULT_MODIFIER = c -> Math.pow(c.getVariable("score"), 2); + private static final Formula DEFAULT_MODIFIER = + c -> Math.pow(c.getVariable("score"), 2); private final MapPoolManager manager; - private final Formula modifier; + private final VotingPool.VoteConstants constants; + private final Formula modifier; - public static MapVotePicker of(MapPoolManager manager, ConfigurationSection config) { + public static MapVotePicker of( + MapPoolManager manager, VotingPool.VoteConstants constants, ConfigurationSection config) { // Create dummy config to read defaults off of. if (config == null) config = new MemoryConfiguration(); - Formula formula = DEFAULT_MODIFIER; + Formula formula = DEFAULT_MODIFIER; try { formula = - Formula.of( - config.getString("modifier"), - Formula.ContextFactory.ofStatic(new Context().getVariables()), - DEFAULT_MODIFIER); + Formula.of(config.getString("modifier"), MapVoteContext.variables(), DEFAULT_MODIFIER); } catch (IllegalArgumentException e) { PGM.get() .getLogger() .log(Level.SEVERE, "Failed to load vote picker modifier formula, using fallback", e); } - return new MapVotePicker(manager, formula); + return new MapVotePicker(manager, constants, formula); } - public MapVotePicker(MapPoolManager manager, Formula modifier) { + private MapVotePicker( + MapPoolManager manager, + VotingPool.VoteConstants constants, + Formula modifier) { this.manager = manager; + this.constants = constants; this.modifier = modifier; } @@ -84,7 +88,7 @@ protected List getMaps(@Nullable List selected, Map(); List unmodifiable = Collections.unmodifiableList(selected); - while (selected.size() < MAX_VOTE_OPTIONS) { + while (selected.size() < constants.voteOptions()) { MapInfo map = getMap(unmodifiable, scores); if (map == null) break; // Ran out of maps! @@ -115,50 +119,40 @@ protected MapInfo getMap(List selected, Map mapScores) * @return random weight for the map */ public double getWeight(@Nullable List selected, @NotNull MapInfo map, double score) { - if ((selected != null && selected.contains(map)) || score <= 0) return 0; + if ((selected != null && selected.contains(map)) || score <= constants.scoreMinToVote()) + return 0; - Context context = - new Context( - score, - getRepeatedGamemodes(selected, map), - map.getMaxPlayers().stream().mapToInt(i -> i).sum(), - manager.getActivePlayers(null)); + var context = new MapVoteContext( + score, + getRepeatedGamemodes(selected, map), + map.getMaxPlayers().stream().mapToInt(i -> i).sum(), + manager.getActivePlayers(null)); return Math.max(modifier.applyAsDouble(context), 0); } private double getRepeatedGamemodes(List selected, MapInfo map) { if (selected == null || selected.isEmpty()) return 0; - List gamemodes = - map.getTags().stream().filter(MapTag::isGamemode).collect(Collectors.toList()); + Collection gamemodes = map.getGamemodes(); - return selected.stream().filter(s -> !Collections.disjoint(gamemodes, s.getTags())).count(); + return selected.stream() + .filter(s -> !Collections.disjoint(gamemodes, s.getGamemodes())) + .count(); } - private static final class Context implements VariableExpressionContext { - private final ImmutableMap values; - - public Context(double score, double sameGamemode, double mapsize, double players) { - this.values = - ImmutableMap.of( + private static final class MapVoteContext extends ExpressionContext.Impl { + public MapVoteContext(double score, double sameGamemode, double mapsize, double players) { + super( + Map.of( "score", score, "same_gamemode", sameGamemode, "mapsize", mapsize, - "players", players); - } - - private Context() { - this(0, 0, 0, 0); - } - - @Override - public Set getVariables() { - return values.keySet(); + "players", players), + null); } - @Override - public Double getVariable(String s) { - return values.get(s); + static Set variables() { + return new MapVoteContext(0, 0, 0, 0).getVariables(); } } } diff --git a/core/src/main/resources/config.yml b/core/src/main/resources/config.yml index 944a777ca3..d0f5251ae4 100644 --- a/core/src/main/resources/config.yml +++ b/core/src/main/resources/config.yml @@ -29,8 +29,8 @@ map: # - uri: "https://github.com/PGMDev/Maps" # path: "default-maps" - # A path to a map pools file, or empty to disable map pools. - pools: "map-pools.yml" + # A path to a map pools file, empty or commented-out to disable pools. + # pools: "map-pools.yml" # A path to the includes folder, or empty to disable map includes. includes: "includes" diff --git a/core/src/main/resources/map-pools.yml b/core/src/main/resources/map-pools.yml index c580a88db1..0033dfddba 100644 --- a/core/src/main/resources/map-pools.yml +++ b/core/src/main/resources/map-pools.yml @@ -30,6 +30,24 @@ pools: variants: - default + # Voted pool only: num of maps to show each vote + vote-options: 5 + # Voted pool only: modify how score behaves + score: + # How much score should maps start with by default. + default: 0.2 + # How much score decreases/increases after each match. Null to be proportional to amount of maps. + decay: null + rise: null + # When a map is voted, enforce that it never falls out of the range of: + min-after-vote: 0.01 # 1%, avoids fully discarding the map + max-after-vote: 1 + # Maps with less than this are ignored for a vote + min-for-vote: 0.01 + # After the map plays, reset the score to: + # Supports a formula with play_minutes + after-playing: 0 + # Voted pools support modifiers which come in the form of a formula (does not affect any other type of pool). # # This formula is parsed by exp4j library, it's quite flexible and supports many built-in functions. diff --git a/util/src/main/java/tc/oc/pgm/util/math/Formula.java b/util/src/main/java/tc/oc/pgm/util/math/Formula.java index 9cf9750031..3aab3ef4d9 100644 --- a/util/src/main/java/tc/oc/pgm/util/math/Formula.java +++ b/util/src/main/java/tc/oc/pgm/util/math/Formula.java @@ -1,91 +1,100 @@ package tc.oc.pgm.util.math; -import java.util.Collections; import java.util.Set; import java.util.function.ToDoubleFunction; +import java.util.logging.Level; import java.util.stream.Collectors; import net.objecthunter.exp4j.Expression; import net.objecthunter.exp4j.ExpressionBuilder; import net.objecthunter.exp4j.ExpressionContext; import net.objecthunter.exp4j.function.Function; +import tc.oc.pgm.util.bukkit.BukkitUtils; public interface Formula extends ToDoubleFunction { - Function BOUND = - new Function("bound", 3) { - @Override - public double apply(double... doubles) { - double val = doubles[0]; - double min = doubles[1]; - double max = doubles[2]; - return Math.max(min, Math.min(val, max)); - } - }; - - Function RANDOM = - new Function("random", 0) { - @Override - public double apply(double... doubles) { - return Math.random(); - } - }; + Function BOUND = new Function("bound", 3) { + @Override + public double apply(double... doubles) { + double val = doubles[0]; + double min = doubles[1]; + double max = doubles[2]; + return Math.max(min, Math.min(val, max)); + } + }; - Function MAX = - new Function("max") { - @Override - public double apply(double... doubles) { - double max = doubles[0]; - for (int i = 1; i < doubles.length; i++) max = Math.max(max, doubles[i]); - return max; - } + Function RANDOM = new Function("random", 0) { + @Override + public double apply(double... doubles) { + return Math.random(); + } + }; - @Override - public boolean isValidArgCount(int count) { - return count >= 1; - } - }; + Function MAX = new Function("max") { + @Override + public double apply(double... doubles) { + double max = doubles[0]; + for (int i = 1; i < doubles.length; i++) max = Math.max(max, doubles[i]); + return max; + } - Function MIN = - new Function("min") { - @Override - public double apply(double... doubles) { - double min = doubles[0]; - for (int i = 1; i < doubles.length; i++) min = Math.min(min, doubles[i]); - return min; - } + @Override + public boolean isValidArgCount(int count) { + return count >= 1; + } + }; - @Override - public boolean isValidArgCount(int count) { - return count >= 1; - } - }; + Function MIN = new Function("min") { + @Override + public double apply(double... doubles) { + double min = doubles[0]; + for (int i = 1; i < doubles.length; i++) min = Math.min(min, doubles[i]); + return min; + } - static Formula of(String expression, ContextFactory context, Formula fallback) - throws IllegalArgumentException { + @Override + public boolean isValidArgCount(int count) { + return count >= 1; + } + }; + + /** + * Create a formula for a config, if there's a misconfiguration it logs and uses fallback + * + * @param expression The expression to parse + * @param variables The set of available variables in the formula + * @param fallback A fallback if no value is defined or parsing fails + * @return The formula if it parsed correctly, fallback if anything goes wrong + * @param Type of expression context to use + */ + static Formula of( + String expression, Set variables, Formula fallback) { if (expression == null) return fallback; - return Formula.of(expression, context); + try { + return Formula.of(expression, ContextFactory.ofStatic(variables)); + } catch (IllegalArgumentException e) { + BukkitUtils.getPlugin() + .getLogger() + .log(Level.SEVERE, "Failed to load formula '" + expression + "' using fallback", e); + return fallback; + } } static Formula of(String expression, ContextFactory context) throws IllegalArgumentException { - Expression exp = - new ExpressionBuilder(expression) - .variables(context.getVariables()) - .functions(BOUND, RANDOM, MAX, MIN) - .functions( - context.getArrays().stream() - .map( - str -> - new Function(str, 1) { - @Override - public double apply(double... doubles) { - throw new UnsupportedOperationException( - "Cannot get array value without replacement!"); - } - }) - .collect(Collectors.toList())) - .build(); + Expression exp = new ExpressionBuilder(expression) + .variables(context.getVariables()) + .functions(BOUND, RANDOM, MAX, MIN) + .functions(context.getArrays().stream() + .map(str -> new Function(str, 1) { + @Override + public double apply(double... doubles) { + throw new UnsupportedOperationException( + "Cannot get array value without replacement!"); + } + }) + .collect(Collectors.toList())) + .build(); return new ExpFormula<>(exp, context); } @@ -113,17 +122,10 @@ interface ContextFactory { ExpressionContext withContext(T t); static ContextFactory ofStatic(Set variables) { - return of(variables, Collections.emptySet(), t -> t); - } - - static ContextFactory of( - Set variables, - Set arrays, - java.util.function.Function builder) { - return new ContextFactory() { + return new ContextFactory<>() { @Override public ExpressionContext withContext(T t) { - return builder.apply(t); + return t; } @Override @@ -133,7 +135,7 @@ public Set getVariables() { @Override public Set getArrays() { - return arrays; + return Set.of(); } }; } diff --git a/util/src/main/java/tc/oc/pgm/util/math/VariableExpressionContext.java b/util/src/main/java/tc/oc/pgm/util/math/VariableExpressionContext.java deleted file mode 100644 index 604a23a19d..0000000000 --- a/util/src/main/java/tc/oc/pgm/util/math/VariableExpressionContext.java +++ /dev/null @@ -1,19 +0,0 @@ -package tc.oc.pgm.util.math; - -import java.util.Collections; -import java.util.Set; -import net.objecthunter.exp4j.ExpressionContext; -import net.objecthunter.exp4j.function.Function; - -public interface VariableExpressionContext extends ExpressionContext { - - @Override - default Set getFunctions() { - return Collections.emptySet(); - } - - @Override - default Function getFunction(String s) { - return null; - } -}