Fix tasks leaking after match end

core/src/main/java/tc/oc/pgm/listeners/ChatDispatcher.java

@@ -12,6 +12,8 @@
 import com.google.common.cache.CacheBuilder;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -31,6 +33,7 @@
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
 import org.bukkit.event.player.AsyncPlayerChatEvent;
+import org.bukkit.event.player.PlayerQuitEvent;
 import org.incendo.cloud.annotation.specifier.Greedy;
 import org.incendo.cloud.annotations.Argument;
 import org.incendo.cloud.annotations.Command;
@@ -52,7 +55,6 @@
 import tc.oc.pgm.util.Audience;
 import tc.oc.pgm.util.Players;
 import tc.oc.pgm.util.bukkit.BukkitUtils;
-import tc.oc.pgm.util.bukkit.OnlinePlayerMapAdapter;
 import tc.oc.pgm.util.channels.Channel;
 import tc.oc.pgm.util.event.ChannelMessageEvent;
 import tc.oc.pgm.util.named.NameStyle;
@@ -69,7 +71,7 @@ public static ChatDispatcher get() {
   }
 
   private final MatchManager manager;
-  private final OnlinePlayerMapAdapter<MessageSenderIdentity> lastMessagedBy;
+  private final Map<UUID, MessageSenderIdentity> lastMessagedBy;
 
   public static final TextComponent ADMIN_CHAT_PREFIX = text()
       .append(text("[", NamedTextColor.WHITE))
