From d8c52da2c59af716780b682bbc91f98a1c07049e Mon Sep 17 00:00:00 2001 From: psikomonkie <189469115+psikomonkie@users.noreply.github.com> Date: Wed, 4 Dec 2024 13:01:37 -0500 Subject: [PATCH 1/3] Issue 5256: Refactored MekHQ Unit's Gunners to be Set to ensure gunners are unique --- MekHQ/src/mekhq/campaign/unit/Unit.java | 24 ++++++++++--------- .../unit/actions/HirePersonnelUnitAction.java | 4 ++-- .../mekhq/campaign/unit/UnitPersonTest.java | 3 ++- .../mekhq/gui/model/UnitTableModelTest.java | 6 +++-- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/MekHQ/src/mekhq/campaign/unit/Unit.java b/MekHQ/src/mekhq/campaign/unit/Unit.java index 2eb4e33569..8ff838fdee 100644 --- a/MekHQ/src/mekhq/campaign/unit/Unit.java +++ b/MekHQ/src/mekhq/campaign/unit/Unit.java @@ -125,7 +125,7 @@ public class Unit implements ITechnology { protected int scenarioId; private List drivers; - private List gunners; + private Set gunners; private List vesselCrew; // Contains unique Id of each Infantry/BA Entity assigned to this unit as // marines @@ -180,7 +180,7 @@ public Unit(Entity en, Campaign c) { this.parts = new ArrayList<>(); this.podSpace = new ArrayList<>(); this.drivers = new ArrayList<>(); - this.gunners = new ArrayList<>(); + this.gunners = new HashSet<>(); this.vesselCrew = new ArrayList<>(); forceId = Force.FORCE_NONE; scenarioId = Scenario.S_DEFAULT_ID; @@ -4688,8 +4688,8 @@ public List getDrivers() { return Collections.unmodifiableList(drivers); } - public List getGunners() { - return Collections.unmodifiableList(gunners); + public Set getGunners() { + return Collections.unmodifiableSet(gunners); } public List getVesselCrew() { @@ -5843,15 +5843,17 @@ public void fixReferences(Campaign campaign) { } } } - for (int ii = gunners.size() - 1; ii >= 0; --ii) { - Person gunner = gunners.get(ii); + for(Person gunner : gunners){ if (gunner instanceof UnitPersonRef) { - gunners.set(ii, campaign.getPerson(gunner.getId())); - if (gunners.get(ii) == null) { + gunners.remove(gunner); + Person updatedGunner = campaign.getPerson(gunner.getId()); + if(updatedGunner != null){ + gunners.add(updatedGunner); + } + else{ logger.error( - String.format("Unit %s ('%s') references missing gunner %s", - getId(), getName(), gunner.getId())); - gunners.remove(ii); + String.format("Unit %s ('%s') references missing gunner %s", + getId(), getName(), gunner.getId())); } } } diff --git a/MekHQ/src/mekhq/campaign/unit/actions/HirePersonnelUnitAction.java b/MekHQ/src/mekhq/campaign/unit/actions/HirePersonnelUnitAction.java index 5515b0e279..cf89ceca14 100644 --- a/MekHQ/src/mekhq/campaign/unit/actions/HirePersonnelUnitAction.java +++ b/MekHQ/src/mekhq/campaign/unit/actions/HirePersonnelUnitAction.java @@ -29,7 +29,7 @@ import mekhq.campaign.personnel.generator.DefaultSkillGenerator; import mekhq.campaign.unit.Unit; -import java.util.List; +import java.util.Set; /** * Hires a full complement of personnel for a unit. @@ -155,7 +155,7 @@ public void execute(Campaign campaign, Unit unit) { && unit.getEntity().getWeaponList().stream() .anyMatch(weapon -> (weapon.getType() instanceof WeaponType) && (((WeaponType) weapon.getType()).getDamage() == WeaponType.DAMAGE_ARTILLERY))) { - final List gunners = unit.getGunners(); + final Set gunners = unit.getGunners(); if (!gunners.isEmpty() && gunners.stream().noneMatch(person -> person.getSkills().hasSkill(SkillType.S_ARTILLERY))) { new DefaultSkillGenerator(campaign.getRandomSkillPreferences()).generateArtillerySkill(ObjectUtility.getRandomItem(gunners)); } diff --git a/MekHQ/unittests/mekhq/campaign/unit/UnitPersonTest.java b/MekHQ/unittests/mekhq/campaign/unit/UnitPersonTest.java index a4558278ad..c533dea0f7 100644 --- a/MekHQ/unittests/mekhq/campaign/unit/UnitPersonTest.java +++ b/MekHQ/unittests/mekhq/campaign/unit/UnitPersonTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import java.util.List; +import java.util.Set; import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -301,7 +302,7 @@ public void testGunner() { verify(unit, times(1)).resetPilotAndEntity(); // Ensure when getting the gunner that it is the same gunner - List gunners = unit.getGunners(); + Set gunners = unit.getGunners(); assertTrue(gunners.contains(mockGunner)); assertTrue(unit.isGunner(mockGunner)); diff --git a/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java b/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java index 3da8b24d5e..0eb99c17a5 100644 --- a/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java +++ b/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java @@ -29,7 +29,9 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.util.Collections; + import java.util.HashSet; import java.util.List; + import java.util.Set; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.mock; @@ -66,7 +68,7 @@ private void setCrew(int driverCount, int totalDriverNeeds, boolean hasNavigator, Entity entity, String expected) { List drivers = Collections.nCopies(driverCount, crewMember); - List gunners = Collections.nCopies(gunnerCount, crewMember); + Set gunners = new HashSet(Collections.nCopies(gunnerCount, crewMember)); List crew = Collections.nCopies(crewCount + drivers.size() + gunners.size() + (hasNavigator ? 1 : 0), crewMember); @@ -134,4 +136,4 @@ public void noAssignedCrew() { false, mock(Jumpship.class), "Drivers: 0/1
Gunners: 0/1
Crew: 0/1
Navigator: 0/1"); } -} \ No newline at end of file +} From c9aa0e770e4f54a0d3ac59395b0bd5490142da8a Mon Sep 17 00:00:00 2001 From: psikomonkie <189469115+psikomonkie@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:45:03 -0500 Subject: [PATCH 2/3] Issue 5256: Fixed UnitTableModelTest --- .../mekhq/gui/model/UnitTableModelTest.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java b/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java index 0eb99c17a5..966a531132 100644 --- a/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java +++ b/MekHQ/unittests/mekhq/gui/model/UnitTableModelTest.java @@ -28,10 +28,8 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - import java.util.Collections; - import java.util.HashSet; - import java.util.List; - import java.util.Set; + import java.util.*; + import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.mock; @@ -68,7 +66,12 @@ private void setCrew(int driverCount, int totalDriverNeeds, boolean hasNavigator, Entity entity, String expected) { List drivers = Collections.nCopies(driverCount, crewMember); - Set gunners = new HashSet(Collections.nCopies(gunnerCount, crewMember)); + + Set gunners = new HashSet<>(); + for(int i = 0; i < gunnerCount; i++){ + gunners.add(mock(Person.class)); + } + List crew = Collections.nCopies(crewCount + drivers.size() + gunners.size() + (hasNavigator ? 1 : 0), crewMember); From dc313ed2ffd388dc73955506ab924833743a75ba Mon Sep 17 00:00:00 2001 From: psikomonkie <189469115+psikomonkie@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:10:01 -0500 Subject: [PATCH 3/3] Issue 5256: Refactored MekHQ Unit's Gunners to be Set to ensure gunners are unique --- MekHQ/src/mekhq/campaign/unit/Unit.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/MekHQ/src/mekhq/campaign/unit/Unit.java b/MekHQ/src/mekhq/campaign/unit/Unit.java index 8ff838fdee..6ed9b745f6 100644 --- a/MekHQ/src/mekhq/campaign/unit/Unit.java +++ b/MekHQ/src/mekhq/campaign/unit/Unit.java @@ -5843,9 +5843,10 @@ public void fixReferences(Campaign campaign) { } } } + Set gunnersToRemove = new HashSet<>(); for(Person gunner : gunners){ if (gunner instanceof UnitPersonRef) { - gunners.remove(gunner); + gunnersToRemove.add(gunner); Person updatedGunner = campaign.getPerson(gunner.getId()); if(updatedGunner != null){ gunners.add(updatedGunner); @@ -5857,6 +5858,8 @@ public void fixReferences(Campaign campaign) { } } } + gunners.removeAll(gunnersToRemove); + for (int ii = vesselCrew.size() - 1; ii >= 0; --ii) { Person crew = vesselCrew.get(ii); if (crew instanceof UnitPersonRef) {