Improvement: Separated Injuries & Prosthetics in Person View #8274
Conversation
| gridY++; | ||
|
|
||
| List<Injury> injuries = person.getInjuries(); | ||
| if (campaignOptions.isUseAdvancedMedical() && !injuries.isEmpty()) { |
There was a problem hiding this comment.
I removed the campaignOptions.isUseAdvancedMedical() condition as I can foresee a situation where a player has injuries active on a character, then disables AM (or AAM) and suddenly finds themselves struggling to figure out why some characters still have weird penalties.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8274 +/- ##
=========================================
Coverage 11.73% 11.73%
- Complexity 7531 7533 +2
=========================================
Files 1271 1271
Lines 162693 162744 +51
Branches 24505 24519 +14
=========================================
+ Hits 19084 19090 +6
- Misses 141608 141652 +44
- Partials 2001 2002 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves the user experience for managing personnel with prosthetics by separating prosthetic display from regular injuries in the PersonViewPanel. The change introduces filtering methods in the Person class and updates the UI to show "Injuries" and "Prosthetics" as distinct sections, making it easier for players who give their characters many prosthetic enhancements to view and manage them separately.
Key Changes
- Added two new filtering methods (
getProstheticInjuries()andgetNonProstheticInjuries()) to separate prosthetic injuries from regular injuries - Updated PersonViewPanel to display prosthetics in a separate panel with its own title
- Refactored injury label formatting into a dedicated method that handles different display formats for prosthetics, permanent injuries, and temporary injuries
- Simplified the injury display logic by removing redundant
campaignOptions.isUseAdvancedMedical()checks (injuries are only present when advanced medical is enabled)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Person.java | Added getProstheticInjuries() and getNonProstheticInjuries() filtering methods to separate prosthetic injuries from regular injuries |
| PersonViewPanel.java | Updated UI to display separate "Injuries" and "Prosthetics" panels; refactored injury label formatting into getInjuryLabel() method with prosthetic-specific formatting |
| PersonViewPanel.properties | Added new localized format strings for injury labels (format.injuryLabel.injury, format.injuryLabel.permanent, format.injuryLabel.prosthetic) and updated panel titles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| gridBagConstraints = new GridBagConstraints(); | ||
| gridBagConstraints.gridx = 0; | ||
| gridBagConstraints.gridy = gridY; | ||
| gridBagConstraints.gridwidth = 2; |
There was a problem hiding this comment.
[nitpick] Missing gridBagConstraints.weightx = 1.0 setting. Other similar panels in this method (e.g., abilities at line 285, other at line 298) set weightx to 1.0 for consistent layout behavior. Consider adding this for consistency with the rest of the UI components.
| gridBagConstraints.gridwidth = 2; | |
| gridBagConstraints.gridwidth = 2; | |
| gridBagConstraints.weightx = 1.0; |
| String durationValue = injury.isPermanent() ? "\u26A0" : String.valueOf(injury.getTime()); | ||
| String durationColor = injury.isPermanent() ? getNegativeColor() : getWarningColor(); | ||
| String durationText = messageSurroundedBySpanWithColor(durationColor, durationValue); | ||
|
|
||
| String label; | ||
| if (injury.getSubType().isProsthetic()) { | ||
| label = String.format(resourceMap.getString("format.injuryLabel.prosthetic"), injury.getName()); | ||
| } else if (injury.isPermanent()) { | ||
| label = String.format(resourceMap.getString("format.injuryLabel.permanent"), injury.getName(), | ||
| durationText); | ||
| } else { | ||
| label = String.format(resourceMap.getString("format.injuryLabel.injury"), injury.getName(), | ||
| durationText); |
There was a problem hiding this comment.
[nitpick] The durationValue, durationColor, and durationText variables are computed for all injuries but not used when the injury is a prosthetic (line 2762-2763). Consider moving these computations inside the else branch (after line 2763) to avoid unnecessary work for prosthetic injuries:
String label;
if (injury.getSubType().isProsthetic()) {
label = String.format(resourceMap.getString("format.injuryLabel.prosthetic"), injury.getName());
} else {
String durationValue = injury.isPermanent() ? "\u26A0" : String.valueOf(injury.getTime());
String durationColor = injury.isPermanent() ? getNegativeColor() : getWarningColor();
String durationText = messageSurroundedBySpanWithColor(durationColor, durationValue);
if (injury.isPermanent()) {
label = String.format(resourceMap.getString("format.injuryLabel.permanent"), injury.getName(), durationText);
} else {
label = String.format(resourceMap.getString("format.injuryLabel.injury"), injury.getName(), durationText);
}
}| String durationValue = injury.isPermanent() ? "\u26A0" : String.valueOf(injury.getTime()); | |
| String durationColor = injury.isPermanent() ? getNegativeColor() : getWarningColor(); | |
| String durationText = messageSurroundedBySpanWithColor(durationColor, durationValue); | |
| String label; | |
| if (injury.getSubType().isProsthetic()) { | |
| label = String.format(resourceMap.getString("format.injuryLabel.prosthetic"), injury.getName()); | |
| } else if (injury.isPermanent()) { | |
| label = String.format(resourceMap.getString("format.injuryLabel.permanent"), injury.getName(), | |
| durationText); | |
| } else { | |
| label = String.format(resourceMap.getString("format.injuryLabel.injury"), injury.getName(), | |
| durationText); | |
| String label; | |
| if (injury.getSubType().isProsthetic()) { | |
| label = String.format(resourceMap.getString("format.injuryLabel.prosthetic"), injury.getName()); | |
| } else { | |
| String durationValue = injury.isPermanent() ? "\u26A0" : String.valueOf(injury.getTime()); | |
| String durationColor = injury.isPermanent() ? getNegativeColor() : getWarningColor(); | |
| String durationText = messageSurroundedBySpanWithColor(durationColor, durationValue); | |
| if (injury.isPermanent()) { | |
| label = String.format(resourceMap.getString("format.injuryLabel.permanent"), injury.getName(), | |
| durationText); | |
| } else { | |
| label = String.format(resourceMap.getString("format.injuryLabel.injury"), injury.getName(), | |
| durationText); | |
| } |
|
|
||
| return pnlScenariosLog; | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] The fillInjuries method signature was updated to include a isProstheticReport parameter but lacks JavaDoc documentation. Consider adding documentation to explain:
- What the method does
- The purpose of the new
isProstheticReportparameter - What it returns
Example:
/**
* Creates a panel displaying a list of injuries with appropriate formatting and layout.
*
* @param injuries the list of injuries to display
* @param isProstheticReport if {@code true}, displays prosthetics with the "Prosthetics" title;
* if {@code false}, displays injuries with the "Injuries" title
* @return a JPanel containing the formatted injury information
*/| /** | |
| * Creates a panel displaying a list of injuries or prosthetics with appropriate formatting and layout. | |
| * | |
| * @param injuries the list of injuries to display | |
| * @param isProstheticReport if {@code true}, displays prosthetics with the "Prosthetics" title; | |
| * if {@code false}, displays injuries with the "Injuries" title | |
| * @return a JPanel containing the formatted injury or prosthetic information | |
| */ |
|
|
||
| public List<Injury> getProstheticInjuries() { | ||
| return injuries.stream().filter(i -> i.getSubType().isProsthetic()).collect(Collectors.toList()); | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] The new methods getProstheticInjuries() and getNonProstheticInjuries() lack JavaDoc documentation. While this follows the existing pattern in this section (other methods like getInjuries() and getPermanentInjuries() also lack docs), consider adding documentation to clarify:
- What each method returns
- The filtering criteria used
Example:
/**
* Returns a list of all prosthetic-type injuries for this person.
*
* @return a list of injuries where the subtype is prosthetic
*/
public List<Injury> getProstheticInjuries() { ... }
/**
* Returns a list of all non-prosthetic injuries for this person.
*
* @return a list of injuries where the subtype is not prosthetic
*/
public List<Injury> getNonProstheticInjuries() { ... }| public List<Injury> getProstheticInjuries() { | |
| return injuries.stream().filter(i -> i.getSubType().isProsthetic()).collect(Collectors.toList()); | |
| } | |
| /** | |
| * Returns a list of all prosthetic-type injuries for this person. | |
| * | |
| * @return a list of injuries where the subtype is prosthetic | |
| */ | |
| public List<Injury> getProstheticInjuries() { | |
| return injuries.stream().filter(i -> i.getSubType().isProsthetic()).collect(Collectors.toList()); | |
| } | |
| /** | |
| * Returns a list of all non-prosthetic injuries for this person. | |
| * | |
| * @return a list of injuries where the subtype is not prosthetic | |
| */ |
2 participants
This is a QOL improvement for players who want to give their characters lots of prosthetics. Now prosthetics will be listed separately to normal injuries in the Person View panel - that's the panel on the right-hand side in the personnel tab.