From 28968decaf5225786d5b84ad26db2608c8eead21 Mon Sep 17 00:00:00 2001 From: Ryan <7389646+ryanbarlow97@users.noreply.github.com> Date: Sun, 27 Sep 2026 07:31:22 +0000 Subject: [PATCH] fix: keep points spent when a player removes a profession upgrade Free points were lifetime points minus the cost of held upgrades, so removing an upgrade silently gave its points back despite the "Points are not refunded!" warning. Player removals now record the cost as forfeited per character, which keeps it spent. Forfeits don't count towards the max-upgrades cap. Admin removeupgrade still refunds, and the admin refund command clears forfeits. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../rpcharacters/database/Database.java | 12 +++ .../rpcharacters/objects/RPCharacter.java | 37 +++++++- .../professions/ProfessionCommandHandler.java | 21 ++++- .../ProfessionUpgradeForfeitTest.java | 86 +++++++++++++++++++ 4 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 src/test/java/net/tfminecraft/rpcharacters/professions/ProfessionUpgradeForfeitTest.java diff --git a/src/main/java/net/tfminecraft/rpcharacters/database/Database.java b/src/main/java/net/tfminecraft/rpcharacters/database/Database.java index 16ad8ee..25ffc34 100644 --- a/src/main/java/net/tfminecraft/rpcharacters/database/Database.java +++ b/src/main/java/net/tfminecraft/rpcharacters/database/Database.java @@ -910,6 +910,13 @@ private JSONObject toAccountProfessionPointsJson(Map points) { } private void loadProfessionFields(RPCharacter character, JSONObject characterJson) { + if (characterJson.get("forfeited-profession-points") instanceof JSONObject forfeitedJson) { + for (Object key : forfeitedJson.keySet()) { + if (forfeitedJson.get(key) instanceof Number amount) { + character.addForfeitedProfessionPoints(key.toString(), amount.intValue()); + } + } + } if (!characterJson.containsKey("profession-upgrades")) { return; } @@ -923,6 +930,11 @@ private void loadProfessionFields(RPCharacter character, JSONObject characterJso @SuppressWarnings("unchecked") private void saveProfessionFields(HashMap defaults, RPCharacter character) { + if (!character.getForfeitedProfessionPoints().isEmpty()) { + JSONObject forfeitedJson = new JSONObject(); + forfeitedJson.putAll(character.getForfeitedProfessionPoints()); + defaults.put("forfeited-profession-points", forfeitedJson); + } if (character.getProfessionUpgrades().isEmpty()) { return; } diff --git a/src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java b/src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java index 4b5e82e..614ffff 100644 --- a/src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java +++ b/src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java @@ -84,6 +84,8 @@ public class RPCharacter { private String birthday; private final Set professionUpgrades = new LinkedHashSet<>(); + /** Points lost by removing upgrades, per lowercase profession id. They stay spent. */ + private final Map forfeitedProfessionPoints = new HashMap<>(); private final Map extraAttributeAllocation = new HashMap<>(); private String lastLocationWorld; @@ -868,8 +870,41 @@ public List resolveProfessionUpgrades() { return resolved; } + public Map getForfeitedProfessionPoints() { + return Collections.unmodifiableMap(forfeitedProfessionPoints); + } + + public void setForfeitedProfessionPoints(Map points) { + forfeitedProfessionPoints.clear(); + if (points != null) { + for (Map.Entry entry : points.entrySet()) { + addForfeitedProfessionPoints(entry.getKey(), entry.getValue() != null ? entry.getValue() : 0); + } + } + } + + public void addForfeitedProfessionPoints(String professionId, int amount) { + if (professionId == null || professionId.isBlank() || amount <= 0) { + return; + } + forfeitedProfessionPoints.merge(professionId.toLowerCase(), amount, Integer::sum); + } + + public void clearForfeitedProfessionPoints() { + forfeitedProfessionPoints.clear(); + } + + /** Removes a held upgrade without giving its cost back to the profession's free points. */ + public void forfeitProfessionUpgrade(ProfessionUpgradeDefinition upgrade) { + if (upgrade == null || !professionUpgrades.remove(upgrade.getId())) { + return; + } + addForfeitedProfessionPoints(upgrade.getProfessionId(), upgrade.getCost()); + } + + /** Held upgrade costs plus forfeited points; what the profession's lifetime points pay for. */ public int getSpentPointsOnProfession(String professionId) { - int spent = 0; + int spent = professionId != null ? forfeitedProfessionPoints.getOrDefault(professionId.toLowerCase(), 0) : 0; for (ProfessionUpgradeDefinition upgrade : resolveProfessionUpgrades()) { if (upgrade.getProfessionId().equalsIgnoreCase(professionId)) { spent += upgrade.getCost(); diff --git a/src/main/java/net/tfminecraft/rpcharacters/professions/ProfessionCommandHandler.java b/src/main/java/net/tfminecraft/rpcharacters/professions/ProfessionCommandHandler.java index 4d77f0c..6519f07 100644 --- a/src/main/java/net/tfminecraft/rpcharacters/professions/ProfessionCommandHandler.java +++ b/src/main/java/net/tfminecraft/rpcharacters/professions/ProfessionCommandHandler.java @@ -104,7 +104,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St RPTexts.send(sender, RPTexts.ERROR + "Invalid player or upgrade."); return true; } - removeUpgradeFromActiveCharacter(target, upgrade); + removeUpgradeFromActiveCharacter(target, upgrade, false); RPTexts.send(sender, RPTexts.ERROR + "Removed upgrade " + upgrade.getId() + " from " + target.getName()); return true; } @@ -176,7 +176,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St RPTexts.send(player, RPTexts.ERROR + "Nothing to confirm."); return true; } - removeUpgradeFromActiveCharacter(player, upgrade); + removeUpgradeFromActiveCharacter(player, upgrade, true); RPTexts.send(player, RPTexts.ERROR + "Lost the " + RPTexts.WARN + upgrade.getMenuItem().getItemMeta().getDisplayName() + RPTexts.ERROR + " upgrade!"); return true; @@ -207,7 +207,12 @@ public static void reapplyActiveCharacterPerms() { } } - public static void removeUpgradeFromActiveCharacter(Player player, ProfessionUpgradeDefinition upgrade) { + /** + * @param forfeitPoints true for a player's own removal, which keeps the upgrade's cost spent so + * removing and re-buying cannot be used to respec + */ + public static void removeUpgradeFromActiveCharacter(Player player, ProfessionUpgradeDefinition upgrade, + boolean forfeitPoints) { PlayerData pd = PlayerManager.get(player); if (pd == null) { return; @@ -217,7 +222,11 @@ public static void removeUpgradeFromActiveCharacter(Player player, ProfessionUpg return; } ProfessionIntegrator.removeUpgrade(player, upgrade); - character.removeProfessionUpgrade(upgrade.getId()); + if (forfeitPoints) { + character.forfeitProfessionUpgrade(upgrade); + } else { + character.removeProfessionUpgrade(upgrade.getId()); + } RPCharacters.getPlayerManager().savePlayer(player); } @@ -248,6 +257,10 @@ public static void refundActiveCharacter(Player player) { resetActiveCharacterUpgrades(player, true); PlayerData pd = PlayerManager.get(player); if (pd != null) { + RPCharacter character = pd.getActiveCharacter(); + if (character != null) { + character.clearForfeitedProfessionPoints(); + } pd.clearAccountProfessionPoints(); pd.setProfessionPointsInitialized(false); } diff --git a/src/test/java/net/tfminecraft/rpcharacters/professions/ProfessionUpgradeForfeitTest.java b/src/test/java/net/tfminecraft/rpcharacters/professions/ProfessionUpgradeForfeitTest.java new file mode 100644 index 0000000..6f969e7 --- /dev/null +++ b/src/test/java/net/tfminecraft/rpcharacters/professions/ProfessionUpgradeForfeitTest.java @@ -0,0 +1,86 @@ +package net.tfminecraft.rpcharacters.professions; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import net.tfminecraft.rpcharacters.objects.RPCharacter; + +class ProfessionUpgradeForfeitTest { + + private final ProfessionUpgradeDefinition apprentice = upgrade("smith_1", "Blacksmith", 3); + private final ProfessionUpgradeDefinition journeyman = upgrade("smith_2", "Blacksmith", 5); + + @BeforeEach + void registerUpgrades() { + ProfessionRegistry.setUpgrades(List.of(apprentice, journeyman)); + } + + @AfterEach + void clearRegistry() { + ProfessionRegistry.clear(); + } + + @Test + void aRemovedUpgradeStaysSpentOnItsProfession() { + RPCharacter character = new RPCharacter(null); + character.addProfessionUpgrade(apprentice.getId()); + character.addProfessionUpgrade(journeyman.getId()); + + character.forfeitProfessionUpgrade(journeyman); + + assertFalse(character.hasProfessionUpgrade(journeyman.getId())); + assertEquals(8, character.getSpentPointsOnProfession("blacksmith")); + assertEquals(3, character.getTotalSpentPoints(), "forfeits must not count towards the upgrade cap"); + } + + @Test + void rebuyingAForfeitedUpgradeCostsItsPointsAgain() { + RPCharacter character = new RPCharacter(null); + character.addProfessionUpgrade(apprentice.getId()); + character.forfeitProfessionUpgrade(apprentice); + character.addProfessionUpgrade(apprentice.getId()); + + assertEquals(6, character.getSpentPointsOnProfession("Blacksmith")); + } + + @Test + void forfeitingAnUpgradeThatIsNotHeldCostsNothing() { + RPCharacter character = new RPCharacter(null); + + character.forfeitProfessionUpgrade(apprentice); + + assertEquals(0, character.getSpentPointsOnProfession("blacksmith")); + assertEquals(Map.of(), character.getForfeitedProfessionPoints()); + } + + @Test + void anAdminRemovalStillGivesThePointsBack() { + RPCharacter character = new RPCharacter(null); + character.addProfessionUpgrade(apprentice.getId()); + + character.removeProfessionUpgrade(apprentice.getId()); + + assertEquals(0, character.getSpentPointsOnProfession("blacksmith")); + } + + @Test + void clearingForfeitsRestoresTheFullRefund() { + RPCharacter character = new RPCharacter(null); + character.setForfeitedProfessionPoints(Map.of("Blacksmith", 4, "chef", 0)); + + assertEquals(Map.of("blacksmith", 4), character.getForfeitedProfessionPoints()); + character.clearForfeitedProfessionPoints(); + assertEquals(0, character.getSpentPointsOnProfession("blacksmith")); + } + + private static ProfessionUpgradeDefinition upgrade(String id, String professionId, int cost) { + return new ProfessionUpgradeDefinition(id, professionId, null, cost, "perk", List.of(), List.of()); + } +}