@@ -94,7 +96,7 @@ public static ChatDispatcher get() {
 
   public ChatDispatcher() {
     this.manager = PGM.get().getMatchManager();
-    this.lastMessagedBy = new OnlinePlayerMapAdapter<>(PGM.get());
+    this.lastMessagedBy = new HashMap<>();
     PGM.get().getServer().getPluginManager().registerEvents(this, PGM.get());
   }
 
@@ -439,13 +441,17 @@ private Component getChatFormat(@Nullable Component prefix, MatchPlayer player,
         .build();
   }
 
+  @EventHandler(ignoreCancelled = true)
+  public void onPlayerQuit(PlayerQuitEvent event) {
+    this.lastMessagedBy.remove(event.getPlayer().getUniqueId());
+  }
+
   private void trackMessage(Player receiver, Player sender) {
-    MessageSenderIdentity senderIdent = new MessageSenderIdentity(receiver, sender);
-    this.lastMessagedBy.put(receiver, senderIdent);
+    this.lastMessagedBy.put(receiver.getUniqueId(), new MessageSenderIdentity(receiver, sender));
   }
 
   private UUID getLastMessagedId(Player sender) {
-    MessageSenderIdentity targetIdent = lastMessagedBy.get(sender);
+    MessageSenderIdentity targetIdent = lastMessagedBy.get(sender.getUniqueId());
     if (targetIdent == null) return null;
     MatchPlayer target = manager.getPlayer(targetIdent.getPlayerId());
 
@@ -466,10 +472,9 @@ private UUID getLastMessagedId(Player sender) {
     return null;
   }
 
-  private class MessageSenderIdentity {
-
-    private UUID playerId;
-    private String name;
+  private static class MessageSenderIdentity {
+    private final UUID playerId;
+    private final String name;
 
     public MessageSenderIdentity(Player viewer, Player player) {
       this.playerId = player.getUniqueId();

core/src/main/java/tc/oc/pgm/listeners/MatchAnnouncer.java

@@ -53,8 +53,7 @@ public void onMatchLoad(final MatchLoadEvent event) {
     final Match match = event.getMatch();
     match
         .getExecutor(MatchScope.LOADED)
-        .scheduleWithFixedDelay(
-            () -> match.getPlayers().forEach(this::sendCurrentlyPlaying), 0, 5, TimeUnit.MINUTES);
+        .scheduleWithFixedDelay(() -> sendCurrentlyPlaying(match), 0, 5, TimeUnit.MINUTES);
   }
 
   @EventHandler(priority = EventPriority.MONITOR)
@@ -83,12 +82,11 @@ public void onMatchEnd(final MatchFinishEvent event) {
         title = translatable("broadcast.gameOver");
       } else {
         if (singleWinner) {
-          title =
-              translatable(
-                  Iterables.getOnlyElement(winners).isNamePlural()
-                      ? "broadcast.gameOver.teamWinners"
-                      : "broadcast.gameOver.teamWinner",
-                  TextFormatter.nameList(winners, NameStyle.FANCY, NamedTextColor.WHITE));
+          title = translatable(
+              Iterables.getOnlyElement(winners).isNamePlural()
+                  ? "broadcast.gameOver.teamWinners"
+                  : "broadcast.gameOver.teamWinner",
+              TextFormatter.nameList(winners, NameStyle.FANCY, NamedTextColor.WHITE));
         }
 
         // Use stream here instead of #contains to avoid unchecked cast
@@ -156,13 +154,11 @@ public void sendWelcomeMessage(MatchPlayer viewer, List<Component> extraLines) {
 
     Collection<Contributor> authors = mapInfo.getAuthors();
     if (!authors.isEmpty()) {
-      viewer.sendMessage(
-          space()
-              .append(
-                  translatable(
-                      "misc.createdBy",
-                      NamedTextColor.GRAY,
-                      TextFormatter.nameList(authors, NameStyle.FANCY, NamedTextColor.GRAY))));
+      viewer.sendMessage(space()
+          .append(translatable(
+              "misc.createdBy",
+              NamedTextColor.GRAY,
+              TextFormatter.nameList(authors, NameStyle.FANCY, NamedTextColor.GRAY))));
     }
 
     // Send extra info from other plugins
@@ -173,11 +169,10 @@ public void sendWelcomeMessage(MatchPlayer viewer, List<Component> extraLines) {
     viewer.sendMessage(TextFormatter.horizontalLine(NamedTextColor.WHITE, 200));
   }
 
-  private void sendCurrentlyPlaying(MatchPlayer viewer) {
-    viewer.sendMessage(
-        translatable(
-            "misc.playing",
-            NamedTextColor.DARK_PURPLE,
-            viewer.getMatch().getMap().getStyledName(MapNameStyle.COLOR_WITH_AUTHORS)));
+  private void sendCurrentlyPlaying(Match match) {
+    match.sendMessage(translatable(
+        "misc.playing",
+        NamedTextColor.DARK_PURPLE,
+        match.getMap().getStyledName(MapNameStyle.COLOR_WITH_AUTHORS)));
   }
 }

core/src/main/java/tc/oc/pgm/loot/LootableMatchModule.java

@@ -57,18 +57,16 @@ public LootableMatchModule(
     this.fillers = fillers;
     this.match = match;
     this.caches = caches;
-    this.filledAt = new InstantMap<>(new WorldTickClock(match.getWorld()));
+    this.filledAt = new InstantMap<>(match.getClock());
     this.immm = match.getModule(ItemModifyMatchModule.class);
 
     final FilterMatchModule fmm = match.needModule(FilterMatchModule.class);
 
-    fillers.forEach(
-        filler -> {
-          fmm.onRise(
-              Match.class,
-              filler.getRefillTrigger(),
-              m -> this.filledAt.keySet().removeIf(f -> filler.equals(f.getRight())));
-        });
+    fillers.forEach(filler -> {
+      fmm.onRise(Match.class, filler.getRefillTrigger(), m -> this.filledAt
+          .keySet()
+          .removeIf(f -> filler.equals(f.getRight())));
+    });
   }
 
   /**
@@ -78,9 +76,10 @@ public LootableMatchModule(
   private static @Nullable Predicate<Filter> filterPredicate(InventoryHolder holder) {
     if (holder instanceof DoubleChest) {
       final DoubleChest doubleChest = (DoubleChest) holder;
-      return filter ->
-          !filter.query(new BlockQuery((Chest) doubleChest.getLeftSide())).isDenied()
-              || !filter.query(new BlockQuery((Chest) doubleChest.getRightSide())).isDenied();
+      return filter -> !filter
+              .query(new BlockQuery((Chest) doubleChest.getLeftSide()))
+              .isDenied()
+          || !filter.query(new BlockQuery((Chest) doubleChest.getRightSide())).isDenied();
     } else if (holder instanceof BlockState) {
       return filter -> !filter.query(new BlockQuery((BlockState) holder)).isDenied();
     } else if (holder instanceof Player) {
@@ -104,25 +103,17 @@ public void onInventoryOpen(InventoryOpenEvent event) {
     final Predicate<Filter> filterPredicate = filterPredicate(inventory.getHolder());
     if (filterPredicate == null) return;
 
-    logger.fine(
-        () ->
-            opener.getNameLegacy()
-                + " opened a "
-                + inventory.getHolder().getClass().getSimpleName());
+    logger.fine(() ->
+        opener.getNameLegacy() + " opened a " + inventory.getHolder().getClass().getSimpleName());
 
     // Find all Fillers that apply to the holder of the opened inventory
-    final List<FillerDefinition> fillers =
-        this.fillers.stream()
-            .filter(filler -> filterPredicate.test(filler.getFilter()))
-            .collect(Collectors.toList());
+    final List<FillerDefinition> fillers = this.fillers.stream()
+        .filter(filler -> filterPredicate.test(filler.getFilter()))
+        .collect(Collectors.toList());
     if (fillers.isEmpty()) return;
 
-    logger.fine(
-        () ->
-            "Found fillers "
-                + fillers.stream()
-                    .map(FillerDefinition::toString)
-                    .collect(Collectors.joining(", ")));
+    logger.fine(() -> "Found fillers "
+        + fillers.stream().map(FillerDefinition::toString).collect(Collectors.joining(", ")));
 
     // Find all Caches that the opened inventory is part of
     final List<Fillable> fillables = new ArrayList<>();
@@ -145,67 +136,54 @@ abstract class Fillable {
 
     void fill(MatchPlayer opener, List<FillerDefinition> fillers) {
       // Build a short list of Fillers that are NOT cooling down from a previous fill
-      final List<FillerDefinition> coolFillers =
-          fillers.stream()
-              .filter(
-                  filler ->
-                      filledAt.putUnlessNewer(new Pair<>(this, filler), filler.getRefillInterval())
-                          == null)
-              .collect(Collectors.toList());
+      final List<FillerDefinition> coolFillers = fillers.stream()
+          .filter(filler ->
+              filledAt.putUnlessNewer(new Pair<>(this, filler), filler.getRefillInterval()) == null)
+          .collect(Collectors.toList());
 
       // Find all the Inventories for this Fillable, and build a map of Fillers to the subset
       // of Inventories that they are allowed to fill, based on the filter of each Filler.
       // Note how duplicate inventories are skipped.
       final SetMultimap<FillerDefinition, Inventory> fillerInventories = HashMultimap.create();
-      inventories()
-          .distinct()
-          .forEach(
-              inventory -> {
-                final Predicate<Filter> passes = filterPredicate(inventory.getHolder());
-                if (passes == null) return;
-                for (FillerDefinition filler : coolFillers) {
-                  if (passes.test(filler.getFilter())) {
-                    fillerInventories.put(filler, inventory);
-                  }
-                }
-              });
-
-      fillerInventories
-          .asMap()
-          .forEach(
-              (filler, inventories) -> {
-                if (filler.cleanBeforeRefill()) {
-                  inventories().forEach(Inventory::clear);
-                }
-              });
+      inventories().distinct().forEach(inventory -> {
+        final Predicate<Filter> passes = filterPredicate(inventory.getHolder());
+        if (passes == null) return;
+        for (FillerDefinition filler : coolFillers) {
+          if (passes.test(filler.getFilter())) {
+            fillerInventories.put(filler, inventory);
+          }
+        }
+      });
+
+      fillerInventories.asMap().forEach((filler, inventories) -> {
+        if (filler.cleanBeforeRefill()) {
+          inventories().forEach(Inventory::clear);
+        }
+      });
 
       final Random random = new Random();
 
-      fillerInventories
-          .asMap()
-          .forEach(
-              (filler, inventories) -> {
-                // For each Filler, build a mutable list of slots that it can fill.
-                // (27 is the standard size for single chests)
-                final List<InventorySlot> slots = new ArrayList<>(inventories.size() * 27);
-                for (Inventory inv : inventories) {
-                  for (int index = 0; index < inv.getSize(); index++) {
-                    if (inv.getItem(index) == null) {
-                      slots.add(new InventorySlot(index, inv));
-                    }
-                  }
-                }
-
-                filler
-                    .getLoot()
-                    .elements(opener)
-                    .limit(slots.size())
-                    .peek(
-                        i -> {
-                          if (immm != null) immm.applyRules(i);
-                        })
-                    .forEachOrdered(i -> slots.remove(random.nextInt(slots.size())).putItem(i));
-              });
+      fillerInventories.asMap().forEach((filler, inventories) -> {
+        // For each Filler, build a mutable list of slots that it can fill.
+        // (27 is the standard size for single chests)
+        final List<InventorySlot> slots = new ArrayList<>(inventories.size() * 27);
+        for (Inventory inv : inventories) {
+          for (int index = 0; index < inv.getSize(); index++) {
+            if (inv.getItem(index) == null) {
+              slots.add(new InventorySlot(index, inv));
+            }
+          }
+        }
+
+        filler
+            .getLoot()
+            .elements(opener)
+            .limit(slots.size())
+            .peek(i -> {
+              if (immm != null) immm.applyRules(i);
+            })
+            .forEachOrdered(i -> slots.remove(random.nextInt(slots.size())).putItem(i));
+      });
     }
   }
 
@@ -256,13 +234,11 @@ Stream<Inventory> inventories() {
           .region()
           .getChunkPositions()
           .map(cp -> cp.getChunk(match.getWorld()))
-          .<InventoryQuery>flatMap(
-              ch ->
-                  Stream.concat(
-                      Stream.of(ch.getTileEntities()).map(BlockQuery::new),
-                      Stream.of(ch.getEntities())
-                          .filter(e -> !(e instanceof Player))
-                          .map(EntityQuery::new)))
+          .<InventoryQuery>flatMap(ch -> Stream.concat(
+              Stream.of(ch.getTileEntities()).map(BlockQuery::new),
+              Stream.of(ch.getEntities())
+                  .filter(e -> !(e instanceof Player))
+                  .map(EntityQuery::new)))
           .filter(q -> cache.jointFilter().query(q).isAllowed())
           .map(InventoryQuery::getInventory)
           .filter(Objects::nonNull);

core/src/main/java/tc/oc/pgm/loot/WorldTickClock.java

@@ -5,7 +5,7 @@
 import java.time.Clock;
 import java.time.Instant;
 import java.time.ZoneId;
-import org.bukkit.World;
+import tc.oc.pgm.api.match.Match;
 import tc.oc.pgm.api.time.Tick;
 import tc.oc.pgm.util.collection.InstantMap;
 
@@ -18,11 +18,11 @@
  */
 public class WorldTickClock extends Clock {
 
-  private final World world;
+  private final Match match;
   private Tick tick;
 
-  public WorldTickClock(World world) {
-    this.world = world;
+  public WorldTickClock(Match match) {
+    this.match = match;
   }
 
   @Override
@@ -45,7 +45,7 @@ public Tick getTick() {
   }
 
   private Tick now() {
-    long tick = NMS_HACKS.getMonotonicTime(this.world);
+    long tick = NMS_HACKS.getMonotonicTime(match.getWorld());
     if (this.tick == null || tick != this.tick.tick) {
       this.tick = new Tick(tick, Instant.now());
     }

core/src/main/java/tc/oc/pgm/match/MatchImpl.java

@@ -124,7 +124,7 @@ protected MatchImpl(String id, MapContext map, World world) {
     this.world = new WeakReference<>(assertNotNull(world));
     this.matchModules = new ConcurrentHashMap<>();
 
-    this.clock = new WorldTickClock(world);
+    this.clock = new WorldTickClock(this);
     this.logger = ClassLogger.get(PGM.get().getLogger(), getClass());
     this.random = new Random();
     this.tickRandoms = new HashMap<>();