From f60e003fcdcab01c2f1249243e48c3f7d2963b29 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:36:04 +0000 Subject: [PATCH 1/2] fix: refresh realm trade when bankruptcy changes --- .../managers/ProvinceManager.java | 10 +++++- .../simplefactions/map/MapSystem.java | 1 + .../managers/ProvinceEconomyCoverageTest.java | 32 +++++++++++++++++++ .../map/MapPublicationCoverageTest.java | 12 +++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java index 5a580c5e..125db381 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java @@ -48,16 +48,24 @@ public class ProvinceManager { private Map> installationAccess = new HashMap<>(); private long stateVersion = 0; private long lastCalculatedVersion = -1; + private Map lastCalculatedBankruptcy = Map.of(); public void markDirty() { stateVersion++; } public void recalculateIfNeeded() { - if (stateVersion == lastCalculatedVersion) return; + if (!Cache.provincesEnabled) return; + // Realm treasury sign changes stability even when no map mutation marks us dirty. + Map bankruptcy = new HashMap<>(); + for (Guild guild : FactionManager.getAllGuilds()) { + if (guild.isBase()) bankruptcy.put(guild, guild.isBankrupt()); + } + if (stateVersion == lastCalculatedVersion && bankruptcy.equals(lastCalculatedBankruptcy)) return; recalculate(); lastCalculatedVersion = stateVersion; + lastCalculatedBankruptcy = bankruptcy; } public List getProvinces() { return new ArrayList<>(provinces.values()); } diff --git a/src/main/java/net/tfminecraft/simplefactions/map/MapSystem.java b/src/main/java/net/tfminecraft/simplefactions/map/MapSystem.java index 4aa51b88..eef382ca 100644 --- a/src/main/java/net/tfminecraft/simplefactions/map/MapSystem.java +++ b/src/main/java/net/tfminecraft/simplefactions/map/MapSystem.java @@ -85,6 +85,7 @@ public void prepareUploadFiles() { /** Cheap payloads that change continuously and ship on every cycle (main thread). */ public void prepareLiveFiles() { + SimpleFactions.getInstance().getProvinceManager().recalculateIfNeeded(); exportProvinces(); exportGuilds(); exportChronicle(); diff --git a/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java index bb457603..5a1bc1f0 100644 --- a/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java @@ -28,6 +28,7 @@ import net.tfminecraft.simplefactions.map.provinces.ProvinceDataEntry; import net.tfminecraft.simplefactions.objects.Faction; import net.tfminecraft.simplefactions.testsupport.FactionDomainFixture; +import net.tfminecraft.simplefactions.testsupport.TestTreasuries; import net.tfminecraft.simplefactions.war.resolution.PillageTradeHit; import org.bukkit.Bukkit; import org.bukkit.configuration.file.YamlConfiguration; @@ -299,6 +300,36 @@ void dirtyRecalculationReplacesEconomicsOnlyWhenTheModelChanges() { assertEquals(prosperity, guild.getTradeBreakdown().getIncome(), 0.01); } + @Test + void treasuryBankruptcyAndRecoveryInvalidateCachedRealmTrade() { + productiveBranch(); + Faction home = fixture.saved("home", "Alice"); + Guild guild = home.getOrCreateMainGuild(); + Province source = province(1); + home.addProvince(1); + home.setCapital(1, true); + fixture.provincesEnabled(true); + try (TestTreasuries treasury = new TestTreasuries()) { + TestTreasuries.set(guild.getBank(), 100); + manager.recalculateIfNeeded(); + assertTrue(source.getStoredGuildTrade(guild) > 0); + ProvinceDataEntry solvent = source.getData(guild.getId()); + TestTreasuries.set(guild.getBank(), 90); + manager.recalculateIfNeeded(); + assertSame(solvent, source.getData(guild.getId())); + TestTreasuries.set(guild.getBank(), -11.23); + manager.recalculateIfNeeded(); + assertEquals(0, source.getStoredGuildTrade(guild)); + assertEquals(0, guild.getTradeBreakdown().getIncome()); + assertEquals(-11.23, guild.getBank().getWealth(), 0.001); + TestTreasuries.set(guild.getBank(), 0); + manager.recalculateIfNeeded(); + assertTrue(source.getStoredGuildTrade(guild) > 0); + assertTrue(guild.getTradeBreakdown().getIncome() > 0); + assertEquals(0, guild.getBank().getWealth()); + } + } + @Test void disabledIncomeQueriesAndRecalculationRespectWhetherSavingWasRequested() { productiveBranch(); @@ -310,6 +341,7 @@ void disabledIncomeQueriesAndRecalculationRespectWhetherSavingWasRequested() { source.setData(guild.getId(), new ProvinceDataEntry(guild, 10, 3)); source.setProsperity(40); guild.getTradeBreakdown().setIncome(7); + manager.recalculateIfNeeded(); manager.recalculate(); manager.recalculateQuiet(null, null); manager.recalculateForSingleGuild(guild, false); diff --git a/src/test/java/net/tfminecraft/simplefactions/map/MapPublicationCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/map/MapPublicationCoverageTest.java index db94170a..f98d953f 100644 --- a/src/test/java/net/tfminecraft/simplefactions/map/MapPublicationCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/map/MapPublicationCoverageTest.java @@ -92,6 +92,18 @@ private void completeUpload() { fixture.ui.runTasks(); } + @Test + void liveExportsRefreshEconomicsBeforeReadingPayloads() { + doAnswer(call -> { + assertFalse(Files.exists(files.root.resolve("MapAPI/chronicle.json"))); + assertFalse(Files.exists(files.root.resolve("MapAPI/province_data.json"))); + return null; + }).when(fixture.provinces).recalculateIfNeeded(); + map.prepareLiveFiles(); + verify(fixture.provinces).recalculateIfNeeded(); + assertTrue(Files.exists(files.root.resolve("MapAPI/chronicle.json"))); + } + @Test void mutationsDuringAnUploadRemainQueuedForTheNextPublication() throws Exception { map.enqueue("nation", "first"); From 0335990b09f9c462ea84e2ef092351292cbc8422 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 22 Oct 2026 07:30:39 +0000 Subject: [PATCH 2/2] fix: keep bankruptcy cache consistent across recalculation paths --- .../managers/ProvinceManager.java | 17 +++++-- .../managers/ProvinceEconomyCoverageTest.java | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java index 125db381..b16403e5 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java @@ -57,15 +57,16 @@ public void markDirty() { public void recalculateIfNeeded() { if (!Cache.provincesEnabled) return; // Realm treasury sign changes stability even when no map mutation marks us dirty. + if (stateVersion == lastCalculatedVersion && realmBankruptcy().equals(lastCalculatedBankruptcy)) return; + recalculate(); + } + + private Map realmBankruptcy() { Map bankruptcy = new HashMap<>(); for (Guild guild : FactionManager.getAllGuilds()) { if (guild.isBase()) bankruptcy.put(guild, guild.isBankrupt()); } - if (stateVersion == lastCalculatedVersion && bankruptcy.equals(lastCalculatedBankruptcy)) return; - - recalculate(); - lastCalculatedVersion = stateVersion; - lastCalculatedBankruptcy = bankruptcy; + return bankruptcy; } public List getProvinces() { return new ArrayList<>(provinces.values()); } @@ -124,6 +125,8 @@ public void recalculate() { if (!Cache.provincesEnabled) { return; } + // A partial or failed flood must not leave the old full snapshot marked fresh. + lastCalculatedVersion = -1; InstallationAccess.beginRecalculation(); BankruptcyMemo.Run memo = BankruptcyMemo.begin(); try { @@ -139,6 +142,8 @@ public void recalculate() { } recalculateProsperity(); for(Guild guild : FactionManager.getAllGuilds()) getIncome(guild); + lastCalculatedBankruptcy = realmBankruptcy(); + lastCalculatedVersion = stateVersion; } finally { BankruptcyMemo.end(memo); InstallationAccess.endRecalculation(); @@ -188,6 +193,8 @@ public void recalculateForSingleGuild(Guild g, boolean save) { if (!Cache.provincesEnabled) { return; } + // A partial or failed flood must not leave the old full snapshot marked fresh. + lastCalculatedVersion = -1; InstallationAccess.beginRecalculation(); BankruptcyMemo.Run memo = BankruptcyMemo.begin(); try { diff --git a/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java index 5a1bc1f0..eedcd555 100644 --- a/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/managers/ProvinceEconomyCoverageTest.java @@ -330,6 +330,52 @@ void treasuryBankruptcyAndRecoveryInvalidateCachedRealmTrade() { } } + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void recoveryAfterAnUnguardedFloodRefreshesAgain(boolean singleGuild) { + productiveBranch(); + Faction home = fixture.saved("home", "Alice"); + Guild guild = home.getOrCreateMainGuild(); + Province source = province(1); + home.addProvince(1); + home.setCapital(1, true); + fixture.provincesEnabled(true); + try (TestTreasuries treasury = new TestTreasuries()) { + TestTreasuries.set(guild.getBank(), 100); + manager.recalculateIfNeeded(); + TestTreasuries.set(guild.getBank(), -11.23); + if (singleGuild) manager.recalculateForSingleGuild(guild, true); + else manager.recalculate(); + assertEquals(0, source.getStoredGuildTrade(guild)); + TestTreasuries.set(guild.getBank(), 0); + manager.recalculateIfNeeded(); + assertTrue(source.getStoredGuildTrade(guild) > 0); + assertTrue(guild.getTradeBreakdown().getIncome() > 0); + } + } + + @Test + void directFullFloodAlsoUpdatesTheGuardedCache() { + ProvinceManager watched = spy(manager); + productiveBranch(); + Faction home = fixture.saved("home", "Alice"); + Guild guild = home.getOrCreateMainGuild(); + Province source = province(1); + home.addProvince(1); + home.setCapital(1, true); + fixture.provincesEnabled(true); + try (TestTreasuries treasury = new TestTreasuries()) { + TestTreasuries.set(guild.getBank(), 100); + watched.recalculateIfNeeded(); + TestTreasuries.set(guild.getBank(), -11.23); + watched.recalculate(); + assertEquals(0, source.getStoredGuildTrade(guild)); + clearInvocations(watched); + watched.recalculateIfNeeded(); + verify(watched, never()).recalculate(); + } + } + @Test void disabledIncomeQueriesAndRecalculationRespectWhetherSavingWasRequested() { productiveBranch();