Advanced Atmospheric Control Roll Errata #5867

Algebro7 · 2024-08-06T15:30:05Z

This is currently a WIP implementation of the StratOps Advanced Atmospheric Control Roll Errata introduced on August 2, 2024. The plan is to keep the old rule as an (Unofficial) rule. Please do not merge yet.

Things left to do (please add to this list if you notice something I've missed):

Calculate Aero units' highest threshold
Calculate LAM highest threshold (only for control rolls, not criticals)
Modify threshold control roll logic to compare damageThisRound to the highest threshold
Add setting for old "Unofficial" advanced rule
Update settings entry text to describe new rule
Only roll once when taking a critical hit in addition to the threshold conditions in new rule
Apply to small crafts like dropships? (should already be covered, since Dropships extend Aero where these changes were implemented)

megamek/src/megamek/common/LandAirMech.java

megamek/src/megamek/server/GameManager.java

codecov · 2024-08-06T15:36:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.91%. Comparing base (0d1c946) to head (07bce77).
Report is 40 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #5867   +/-   ##
=========================================
  Coverage     28.91%   28.91%           
- Complexity    13942    13969   +27     
=========================================
  Files          2539     2542    +3     
  Lines        268416   268503   +87     
  Branches      47947    47967   +20     
=========================================
+ Hits          77603    77644   +41     
- Misses       186842   186882   +40     
- Partials       3971     3977    +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

megamek/src/megamek/common/Aero.java

Algebro7 · 2024-08-06T16:21:52Z

megamek/i18n/megamek/common/options/messages.properties

@@ -489,6 +489,8 @@ GameOptionsInfo.option.crashed_dropships_survive.displayableName=(Unofficial) Un
 GameOptionsInfo.option.crashed_dropships_survive.description=If checked, DropShips (and larger) that crash while out of control are not instantly destroyed.
 GameOptionsInfo.option.expanded_kf_drive_damage.displayableName=(Unofficial) Damage individual K-F Drive components on K-F Drive Critical Hit
 GameOptionsInfo.option.expanded_kf_drive_damage.description=If a K-F Drive critical hit is taken, a random component is hit (per BattleSpace rules).
+GameOptionsInfo.option.unoff_adv_atmospheric_control.displayableName=(Unofficial) Old StratOps Advanced Atmospheric Control Rolls


Open to suggestions on the naming/wording for these

Algebro7 · 2024-08-08T20:33:03Z

megamek/src/megamek/server/GameManager.java

+            } else {
+                // Not an atmospheric control roll under the new rules, so treat normally
+                rolls.addElement(modifier);
+                if (!reasons.isEmpty()) {
+                    reasons.append("; ");
+                }
+                reasons.append(modifier.getPlainDesc());
+              }


Will there ever be any control rolls that fall into this condition? Or at this phase of the game (end phase) will all control rolls be thresh/highest thresh/avionics/control? I handled it just to be safe but this condition may be unnecessary

megamek/unittests/megamek/server/GameManagerTest.java

Algebro7 · 2024-09-03T00:36:23Z

@Sleet01 I think this is ready for review now that 0.50 is out, whenever you get a chance. @Thom293 tested it awhile back and thought it worked correctly, but I'd like to get an experienced set of eyes on this.

Note my open question above about different types of control rolls in the endphase.

Thanks in advance!

Sleet01 · 2024-09-03T15:44:20Z

@Algebro7 Looks like NeoAncient's PR updating GameManager.java just went in, so you will need to resolve the new merge conflicts.

I've found the following approach to give good results and, more importantly, not cause a lot of headaches:

Update your fork (via GitHub web UI or CLI) to match the current main repo state.
Switch to master branch on your PC
Pull from the fork
Switch to the feature branch
Run git rebase master via CLI, or use the IDE to rebase the feature branch on the latest master branch state
Resolve any merge conflicts
Push to your fork (will probably require a force, like git push origin atmospheric-control-roll-errata --force-with-lease or the GUI equivalent)

… a separate method

…eam filter

Algebro7 · 2024-09-03T17:05:28Z

Thanks, I've rebased and fixed the failing test.

Sleet01 · 2024-09-04T05:52:34Z

megamek/src/megamek/common/LandAirMech.java

