From 9c6a107bff07c3ba9386676329916068590abf85 Mon Sep 17 00:00:00 2001 From: Magnus Ulf Date: Thu, 16 Jan 2020 10:56:05 +0100 Subject: [PATCH] Fix FactionsIndex bug --- .../massivecraft/factions/FactionsIndex.java | 16 +++++++++++++- .../factions/cmd/CmdFactionsList.java | 8 +------ .../massivecraft/factions/entity/MPlayer.java | 22 +++++++++++++------ 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/com/massivecraft/factions/FactionsIndex.java b/src/com/massivecraft/factions/FactionsIndex.java index 045dbff7..0b98cd18 100644 --- a/src/com/massivecraft/factions/FactionsIndex.java +++ b/src/com/massivecraft/factions/FactionsIndex.java @@ -86,7 +86,21 @@ public class FactionsIndex { if (mplayer == null) throw new NullPointerException("mplayer"); if (!FactionColl.get().isActive()) throw new IllegalStateException("The FactionColl is not yet fully activated."); - if (!mplayer.attached()) return; + + // TODO: This is not optimal but here we remove a player from the index when + // the mplayer object is deattached. Optimally it should be removed + // automatically because it is stored as a weak reference. + // But sometimes that doesn't happen so we do this instead. + // Is there a memory leak somewhere? + if (!mplayer.attached()) + { + Faction factionIndexed = this.mplayer2faction.remove(mplayer); + if (factionIndexed != null) + { + faction2mplayers.get(factionIndexed).remove(mplayer); + } + return; + } Faction factionActual = mplayer.getFaction(); Faction factionIndexed = this.getFaction(mplayer); diff --git a/src/com/massivecraft/factions/cmd/CmdFactionsList.java b/src/com/massivecraft/factions/cmd/CmdFactionsList.java index baca489c..03913c11 100644 --- a/src/com/massivecraft/factions/cmd/CmdFactionsList.java +++ b/src/com/massivecraft/factions/cmd/CmdFactionsList.java @@ -9,15 +9,11 @@ import com.massivecraft.massivecore.MassiveException; import com.massivecraft.massivecore.command.Parameter; import com.massivecraft.massivecore.pager.Pager; import com.massivecraft.massivecore.pager.Stringifier; -import com.massivecraft.massivecore.predicate.PredicateAnd; -import com.massivecraft.massivecore.predicate.PredicateVisibleTo; -import com.massivecraft.massivecore.store.SenderColl; import com.massivecraft.massivecore.util.Txt; import org.bukkit.Bukkit; import org.bukkit.command.CommandSender; import java.util.List; -import java.util.function.Predicate; public class CmdFactionsList extends FactionsCommand { @@ -46,8 +42,6 @@ public class CmdFactionsList extends FactionsCommand // NOTE: The faction list is quite slow and mostly thread safe. // We run it asynchronously to spare the primary server thread. - Predicate predicateOnline = PredicateAnd.get(mp -> mp.getId() != null, SenderColl.PREDICATE_ONLINE, PredicateVisibleTo.get(sender)); - // Pager Create final Pager pager = new Pager<>(this, "Faction List", page, (Stringifier) (faction, index) -> { if (faction.isNone()) @@ -58,7 +52,7 @@ public class CmdFactionsList extends FactionsCommand { return Txt.parse("%s %d/%d online, %d/%d/%d", faction.getName(msender), - faction.getMPlayersWhere(predicateOnline).size(), + faction.getMPlayersWhereOnlineTo(sender).size(), faction.getMPlayers().size(), faction.getLandCount(), faction.getPowerRounded(), diff --git a/src/com/massivecraft/factions/entity/MPlayer.java b/src/com/massivecraft/factions/entity/MPlayer.java index bf8eee75..f01bf190 100644 --- a/src/com/massivecraft/factions/entity/MPlayer.java +++ b/src/com/massivecraft/factions/entity/MPlayer.java @@ -33,7 +33,8 @@ import java.util.Iterator; import java.util.Map.Entry; import java.util.Set; -public class MPlayer extends SenderEntity implements FactionsParticipator, MPerm.MPermable { +public class MPlayer extends SenderEntity implements FactionsParticipator, MPerm.MPermable +{ // -------------------------------------------- // // META // -------------------------------------------- // @@ -60,7 +61,8 @@ public class MPlayer extends SenderEntity implements FactionsParticipat // -------------------------------------------- // @Override - public MPlayer load(MPlayer that) { + public MPlayer load(MPlayer that) + { this.setLastActivityMillis(that.lastActivityMillis); this.setFactionId(that.factionId); this.rankId = that.rankId; @@ -79,7 +81,8 @@ public class MPlayer extends SenderEntity implements FactionsParticipat // -------------------------------------------- // @Override - public boolean isDefault() { + public boolean isDefault() + { // Last activity millis is data we use for clearing out cleanable players. So it does not in itself make the player data worth keeping. if (this.hasFaction()) return false; // Role means nothing without a faction. @@ -98,18 +101,23 @@ public class MPlayer extends SenderEntity implements FactionsParticipat // -------------------------------------------- // @Override - public void postAttach(String id) { + public void postAttach(String id) + { FactionsIndex.get().update(this); } @Override - public void preDetach(String id) { + public void postDetach(String id) + { + //System.out.println("Detaching: " + id + " : " + this.getFactionId()); FactionsIndex.get().update(this); } @Override - public void preClean() { - if (this.getRank().isLeader()) { + public void preClean() + { + if (this.getRank().isLeader()) + { this.getFaction().promoteNewLeader(); }