Skip to content

Commit

Permalink
Fix role eligibility checks and ensure proper role assignment
Browse files Browse the repository at this point in the history 
Updated role eligibility logic to prevent assigning ineligible roles and removed redundant `Person` parameter in `canPerformRole`. Ensured consistent behavior when validating and updating both primary and secondary roles, resolving a long-standing bug. Improved GUI role selection to validate against all selected personnel.
  • Loading branch information
@IllianiCBT
IllianiCBT committed Jan 10, 2025
1 parent f4b7479 commit 7553aa4
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 22 deletions.
38 changes: 23 additions & 15 deletions MekHQ/src/mekhq/campaign/personnel/Person.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ public String getSecondaryRoleDesc() {
return getSecondaryRole().getName(isClanPersonnel());
}

public boolean canPerformRole(LocalDate today, Person person, final PersonnelRole role, final boolean primary) {
public boolean canPerformRole(LocalDate today, final PersonnelRole role, final boolean primary) {
if (primary) {
// Primary Role:
// We only do a few here, as it is better on the UX-side to correct the issues
Expand Down Expand Up @@ -930,7 +930,7 @@ public boolean canPerformRole(LocalDate today, Person person, final PersonnelRol
}
}

if (person.isChild(today)) {
if (isChild(today)) {
return false;
}

Expand All @@ -943,8 +943,7 @@ public boolean canPerformRole(LocalDate today, Person person, final PersonnelRol
case NAVAL_VEHICLE_DRIVER -> hasSkill(SkillType.S_PILOT_NVEE);
case VTOL_PILOT -> hasSkill(SkillType.S_PILOT_VTOL);
case VEHICLE_GUNNER -> hasSkill(SkillType.S_GUN_VEE);
case MECHANIC -> hasSkill(SkillType.S_TECH_MECHANIC)
&& (getSkill(SkillType.S_TECH_MECHANIC).getExperienceLevel() > SkillType.EXP_ULTRA_GREEN);
case MECHANIC -> hasSkill(SkillType.S_TECH_MECHANIC);
case VEHICLE_CREW ->
Stream.of(SkillType.S_TECH_MEK, SkillType.S_TECH_AERO, SkillType.S_TECH_MECHANIC,
SkillType.S_TECH_BA, SkillType.S_DOCTOR, SkillType.S_MEDTECH, SkillType.S_ASTECH)
Expand All @@ -958,18 +957,11 @@ public boolean canPerformRole(LocalDate today, Person person, final PersonnelRol
case VESSEL_CREW -> hasSkill(SkillType.S_TECH_VESSEL);
case VESSEL_GUNNER -> hasSkill(SkillType.S_GUN_SPACE);
case VESSEL_NAVIGATOR -> hasSkill(SkillType.S_NAV);
case MEK_TECH ->
hasSkill(SkillType.S_TECH_MEK)
&& (getSkill(SkillType.S_TECH_MEK).getExperienceLevel() > SkillType.EXP_ULTRA_GREEN);
case AERO_TEK ->
hasSkill(SkillType.S_TECH_AERO)
&& (getSkill(SkillType.S_TECH_AERO).getExperienceLevel() > SkillType.EXP_ULTRA_GREEN);
case BA_TECH ->
hasSkill(SkillType.S_TECH_BA)
&& (getSkill(SkillType.S_TECH_BA).getExperienceLevel() > SkillType.EXP_ULTRA_GREEN);
case MEK_TECH -> hasSkill(SkillType.S_TECH_MEK);
case AERO_TEK -> hasSkill(SkillType.S_TECH_AERO);
case BA_TECH -> hasSkill(SkillType.S_TECH_BA);
case ASTECH -> hasSkill(SkillType.S_ASTECH);
case DOCTOR -> hasSkill(SkillType.S_DOCTOR)
&& (getSkill(SkillType.S_DOCTOR).getExperienceLevel() > SkillType.EXP_ULTRA_GREEN);
case DOCTOR -> hasSkill(SkillType.S_DOCTOR);
case MEDIC -> hasSkill(SkillType.S_MEDTECH);
case ADMINISTRATOR_COMMAND, ADMINISTRATOR_LOGISTICS, ADMINISTRATOR_TRANSPORT, ADMINISTRATOR_HR ->
hasSkill(SkillType.S_ADMIN);
Expand Down Expand Up @@ -2715,6 +2707,22 @@ public static Person generateInstanceFromXML(Node wn, Campaign c, Version versio
if (retVal.getRecruitment() == null) {
retVal.setRecruitment(c.getLocalDate());
}

// This resolves a bug squashed in 2025 (50.03) but lurked in our codebase
// potentially as far back as 2014. The next two handlers should never be removed.
if (!retVal.canPerformRole(c.getLocalDate(), retVal.getPrimaryRole(), true)) {
retVal.setPrimaryRole(c, PersonnelRole.NONE);
logger.info(String.format("%s was found to be ineligible for their" +
" primary role. That role has been removed and they were assigned the" +
" NONE role.", retVal.getFullTitle()));
}

if (!retVal.canPerformRole(c.getLocalDate(), retVal.getSecondaryRole(), false)) {
retVal.setSecondaryRole(PersonnelRole.NONE);
logger.info(String.format("%s was found to be ineligible for their" +
" secondary role. That role has been removed and they were assigned the" +
" NONE role.", retVal.getFullTitle()));
}
} catch (Exception e) {
logger.error("Failed to read person {} from file", retVal.getFullName(), e);
retVal = null;
Expand Down
34 changes: 27 additions & 7 deletions MekHQ/src/mekhq/gui/adapter/PersonnelTableMouseAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1489,26 +1489,46 @@ protected Optional<JPopupMenu> createPopupMenu() {
}

final PersonnelRole[] roles = PersonnelRole.values();
menu = new JMenu(resources.getString("changePrimaryRole.text"));

menu = new JMenu(resources.getString("changePrimaryRole.text"));
for (final PersonnelRole role : roles) {
if (person.canPerformRole(gui.getCampaign().getLocalDate(), person, role, true)) {
cbMenuItem = new JCheckBoxMenuItem(role.getName(person.isClanPersonnel()));
boolean allCanPerform = true;

for (Person selectedPerson : getSelectedPeople()) {
if (!selectedPerson.canPerformRole(gui.getCampaign().getLocalDate(), role, true)) {
allCanPerform = false;
}
}

if (allCanPerform) {
cbMenuItem = new JCheckBoxMenuItem(role.toString());
cbMenuItem.setActionCommand(makeCommand(CMD_PRIMARY_ROLE, role.name()));
cbMenuItem.setSelected(person.getPrimaryRole() == role);
cbMenuItem.addActionListener(this);
if (oneSelected && role == person.getPrimaryRole()) {
cbMenuItem.setSelected(true);
}
menu.add(cbMenuItem);
}
}
JMenuHelpers.addMenuIfNonEmpty(popup, menu);

menu = new JMenu(resources.getString("changeSecondaryRole.text"));
for (final PersonnelRole role : roles) {
if (person.canPerformRole(gui.getCampaign().getLocalDate(), person, role, false)) {
cbMenuItem = new JCheckBoxMenuItem(role.getName(person.isClanPersonnel()));
boolean allCanPerform = true;

for (Person selectedPerson : getSelectedPeople()) {
if (!selectedPerson.canPerformRole(gui.getCampaign().getLocalDate(), role, false)) {
allCanPerform = false;
}
}

if (allCanPerform) {
cbMenuItem = new JCheckBoxMenuItem(role.toString());
cbMenuItem.setActionCommand(makeCommand(CMD_SECONDARY_ROLE, role.name()));
cbMenuItem.setSelected(person.getSecondaryRole() == role);
cbMenuItem.addActionListener(this);
if (oneSelected && role == person.getSecondaryRole()) {
cbMenuItem.setSelected(true);
}
menu.add(cbMenuItem);
}
}
Expand Down

0 comments on commit 7553aa4

Please sign in to comment.