Improvement: Adjusted the Rate of Captured Clan Personnel Committing Bondsref; Added the Ability for Captured Combine Personnel to Commit Seppuku #6907
Conversation
…Bondsref; Added the Ability for Captured Combine Personnel to Commit Seppuku
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6907 +/- ##
============================================
- Coverage 11.97% 11.71% -0.26%
+ Complexity 6785 6676 -109
============================================
Files 1101 1101
Lines 141414 141434 +20
Branches 21901 21907 +6
============================================
- Hits 16937 16575 -362
- Misses 122677 123155 +478
+ Partials 1800 1704 -96 ☔ 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 adjusts the probability logic for captured clan personnel committing Bondsref and introduces the ability for captured Combine personnel to commit Seppuku. In addition, the PersonnelStatus enum and associated test cases and resource messages have been updated to include the new SEPPUKU status.
- Updated test cases to include SEPPUKU in relevant status lists.
- Modified CapturePrisoners logic to simulate a 2d6 roll for clan personnel and added a branch for Combine personnel seppuku.
- Added a new SEPPUKU entry in PersonnelStatus and updated resource files accordingly.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| MekHQ/unittests/mekhq/campaign/personnel/enums/PersonnelStatusTest.java | Added SEPPUKU to test lists verifying status-related methods. |
| MekHQ/src/mekhq/campaign/randomEvents/prisoners/CapturePrisoners.java | Adjusted dice roll calculation for Bondsref and added Seppuku branch. |
| MekHQ/src/mekhq/campaign/personnel/enums/PersonnelStatus.java | Extended the enum with SEPPUKU and provided its accessor method. |
| MekHQ/resources/mekhq/resources/PrisonerEvents.properties | Added resource entries for SEPPUKU event reporting. |
| MekHQ/resources/mekhq/resources/PersonnelStatus.properties | Added SEPPUKU resource entries for labels, tooltips, reports, and logs. |
| if (capturingFaction != null && capturingFaction.isClan()) { | ||
| if (isMekHQCaptureStyle && prisoner.isClanPersonnel() && (bondsmanRoll == 1)) { | ||
| if (isMekHQCaptureStyle && prisoner.isClanPersonnel() && (bondsmanRoll + d6(1) == 2)) { |
There was a problem hiding this comment.
[nitpick] Consider extracting the dice roll calculation (bondsmanRoll + d6(1)) into a clearly named variable to clarify that it simulates a 2d6 roll, which would improve overall readability.
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| if (isMekHQCaptureStyle) { | ||
| if (Objects.equals(prisoner.getOriginFaction().getShortName(), "DC")) { |
There was a problem hiding this comment.
[nitpick] The hard-coded faction short name "DC" appears as a magic string; consider defining it as a constant for improved maintainability and clarity.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Bondsref was occurring more regularly than intended for capture by honorable factions, so I adjusted it so that it only occurs on a 2d6 roll of 2 when captured by another Clan. For capture by non-Clans I kept the original 1 in 6.
I also added the ability for captured Draconis Combine personnel to commit Seppuku on a 2d6 roll of 2. This is added to enhance the flavor of capturing these personnel and is something we see a lot in the fiction. There was a user suggestion to extend this to Liao forces, but I'm not aware of a canon source showing Liao forces committing seppuku in the same manner as Combine forces.