From b60e478aec55ad73df0e10b8bcc95e24793807e2 Mon Sep 17 00:00:00 2001 From: nossr50 Date: Fri, 4 Jul 2025 13:06:31 -0700 Subject: [PATCH] Fix parties unintentionally becoming leader-less Fixes #3771 --- Changelog.txt | 1 + .../gmail/nossr50/datatypes/party/Party.java | 6 +- .../com/gmail/nossr50/party/PartyManager.java | 26 +- .../gmail/nossr50/party/PartyManagerTest.java | 222 +++++++++++++++++- 4 files changed, 242 insertions(+), 13 deletions(-) diff --git a/Changelog.txt b/Changelog.txt index a84b47586..3ed7ca030 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,4 +1,5 @@ Version 2.2.040 + Fixed bug where a party leader could leave a party and the party would be left without a party leader Fixed a bug where EcoEnchants or other similar plugins could cause an infinite loop within mcMMO (Codebase) Updated Adventure Libs Added 'Happy_Ghast' to experience.yml for combat XP diff --git a/src/main/java/com/gmail/nossr50/datatypes/party/Party.java b/src/main/java/com/gmail/nossr50/datatypes/party/Party.java index e62ce449c..7da69df21 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/party/Party.java +++ b/src/main/java/com/gmail/nossr50/datatypes/party/Party.java @@ -93,7 +93,7 @@ public class Party { public List getVisibleMembers(Player player) { ArrayList visibleMembers = new ArrayList<>(); - for (Player p : onlineMembers) { + for (Player p : getOnlineMembers()) { if (player.canSee(p)) { visibleMembers.add(p); } @@ -116,11 +116,11 @@ public class Party { } public boolean addOnlineMember(Player player) { - return onlineMembers.add(player); + return getOnlineMembers().add(player); } public boolean removeOnlineMember(Player player) { - return onlineMembers.remove(player); + return getOnlineMembers().remove(player); } public String getName() { diff --git a/src/main/java/com/gmail/nossr50/party/PartyManager.java b/src/main/java/com/gmail/nossr50/party/PartyManager.java index 2dcf2188d..53e355cef 100644 --- a/src/main/java/com/gmail/nossr50/party/PartyManager.java +++ b/src/main/java/com/gmail/nossr50/party/PartyManager.java @@ -273,10 +273,25 @@ public final class PartyManager { requireNonNull(player, "player cannot be null!"); requireNonNull(party, "party cannot be null!"); - LinkedHashMap members = party.getMembers(); - String playerName = player.getName(); + final LinkedHashMap members = party.getMembers(); + final String playerName = player.getName(); - members.remove(player.getUniqueId()); + if (party.getLeader().getUniqueId().equals(player.getUniqueId())) { + members.remove(player.getUniqueId()); + if (!members.isEmpty()) { + for (Entry entry : members.entrySet()) { + final UUID memberUUID = entry.getKey(); + final String memberName = entry.getValue(); + if (!memberUUID.equals(party.getLeader().getUniqueId())) { + party.setLeader(new PartyLeader(memberUUID, memberName)); + break; + } + } + } + + } else { + members.remove(player.getUniqueId()); + } if (player.isOnline()) { party.getOnlineMembers().remove(player.getPlayer()); @@ -285,11 +300,6 @@ public final class PartyManager { if (members.isEmpty()) { parties.remove(party); } else { - // If the leaving player was the party leader, appoint a new leader from the party members - if (party.getLeader().getUniqueId().equals(player.getUniqueId())) { - setPartyLeader(members.keySet().iterator().next(), party); - } - informPartyMembersQuit(party, playerName); } } diff --git a/src/test/java/com/gmail/nossr50/party/PartyManagerTest.java b/src/test/java/com/gmail/nossr50/party/PartyManagerTest.java index e009dfb11..2cb0e426b 100644 --- a/src/test/java/com/gmail/nossr50/party/PartyManagerTest.java +++ b/src/test/java/com/gmail/nossr50/party/PartyManagerTest.java @@ -1,15 +1,30 @@ package com.gmail.nossr50.party; import static java.util.logging.Logger.getLogger; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.contains; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.gmail.nossr50.MMOTestEnvironment; +import com.gmail.nossr50.datatypes.interactions.NotificationType; +import com.gmail.nossr50.datatypes.party.Party; +import com.gmail.nossr50.datatypes.party.PartyLeader; import com.gmail.nossr50.datatypes.player.McMMOPlayer; import com.gmail.nossr50.mcMMO; +import com.gmail.nossr50.util.player.NotificationManager; +import com.gmail.nossr50.util.player.UserManager; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.logging.Logger; +import org.bukkit.OfflinePlayer; import org.bukkit.entity.Player; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -24,7 +39,7 @@ class PartyManagerTest extends MMOTestEnvironment { mockBaseEnvironment(logger); // currently unnecessary, but may be needed for future tests - Mockito.when(partyConfig.isPartyEnabled()).thenReturn(true); + when(partyConfig.isPartyEnabled()).thenReturn(true); } @AfterEach @@ -32,7 +47,7 @@ class PartyManagerTest extends MMOTestEnvironment { cleanUpStaticMocks(); // disable parties in config for other tests - Mockito.when(partyConfig.isPartyEnabled()).thenReturn(false); + when(partyConfig.isPartyEnabled()).thenReturn(false); } @Test @@ -94,4 +109,207 @@ class PartyManagerTest extends MMOTestEnvironment { () -> partyManager.createParty(null, partyName, partyPassword)); } + @Test + public void checkPartyPasswordFailsWithIncorrectPassword() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + Party party = Mockito.mock(Party.class); + Player player = Mockito.mock(Player.class); + + when(party.isLocked()).thenReturn(true); + when(party.getPassword()).thenReturn("correctPassword"); + + boolean result = partyManager.checkPartyPassword(player, party, "wrongPassword"); + + assertThat(result).isFalse(); + verify(player).sendMessage(contains("Party password is incorrect")); + } + + @Test + public void checkPartyPasswordFailsWithNullInput() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + Party party = Mockito.mock(Party.class); + Player player = Mockito.mock(Player.class); + + when(party.isLocked()).thenReturn(true); + when(party.getPassword()).thenReturn("secure"); + + boolean result = partyManager.checkPartyPassword(player, party, null); + + assertThat(result).isFalse(); + verify(player).sendMessage( + contains("This party is password protected. Please provide a password to join.")); + } + + @Test + public void checkPartyExistenceReturnsTrueIfExists() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + Party party = Mockito.mock(Party.class); + Mockito.when(party.getName()).thenReturn("ExistingParty"); + + partyManager.getParties().add(party); + + boolean result = partyManager.checkPartyExistence(player, "ExistingParty"); + + assertThat(result).isTrue(); + Mockito.verify(player).sendMessage(Mockito.contains("Party ExistingParty already exists!")); + } + + @Test + public void inSamePartyShouldReturnTrueIfSameParty() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + Party party = Mockito.mock(Party.class); + Player playerA = mock(Player.class); + Player playerB = mock(Player.class); + + McMMOPlayer mmoA = mock(McMMOPlayer.class); + McMMOPlayer mmoB = mock(McMMOPlayer.class); + + mockedUserManager.when(() -> UserManager.getPlayer(playerA)).thenReturn(mmoA); + mockedUserManager.when(() -> UserManager.getPlayer(playerB)).thenReturn(mmoB); + + when(mmoA.getParty()).thenReturn(party); + when(mmoB.getParty()).thenReturn(party); + + assertThat(partyManager.inSameParty(playerA, playerB)).isTrue(); + } + + @Test + public void areAlliesShouldReturnTrueIfMutuallyAllied() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + Player p1 = mock(Player.class); + Player p2 = mock(Player.class); + + McMMOPlayer mmo1 = mock(McMMOPlayer.class); + McMMOPlayer mmo2 = mock(McMMOPlayer.class); + + Party party1 = mock(Party.class); + Party party2 = mock(Party.class); + + mockedUserManager.when(() -> UserManager.getPlayer(p1)).thenReturn(mmo1); + mockedUserManager.when(() -> UserManager.getPlayer(p2)).thenReturn(mmo2); + + when(mmo1.getParty()).thenReturn(party1); + when(mmo2.getParty()).thenReturn(party2); + when(party1.getAlly()).thenReturn(party2); + when(party2.getAlly()).thenReturn(party1); + + assertTrue(partyManager.areAllies(p1, p2)); + } + + @Test + public void removeFromPartyDoesNothing() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + McMMOPlayer mmoPlayer = mock(McMMOPlayer.class); + when(mmoPlayer.getParty()).thenReturn(null); + + partyManager.removeFromParty(mmoPlayer); + } + + @Test + public void removeFromPartyWithPartyRemovesCorrectly() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + McMMOPlayer mmoPlayer = mock(McMMOPlayer.class); + Player player = mock(Player.class); + Party party = mock(Party.class); + UUID uuid = UUID.randomUUID(); + + when(player.getUniqueId()).thenReturn(uuid); + when(player.getName()).thenReturn("PlayerName"); + when(player.isOnline()).thenReturn(true); + when(player.getPlayer()).thenReturn(player); + + when(mmoPlayer.getPlayer()).thenReturn(player); + when(mmoPlayer.getParty()).thenReturn(party); + + when(party.getMembers()).thenReturn(new LinkedHashMap<>(Map.of(uuid, "PlayerName"))); + when(party.getOnlineMembers()).thenReturn(new ArrayList<>(List.of(player))); + when(party.getLeader()).thenReturn(new PartyLeader(uuid, "PlayerName")); + + partyManager.getParties().add(party); + partyManager.removeFromParty(mmoPlayer); + + // Party should be removed since it had only one member + assertFalse(partyManager.getParties().contains(party)); + } + + @Test + public void changeOrJoinPartyNotInPartyTriggersEventAndReturnsTrue() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + McMMOPlayer mmoPlayer = mock(McMMOPlayer.class); + Player player = mock(Player.class); + + when(mmoPlayer.getPlayer()).thenReturn(player); + when(mmoPlayer.inParty()).thenReturn(false); + + assertTrue(partyManager.changeOrJoinParty(mmoPlayer, "NewParty")); + } + + @Test + public void removeFromPartyLeaderLeavesNewLeaderIsAssigned() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + UUID oldLeaderUUID = UUID.randomUUID(); + UUID newLeaderUUID = UUID.randomUUID(); + + // Setup players + OfflinePlayer oldLeader = mock(OfflinePlayer.class); + when(oldLeader.getUniqueId()).thenReturn(oldLeaderUUID); + when(oldLeader.getName()).thenReturn("OldLeader"); + when(oldLeader.isOnline()).thenReturn(true); + when(oldLeader.getPlayer()).thenReturn( + mock(Player.class)); // required for party.getOnlineMembers() + + OfflinePlayer newLeader = mock(OfflinePlayer.class); + when(newLeader.getUniqueId()).thenReturn(newLeaderUUID); + when(newLeader.getName()).thenReturn("NewLeader"); + + // Setup party and members + Party party = new Party(new PartyLeader(oldLeaderUUID, "OldLeader"), "SomeParty", null); + party.getMembers().put(oldLeaderUUID, "OldLeader"); + party.getMembers().put(newLeaderUUID, "NewLeader"); + + Player newLeaderOnline = mock(Player.class); + when(newLeaderOnline.getUniqueId()).thenReturn(newLeaderUUID); + party.getOnlineMembers().add(newLeaderOnline); // simulate second member online + + partyManager.getParties().add(party); + + // Act + partyManager.removeFromParty(oldLeader, party); + + // Assert + PartyLeader newLeaderObj = party.getLeader(); + assertThat(newLeaderUUID).isEqualTo(newLeaderObj.getUniqueId()); + assertThat("NewLeader").isEqualTo(newLeaderObj.getPlayerName()); + } + + @Test + public void joinInvitedPartyPartyDoesNotExistDoesNotJoin() { + PartyManager partyManager = new PartyManager(mcMMO.p); + + McMMOPlayer mmoPlayer = mock(McMMOPlayer.class); + Player player = mock(Player.class); + Party partyWhichNoLongerExists = mock(Party.class); + + when(mmoPlayer.getPartyInvite()).thenReturn(partyWhichNoLongerExists); + when(mmoPlayer.getPlayer()).thenReturn(player); + + assertFalse(partyManager.getParties().contains(partyWhichNoLongerExists)); + + partyManager.joinInvitedParty(mmoPlayer); + + // Should have sent disband message + notificationManager.verify(() -> + NotificationManager.sendPlayerInformation(player, NotificationType.PARTY_MESSAGE, + "Party.Disband")); + } + } \ No newline at end of file