Track advancement criterion counters on a per-advancement basis (should fix multiple triggering at once)

src/main/java/com/minelittlepony/unicopia/advancement/AbstractRepeatingCriterion.java

@@ -0,0 +1,78 @@
+package com.minelittlepony.unicopia.advancement;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiPredicate;
+import com.minelittlepony.unicopia.entity.player.Pony;
+
+import net.minecraft.advancement.PlayerAdvancementTracker;
+import net.minecraft.advancement.criterion.AbstractCriterion;
+import net.minecraft.advancement.criterion.Criterion;
+import net.minecraft.loot.context.LootContext;
+import net.minecraft.predicate.entity.EntityPredicate;
+import net.minecraft.server.network.ServerPlayerEntity;
+
+public abstract class AbstractRepeatingCriterion<T extends AbstractRepeatingCriterion.Conditions> implements Criterion<T> {
+    private final Map<PlayerAdvancementTracker, Set<Criterion.ConditionsContainer<T>>> progressions = new IdentityHashMap<>();
+
+    @Override
+    public final void beginTrackingCondition(PlayerAdvancementTracker manager, Criterion.ConditionsContainer<T> conditions) {
+        progressions.computeIfAbsent(manager, m -> new HashSet<>()).add(conditions);
+    }
+
+    @Override
+    public final void endTrackingCondition(PlayerAdvancementTracker manager, Criterion.ConditionsContainer<T> conditions) {
+        var set = progressions.get(manager);
+        if (set != null) {
+            set.remove(conditions);
+            if (set.isEmpty()) {
+                progressions.remove(manager);
+            }
+        }
+    }
+
+    @Override
+    public final void endTracking(PlayerAdvancementTracker tracker) {
+        progressions.remove(tracker);
+    }
+
+    protected void trigger(ServerPlayerEntity player, BiPredicate<Integer, T> predicate) {
+        PlayerAdvancementTracker tracker = player.getAdvancementTracker();
+        TriggerCountTracker counter = Pony.of(player).getAdvancementProgress();
+        counter.removeGranted(player, tracker);
+
+        var advancements = progressions.get(tracker);
+        if (advancements != null && !advancements.isEmpty()) {
+            LootContext lootContext = EntityPredicate.createAdvancementEntityLootContext(player, player);
+            List<Criterion.ConditionsContainer<T>> matches = null;
+
+            for (var condition : advancements) {
+                T conditions = condition.conditions();
+                if (predicate.test(counter.update(condition.advancement(), condition.id()), conditions)) {
+                    var playerPredicate = conditions.player();
+                    if (playerPredicate.isEmpty() || playerPredicate.get().test(lootContext)) {
+                        if (matches == null) {
+                            matches = new ArrayList<>();
+                        }
+
+                        matches.add(condition);
+                    }
+                }
+            }
+
+            if (matches != null) {
+                for (var advancement : matches) {
+                    advancement.grant(tracker);
+                }
+                counter.removeGranted(player, tracker);
+            }
+        }
+    }
+
+    public interface Conditions extends AbstractCriterion.Conditions {
+    }
+}

src/main/java/com/minelittlepony/unicopia/advancement/CustomEventCriterion.java

@@ -11,13 +11,12 @@ import com.mojang.serialization.codecs.RecordCodecBuilder;
 
 import net.fabricmc.fabric.api.util.TriState;
 import net.minecraft.advancement.AdvancementCriterion;
-import net.minecraft.advancement.criterion.AbstractCriterion;
 import net.minecraft.entity.Entity;
 import net.minecraft.predicate.entity.EntityPredicate;
 import net.minecraft.predicate.entity.LootContextPredicate;
 import net.minecraft.server.network.ServerPlayerEntity;
 
