Skip to content

Commit

Permalink
Fix tasks & world leaking after match end (#1383)
Browse files Browse the repository at this point in the history 
Signed-off-by: Pablo Herrera <[email protected]>
  • Loading branch information
@Pablete1234
Pablete1234 committed Nov 30, 2024
1 parent f3a3f92 commit 4e147c4
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 150 deletions.
25 changes: 15 additions & 10 deletions core/src/main/java/tc/oc/pgm/listeners/ChatDispatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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))
Expand All @@ -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());
}

Expand Down Expand Up @@ -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());

Expand All @@ -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();
Expand Down
37 changes: 16 additions & 21 deletions core/src/main/java/tc/oc/pgm/listeners/MatchAnnouncer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)));
}
}
148 changes: 62 additions & 86 deletions core/src/main/java/tc/oc/pgm/loot/LootableMatchModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
});
}

/**
Expand All @@ -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) {
Expand All @@ -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<>();
Expand All @@ -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));
});
}
}

Expand Down Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/tc/oc/pgm/loot/WorldTickClock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand All @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/tc/oc/pgm/match/MatchImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Expand Down
Loading

0 comments on commit 4e147c4

Please sign in to comment.