diff --git a/src/main/java/net/tfminecraft/simplefactions/economy/FrozenReads.java b/src/main/java/net/tfminecraft/simplefactions/economy/FrozenReads.java index c96133d7..3a6653f9 100644 --- a/src/main/java/net/tfminecraft/simplefactions/economy/FrozenReads.java +++ b/src/main/java/net/tfminecraft/simplefactions/economy/FrozenReads.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.function.Supplier; @@ -11,14 +12,15 @@ import net.tfminecraft.simplefactions.government.StabilityModifier; import net.tfminecraft.simplefactions.guild.Guild; import net.tfminecraft.simplefactions.guild.income.Ledger; +import net.tfminecraft.simplefactions.inactivity.InactivityService; import net.tfminecraft.simplefactions.managers.FactionManager; import net.tfminecraft.simplefactions.objects.Bank; import net.tfminecraft.simplefactions.objects.Faction; /** - * Guild treasuries, held tax, node upkeep and espionage stability read once on the main thread. - * DenarEconomy, Dowsing and TLibs character identity refuse reads from other threads, so - * income previews that run async read these instead. + * Guild treasuries, held tax, node upkeep, espionage stability and inactive members read once + * on the main thread. DenarEconomy, Dowsing and TLibs character identity refuse reads from + * other threads, so income previews that run async read these instead. */ public final class FrozenReads { private static final ThreadLocal CURRENT = new ThreadLocal<>(); @@ -27,17 +29,23 @@ public final class FrozenReads { private final Map> taxContributors; private final Map nodeUpkeep; private final Map> espionage; + private final Set inactive; private FrozenReads( Map cents, Map> taxContributors, - Map nodeUpkeep, Map> espionage) { + Map nodeUpkeep, Map> espionage, + Set inactive) { this.cents = Map.copyOf(cents); this.taxContributors = Map.copyOf(taxContributors); this.nodeUpkeep = Map.copyOf(nodeUpkeep); this.espionage = Map.copyOf(espionage); + this.inactive = Set.copyOf(inactive); } - /** Reads guild treasuries, held tax and node upkeep and realm espionage modifiers now. Call on the main thread. */ + /** + * Reads guild treasuries, held tax, node upkeep, realm espionage modifiers and inactive + * members now. Call on the main thread. + */ public static FrozenReads capture() { long now = System.currentTimeMillis(); Map> espionage = new HashMap<>(); @@ -68,7 +76,9 @@ public static FrozenReads capture() { nodeUpkeep.put(incarnation, Ledger.liveNodeUpkeep(guild)); } } - return new FrozenReads(cents, taxContributors, nodeUpkeep, espionage); + // Member shares (stability, upkeep) skip inactive members; whether one is online asks TLibs. + Set inactive = InactivityService.captureInactiveMembers(); + return new FrozenReads(cents, taxContributors, nodeUpkeep, espionage, inactive); } /** Runs the work with these reads standing in for the live state on this thread. */ @@ -113,6 +123,12 @@ public static double nodeUpkeep(Guild guild) { return Ledger.liveNodeUpkeep(guild); } + /** Inside {@link #read}, the members who were inactive at capture; otherwise null, so callers check live. */ + public static Set inactiveMembers() { + FrozenReads current = CURRENT.get(); + return current == null ? null : current.inactive; + } + /** The frozen espionage modifiers inside {@link #read}, otherwise the live ones at {@code now}. */ public static List espionage(Faction faction, long now) { FrozenReads current = CURRENT.get(); diff --git a/src/main/java/net/tfminecraft/simplefactions/inactivity/InactivityService.java b/src/main/java/net/tfminecraft/simplefactions/inactivity/InactivityService.java index 4b25af29..2dbbdbfc 100644 --- a/src/main/java/net/tfminecraft/simplefactions/inactivity/InactivityService.java +++ b/src/main/java/net/tfminecraft/simplefactions/inactivity/InactivityService.java @@ -4,8 +4,10 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.logging.Level; import org.bukkit.Bukkit; @@ -15,6 +17,7 @@ import net.tfminecraft.simplefactions.SimpleFactions; import net.tfminecraft.simplefactions.database.Database; import net.tfminecraft.simplefactions.database.JsonUtil; +import net.tfminecraft.simplefactions.economy.FrozenReads; import net.tfminecraft.simplefactions.guild.Guild; import net.tfminecraft.simplefactions.inactivity.DecayClock.Event; import net.tfminecraft.simplefactions.managers.BackgroundRecalculation; @@ -110,11 +113,42 @@ public static int prestigePercent(Faction faction) { public static boolean isMemberInactive(String name) { if (!InactivityRules.isTrackedMember(name)) return false; + // Async income previews: TLibs refuses the online lookup off the server thread, and the + // day counts change on it, so they read the set captured with the preview copy. + Set frozen = FrozenReads.inactiveMembers(); + if (frozen != null) return frozen.contains(name); // The day count is a map read; only a member already past the threshold needs the online lookup. if (!InactivityRules.isInactive(FactionCleanup.daysOffline(name))) return false; return !isOnline(name); } + /** + * Every tracked member inactive right now: past the offline threshold and not playing. + * Server thread only. One pass over the day counts, and one character lookup per online + * player instead of one per member past the threshold. + */ + public static Set captureInactiveMembers() { + List away = FactionCleanup.membersOffline(InactivityRules::isInactive); + if (away.isEmpty()) return Set.of(); + Set online = onlineMemberKeys(); + Set inactive = new HashSet<>(); + for (String member : away) { + if (InactivityRules.isTrackedMember(member) && !online.contains(member)) inactive.add(member); + } + return inactive; + } + + /** The character keys being played now, which is what {@link Members#isOnline} checks per key. */ + private static Set onlineMemberKeys() { + if (Bukkit.getServer() == null) return Set.of(); + Set keys = new HashSet<>(); + for (Player player : Bukkit.getOnlinePlayers()) { + String key = Members.key(player); + if (key != null) keys.add(key); + } + return keys; + } + /** The members who still count. Inactive members stay in their guild but add nothing to counts. */ public static List activeOnly(java.util.Collection members) { List active = new ArrayList<>(); diff --git a/src/main/java/net/tfminecraft/simplefactions/utils/FactionCleanup.java b/src/main/java/net/tfminecraft/simplefactions/utils/FactionCleanup.java index 8c6c5e09..0ab8b8d9 100644 --- a/src/main/java/net/tfminecraft/simplefactions/utils/FactionCleanup.java +++ b/src/main/java/net/tfminecraft/simplefactions/utils/FactionCleanup.java @@ -5,11 +5,13 @@ import java.io.FileWriter; import java.io.IOException; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.IntPredicate; import java.util.logging.Level; import org.bukkit.Bukkit; @@ -97,6 +99,17 @@ public static int daysOffline(String member) { return days == null ? 0 : days; } + /** Members whose offline day count matches, in one pass over the day counts. Server thread. */ + public static List membersOffline(IntPredicate days) { + if (!ensureLoaded()) return List.of(); + List members = new ArrayList<>(); + for (Map.Entry entry : offlineDays.entrySet()) { + Integer count = entry.getValue(); + if (days.test(count == null ? 0 : count)) members.add(entry.getKey()); + } + return members; + } + public static void ping(String member) { if (member == null || member.isBlank()) return; try { diff --git a/src/test/java/net/tfminecraft/simplefactions/inactivity/FrozenInactiveMembersTest.java b/src/test/java/net/tfminecraft/simplefactions/inactivity/FrozenInactiveMembersTest.java new file mode 100644 index 00000000..89790087 --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/inactivity/FrozenInactiveMembersTest.java @@ -0,0 +1,197 @@ +package net.tfminecraft.simplefactions.inactivity; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CompletableFuture; + +import org.bukkit.Bukkit; +import org.bukkit.Server; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import net.tfminecraft.simplefactions.database.JsonUtil; +import net.tfminecraft.simplefactions.economy.FrozenReads; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.testsupport.PersistenceFilesFixture; +import net.tfminecraft.simplefactions.utils.FactionCleanup; +import net.tfminecraft.tlibs.character.CharacterInfo; +import net.tfminecraft.tlibs.character.CharacterSession; + +/** + * Async income previews count member shares (stability, upkeep). Whether an inactive member + * is online asks TLibs, which refuses other threads, so previews read the set captured with + * their copy on the main thread (Main, 11-11: every trade, law and favour estimate failed). + */ +class FrozenInactiveMembersTest { + private static final int AWAY = InactivityRules.DAYS_UNTIL_INACTIVE; + + private final List savedFactions = new ArrayList<>(); + private PersistenceFilesFixture disk; + private Field history; + private Object savedHistory; + private Characters characters; + + @BeforeEach + void setUp() throws Exception { + disk = new PersistenceFilesFixture(); + history = FactionCleanup.class.getDeclaredField("offlineDays"); + history.setAccessible(true); + savedHistory = history.get(null); + // No realms, so capture reads no treasuries and only the inactive members matter here. + savedFactions.addAll(FactionManager.factions); + FactionManager.factions.clear(); + characters = new Characters(); + Members.setResolver(characters); + } + + @AfterEach + void tearDown() throws Exception { + try { + Members.setResolver(null); + FactionManager.factions.clear(); + FactionManager.factions.addAll(savedFactions); + history.set(null, savedHistory); + } finally { + disk.close(); + } + } + + private void days(Map days) throws Exception { + disk.write("Cache/logins.json", JsonUtil.GSON.toJson(days)); + history.set(null, null); + } + + private static MockedStatic server(Player... online) { + MockedStatic bukkit = mockStatic(Bukkit.class); + Server server = mock(Server.class); + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getOnlinePlayers).thenAnswer(call -> List.of(online)); + return bukkit; + } + + @Test + void previewsReadTheInactiveMembersCapturedOnTheMainThread() throws Exception { + days(Map.of("bob", AWAY, "carl", AWAY + 10, "dana", AWAY - 1, "dummy_ghost", AWAY + 40)); + Player carl = mock(Player.class); + characters.playing.put(carl, "carl"); + List realm = List.of("alice", "bob", "carl", "dana", "dummy_ghost"); + + FrozenReads frozen; + try (var bukkit = server(carl)) { + frozen = FrozenReads.capture(); + // The copy answers as the live check does for each kind of member. + for (String member : List.of("bob", "carl", "dana", "dummy_ghost", "erin")) { + assertEquals(InactivityService.isMemberInactive(member), + frozen.read(() -> InactivityService.isMemberInactive(member)), member); + } + + // Off the main thread TLibs refuses the online lookup, as it did on Main. + characters.refuse = true; + assertThrows(IllegalStateException.class, () -> InactivityService.isMemberInactive("bob")); + + assertTrue(frozen.read(() -> InactivityService.isMemberInactive("bob"))); + assertFalse(frozen.read(() -> InactivityService.isMemberInactive("carl")), "online at capture"); + assertFalse(frozen.read(() -> InactivityService.isMemberInactive("dana")), "under the threshold"); + assertFalse(frozen.read(() -> InactivityService.isMemberInactive("dummy_ghost")), "not tracked"); + assertFalse(frozen.read(() -> InactivityService.isMemberInactive("erin")), "never counted"); + assertFalse(frozen.read(() -> InactivityService.isMemberInactive(null))); + } + + // Day counts written after the copy belong to the next preview, not this one. + days(Map.of()); + List active = List.of("alice", "carl", "dana", "dummy_ghost"); + assertEquals(active, CompletableFuture.supplyAsync(() -> frozen.read(() -> InactivityService.activeOnly(realm))).get()); + assertFalse(InactivityService.isMemberInactive("bob")); + assertNull(FrozenReads.inactiveMembers()); + } + + @Test + void noOneAwayNeedsNoCharacterLookup() throws Exception { + days(Map.of("bob", AWAY - 1)); + characters.refuse = true; + try (var bukkit = server(mock(Player.class))) { + assertEquals(Set.of(), InactivityService.captureInactiveMembers()); + } + } + + @Test + void playersWithoutACharacterAreNobodyOnline() throws Exception { + days(Map.of("bob", AWAY)); + try (var bukkit = server(mock(Player.class))) { + assertEquals(Set.of("bob"), InactivityService.captureInactiveMembers()); + } + } + + @Test + void withoutAServerNobodyIsOnline() throws Exception { + // Raw JSON: Gson leaves null entries out when it writes. + disk.write("Cache/logins.json", "{\"bob\":" + AWAY + ",\"carl\":null}"); + history.set(null, null); + assertEquals(List.of("carl"), FactionCleanup.membersOffline(days -> days == 0)); + assertEquals(Set.of("bob"), InactivityService.captureInactiveMembers()); + } + + @Test + void unreadableDayCountsMarkNobodyInactive() throws Exception { + disk.write("Cache/logins.json", "{"); + history.set(null, null); + assertEquals(List.of(), FactionCleanup.membersOffline(days -> true)); + assertEquals(Set.of(), InactivityService.captureInactiveMembers()); + } + + /** Plays the given characters and, once told to, refuses lookups like TLibs off the main thread. */ + private static final class Characters implements Members.Resolver { + private final Map playing = new HashMap<>(); + private volatile boolean refuse; + + private void guard() { + if (refuse) throw new IllegalStateException("Character identity must be used on the main thread"); + } + + @Override + public String key(Player player) { + guard(); + return playing.get(player); + } + + @Override + public Optional online(String key) { + guard(); + return playing.entrySet().stream().filter(entry -> entry.getValue().equals(key)).map(Map.Entry::getKey).findFirst(); + } + + @Override + public Optional info(String key) { + guard(); + return Optional.empty(); + } + + @Override + public Optional session(Player player) { + return Optional.empty(); + } + + @Override + public boolean isCurrent(CharacterSession session) { + return false; + } + } +}