-public class CustomEventCriterion extends AbstractCriterion<CustomEventCriterion.Conditions> {
+public class CustomEventCriterion extends AbstractRepeatingCriterion<CustomEventCriterion.Conditions> {
     @Override
     public Codec<Conditions> getConditionsCodec() {
         return Conditions.CODEC;
@@ -26,9 +25,7 @@ public class CustomEventCriterion extends AbstractCriterion<CustomEventCriterion
     public CustomEventCriterion.Trigger createTrigger(String name) {
         return player -> {
             if (player instanceof ServerPlayerEntity p) {
-                int counter = Pony.of(p).getAdvancementProgress().compute(name, (key, i) -> i == null ? 1 : i + 1);
+                trigger(p, (count, condition) -> condition.test(name, count, p));
-
-                trigger(p, c -> c.test(name, counter, p));
             }
         };
     }
@@ -58,7 +55,7 @@ public class CustomEventCriterion extends AbstractCriterion<CustomEventCriterion
             String event,
             RacePredicate races,
             TriState flying,
-            int repeatCount) implements AbstractCriterion.Conditions {
+            int repeatCount) implements AbstractRepeatingCriterion.Conditions {
         public static final Codec<Conditions> CODEC = RecordCodecBuilder.create(instance -> instance.group(
                 EntityPredicate.LOOT_CONTEXT_PREDICATE_CODEC.optionalFieldOf("player").forGetter(Conditions::player),
                 Codec.STRING.fieldOf("event").forGetter(Conditions::event),
@@ -72,7 +69,7 @@ public class CustomEventCriterion extends AbstractCriterion<CustomEventCriterion
             return this.event.equalsIgnoreCase(event)
                     && races.test(player)
                     && flying.orElse(isFlying) == isFlying
-                    && (repeatCount <= 0 || (count > 0 && count % repeatCount == 0));
+                    && (repeatCount < 0 || repeatCount == count);
         }
     }
 }

src/main/java/com/minelittlepony/unicopia/advancement/SendViaDragonBreathScrollCriterion.java

@@ -3,13 +3,11 @@ package com.minelittlepony.unicopia.advancement;
 import java.util.Optional;
 import java.util.function.BiConsumer;
 
-import com.minelittlepony.unicopia.entity.player.Pony;
 import com.minelittlepony.unicopia.util.serialization.CodecUtils;
 import com.mojang.serialization.Codec;
 import com.mojang.serialization.codecs.RecordCodecBuilder;
 
 import net.fabricmc.fabric.api.util.TriState;
-import net.minecraft.advancement.criterion.AbstractCriterion;
 import net.minecraft.entity.LivingEntity;
 import net.minecraft.entity.player.PlayerEntity;
 import net.minecraft.item.ItemStack;
@@ -20,7 +18,7 @@ import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.server.world.ServerWorld;
 import net.minecraft.util.TypeFilter;
 
-public class SendViaDragonBreathScrollCriterion extends AbstractCriterion<SendViaDragonBreathScrollCriterion.Conditions> {
+public class SendViaDragonBreathScrollCriterion extends AbstractRepeatingCriterion<SendViaDragonBreathScrollCriterion.Conditions> {
     @Override
     public Codec<Conditions> getConditionsCodec() {
         return Conditions.CODEC;
@@ -28,10 +26,10 @@ public class SendViaDragonBreathScrollCriterion extends AbstractCriterion<SendVi
 
     public void triggerSent(PlayerEntity player, ItemStack payload, String recipient, BiConsumer<String, Integer> counterCallback) {
         if (player instanceof ServerPlayerEntity spe) {
-            trigger(spe, c -> {
+            trigger(spe, (count, c) -> {
                 if (c.test(spe, payload, recipient, false)) {
                     c.counter.ifPresent(counter -> {
-                        counterCallback.accept(counter, Pony.of(spe).getAdvancementProgress().compute(counter, (key, i) -> i == null ? 1 : i + 1));
+                        counterCallback.accept(counter, count);
                     });
                     return true;
                 }
@@ -42,7 +40,7 @@ public class SendViaDragonBreathScrollCriterion extends AbstractCriterion<SendVi
 
     public void triggerReceived(LivingEntity recipient, ItemStack payload) {
         if (recipient instanceof ServerPlayerEntity spe) {
-            trigger(spe, c -> c.test(spe, payload, recipient.getDisplayName().getString(), true));
+            trigger(spe, (count, c) -> c.test(spe, payload, recipient.getDisplayName().getString(), true));
         }
     }
 
@@ -54,7 +52,7 @@ public class SendViaDragonBreathScrollCriterion extends AbstractCriterion<SendVi
             TriState recipientPresent,
             Optional<String> counter,
             RacePredicate races
-    ) implements AbstractCriterion.Conditions {
+    ) implements AbstractRepeatingCriterion.Conditions {
         public static final Codec<Conditions> CODEC = RecordCodecBuilder.create(instance -> instance.group(
                 EntityPredicate.LOOT_CONTEXT_PREDICATE_CODEC.optionalFieldOf("player").forGetter(Conditions::player),
                 ItemPredicate.CODEC.optionalFieldOf("item").forGetter(Conditions::item),
@@ -64,7 +62,6 @@ public class SendViaDragonBreathScrollCriterion extends AbstractCriterion<SendVi
                 Codec.STRING.optionalFieldOf("counter").forGetter(Conditions::counter),
                 RacePredicate.CODEC.optionalFieldOf("races", RacePredicate.EMPTY).forGetter(Conditions::races)
             ).apply(instance, Conditions::new));
-
         public boolean test(ServerPlayerEntity player, ItemStack payload, String recipient, boolean receiving) {
             return isReceivingEnd == receiving
                     && races.test(player)

src/main/java/com/minelittlepony/unicopia/advancement/TriggerCountTracker.java

@@ -0,0 +1,59 @@
+package com.minelittlepony.unicopia.advancement;
+
+import java.util.Map;
+
+import org.jetbrains.annotations.Nullable;
+
+import com.minelittlepony.unicopia.util.Copyable;
+import com.mojang.serialization.Codec;
+import com.mojang.serialization.DataResult;
+
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
+import net.minecraft.advancement.AdvancementEntry;
+import net.minecraft.advancement.PlayerAdvancementTracker;
+import net.minecraft.server.network.ServerPlayerEntity;
+import net.minecraft.util.Identifier;
+
+public class TriggerCountTracker implements Copyable<TriggerCountTracker> {
+    public static final Codec<TriggerCountTracker> CODEC = Codec.unboundedMap(Key.CODEC, Codec.INT).xmap(TriggerCountTracker::new, tracker -> tracker.entries);
+
+    private final Object2IntMap<Key> entries = new Object2IntOpenHashMap<>();
+
+    public TriggerCountTracker(Map<Key, Integer> entries) {
+        this.entries.putAll(entries);
+    }
+
+    public int update(AdvancementEntry advancement, String criterionName) {
+        return entries.computeInt(new Key(advancement.id(), criterionName), (key, initial) -> (initial == null ? 0 : initial) + 1);
+    }
+
+    public void removeGranted(ServerPlayerEntity player, PlayerAdvancementTracker tracker) {
+        entries.keySet().removeIf(key -> {
+            @Nullable
+            AdvancementEntry a = player.getServer().getAdvancementLoader().get(key.advancement());
+            return a == null || tracker.getProgress(a).isDone();
+        });
+    }
+
+    @Override
+    public void copyFrom(TriggerCountTracker other, boolean alive) {
+        entries.clear();
+        entries.putAll(other.entries);
+    }
+
+    record Key(Identifier advancement, String criterion) {
+        public static final Codec<Key> CODEC = Codec.STRING.flatXmap(s -> {
+           String[] parts = s.split(":");
+           return parts.length == 3
+                   ? DataResult.success(new Key(Identifier.of(parts[0], parts[1]), parts[2]))
+                   : DataResult.error(() -> "String '" + s + "' was in the wrong format");
+        }, key -> DataResult.success(key.toString()));
+
+        @Override
+        public String toString() {
+            return advancement.toString() + ":" + criterion;
+        }
+    }
+
+}

src/main/java/com/minelittlepony/unicopia/entity/player/Pony.java

@@ -19,6 +19,7 @@ import com.minelittlepony.unicopia.ability.magic.spell.RageAbilitySpell;
 import com.minelittlepony.unicopia.ability.magic.spell.Spell;
 import com.minelittlepony.unicopia.ability.magic.spell.effect.SpellType;
 import com.minelittlepony.unicopia.ability.magic.spell.trait.TraitDiscovery;
+import com.minelittlepony.unicopia.advancement.TriggerCountTracker;
 import com.minelittlepony.unicopia.advancement.UCriteria;
 import com.minelittlepony.unicopia.entity.*;
 import com.minelittlepony.unicopia.entity.behaviour.EntityAppearance;
@@ -35,6 +36,7 @@ import com.minelittlepony.unicopia.item.UItems;
 import com.minelittlepony.unicopia.item.enchantment.EnchantmentUtil;
 import com.minelittlepony.unicopia.item.enchantment.UEnchantments;
 import com.minelittlepony.unicopia.util.*;
+import com.minelittlepony.unicopia.util.serialization.NbtSerialisable;
 import com.minelittlepony.unicopia.network.*;
 import com.minelittlepony.unicopia.network.track.DataTracker;
 import com.minelittlepony.unicopia.network.track.TrackableDataType;
@@ -84,7 +86,7 @@ public class Pony extends Living<PlayerEntity> implements Copyable<Pony>, Update
     private final Acrobatics acrobatics = new Acrobatics(this, tracker);
     private final CorruptionHandler corruptionHandler = new CorruptionHandler(this);
 
-    private final Map<String, Integer> advancementProgress = new HashMap<>();
+    private TriggerCountTracker advancementProgress = new TriggerCountTracker(Map.of());
 
     private final ManaContainer mana;
     private final PlayerLevelStore levels;
@@ -196,7 +198,7 @@ public class Pony extends Living<PlayerEntity> implements Copyable<Pony>, Update
         return 1 - (((float)animationDuration) / animationMaxDuration);
     }
 
-    public Map<String, Integer> getAdvancementProgress() {
+    public TriggerCountTracker getAdvancementProgress() {
         return advancementProgress;
     }
 
@@ -872,11 +874,7 @@ public class Pony extends Living<PlayerEntity> implements Copyable<Pony>, Update
         compound.put("discoveries", discoveries.toNBT(lookup));
         compound.putInt("ticksInvulnerable", ticksInvulnerable);
         compound.putInt("ticksMetamorphising", ticksMetamorphising);
-        NbtCompound progress = new NbtCompound();
+        compound.put("advancementTriggerCounts", NbtSerialisable.encode(TriggerCountTracker.CODEC, advancementProgress, lookup));
-        advancementProgress.forEach((key, count) -> {
-            progress.putInt(key, count);
-        });
-        compound.put("advancementProgress", progress);
     }
 
     @Override
@@ -894,11 +892,7 @@ public class Pony extends Living<PlayerEntity> implements Copyable<Pony>, Update
         ticksInSun = compound.getInt("ticksInSun");
         hasShades = compound.getBoolean("hasShades");
         ticksMetamorphising = compound.getInt("ticksMetamorphising");
-        NbtCompound progress = compound.getCompound("advancementProgress");
+        advancementProgress = NbtSerialisable.decode(TriggerCountTracker.CODEC, compound.get("advancementTriggerCounts"), lookup).orElseGet(() -> new TriggerCountTracker(Map.of()));
-        advancementProgress.clear();
-        for (String key : progress.getKeys()) {
-            advancementProgress.put(key, progress.getInt(key));
-        }
     }
 
     @Override
@@ -948,8 +942,7 @@ public class Pony extends Living<PlayerEntity> implements Copyable<Pony>, Update
         }
 
         mana.copyFrom(oldPlayer.mana, !forcedSwap);
-
+        advancementProgress.copyFrom(oldPlayer.advancementProgress, alive);
-        advancementProgress.putAll(oldPlayer.getAdvancementProgress());
         setDirty();
         onSpawn();
     }