@@ -127,6 +127,7 @@ public String[] getLocationAbbrs() {
    private int altLoss = 0;
    private int altLossThisRound = 0;

+


Was this added intentionally?

Regardless of the answer to this question, it's surely more effort than it's worth it to worry about a single blank line

It's nearly no effort to fix, and everyone should get in the habit of spotting unintended changes to their PRs as a matter of course.

My bad, fixed in 07bce77

Sleet01

Looks good to me, just one minor question but very nice!

megamek/src/megamek/server/totalwarfare/TWGameManager.java

+                for (Enumeration<PilotingRollData> j = game.getControlRolls(); j.hasMoreElements(); ) {
+                    final PilotingRollData modifier = j.nextElement();
+                    if (modifier.getEntityId() != e.getId()) {
+                        continue;


Algebro7 changed the title ~~DRAFT: Atmospheric control roll errata~~ DRAFT: Advanced Atmospheric Control Roll Errata Aug 6, 2024

Algebro7 commented Aug 6, 2024

View reviewed changes

megamek/src/megamek/common/LandAirMech.java Outdated Show resolved Hide resolved

Algebro7 commented Aug 6, 2024

View reviewed changes

megamek/src/megamek/server/GameManager.java Outdated Show resolved Hide resolved

Algebro7 commented Aug 6, 2024

View reviewed changes

megamek/src/megamek/common/Aero.java Show resolved Hide resolved

Algebro7 commented Aug 6, 2024

View reviewed changes

repligator added Aerospace Errata This covers officially produced CGL errata that the programs need to implement. labels Aug 6, 2024

Algebro7 force-pushed the atmospheric-control-roll-errata branch from fde87ee to 5cced64 Compare August 8, 2024 16:34

Algebro7 commented Aug 8, 2024

View reviewed changes

Algebro7 force-pushed the atmospheric-control-roll-errata branch from b8f0158 to 427fbc2 Compare August 13, 2024 13:21

github-advanced-security bot found potential problems Aug 13, 2024

View reviewed changes

megamek/unittests/megamek/server/GameManagerTest.java Fixed Show fixed Hide fixed

Algebro7 force-pushed the atmospheric-control-roll-errata branch from 427fbc2 to 2b94652 Compare August 15, 2024 13:55

Algebro7 force-pushed the atmospheric-control-roll-errata branch from 2b94652 to ee5d6ac Compare August 25, 2024 18:59

Algebro7 force-pushed the atmospheric-control-roll-errata branch from ee5d6ac to fe0bd22 Compare September 3, 2024 00:34

Algebro7 marked this pull request as ready for review September 3, 2024 00:34

Algebro7 changed the title ~~DRAFT: Advanced Atmospheric Control Roll Errata~~ Advanced Atmospheric Control Roll Errata Sep 3, 2024

Algebro7 added 11 commits September 3, 2024 12:31

WIP: implement 02-08-2024 atmospheric control rolls errata

8f5d2ed

WIP: stub out highest threshold override for LAMs

3519d37

add constant for old/unofficial SO atmospheric control roll rule

ef95e44

add unofficial option for old stratops rule and update current one

a9bfe23

check both old and new control roll rules

474fb4e

implement getHighestThresh() for LAMs

c6ab462

track avionics and control system hits per round

f1f90ad

switch per-round avionics/control hit tracking to boolean

c1fa70c

remove unnecessary hit tracking and extract adv atmo control rolls to…

5879d8b

… a separate method

check highest thresh even if a normal thresh happened; fix bug in str…

c0ec68b

…eam filter

rename method and add docstring

640d05f

Algebro7 added 4 commits September 3, 2024 12:31

show all adv atmo control roll reasons in report

6c2ffd2

add tests for advanced atmospheric control rolls

5fad07d

add LAM tests

7c1ef94

remove redundant asserts

4e381b6

Algebro7 force-pushed the atmospheric-control-roll-errata branch from fe0bd22 to 4e381b6 Compare September 3, 2024 16:32

move Unit Tests into correct TW package

d9886d5

Sleet01 reviewed Sep 4, 2024

View reviewed changes

Sleet01 approved these changes Sep 4, 2024

View reviewed changes

remove erroneous white space

c089690

github-advanced-security bot found potential problems Sep 4, 2024

View reviewed changes

fix whitespace for real this time

07bce77

Sleet01 merged commit 876cd55 into MegaMek:master Sep 4, 2024
5 checks passed

Algebro7 deleted the atmospheric-control-roll-errata branch September 4, 2024 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Advanced Atmospheric Control Roll Errata #5867

Advanced Atmospheric Control Roll Errata #5867

Algebro7 commented Aug 6, 2024 •

edited

Loading

codecov bot commented Aug 6, 2024 •

edited

Loading

Algebro7 Aug 6, 2024

Algebro7 Aug 8, 2024 •

edited

Loading

Algebro7 commented Sep 3, 2024 •

edited

Loading

Sleet01 commented Sep 3, 2024

Algebro7 commented Sep 3, 2024

Sleet01 Sep 4, 2024

pavelbraginskiy Sep 4, 2024

Sleet01 Sep 4, 2024

Algebro7 Sep 4, 2024 •

edited

Loading

Sleet01 left a comment

		@@ -127,6 +127,7 @@ public String[] getLocationAbbrs() {
		private int altLoss = 0;
		private int altLossThisRound = 0;

Advanced Atmospheric Control Roll Errata #5867

Advanced Atmospheric Control Roll Errata #5867

Conversation

Algebro7 commented Aug 6, 2024 • edited Loading

codecov bot commented Aug 6, 2024 • edited Loading

Codecov Report

Algebro7 Aug 6, 2024

Choose a reason for hiding this comment

Algebro7 Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Algebro7 commented Sep 3, 2024 • edited Loading

Sleet01 commented Sep 3, 2024

Algebro7 commented Sep 3, 2024

Sleet01 Sep 4, 2024

Choose a reason for hiding this comment

pavelbraginskiy Sep 4, 2024

Choose a reason for hiding this comment

Sleet01 Sep 4, 2024

Choose a reason for hiding this comment

Algebro7 Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Sleet01 left a comment

Choose a reason for hiding this comment

Algebro7 commented Aug 6, 2024 •

edited

Loading

codecov bot commented Aug 6, 2024 •

edited

Loading

Algebro7 Aug 8, 2024 •

edited

Loading

Algebro7 commented Sep 3, 2024 •

edited

Loading

Algebro7 Sep 4, 2024 •

edited

Loading