Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hacky Fix: Makes StratCon Scenario Generation Respect Non-Combat Flag in TO&E #5010

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

UlyssesSockdrawer
Copy link
Contributor

This is a very simple hacky fix to StratconRulesManager.java's scenario generation.

Please be aware I don't know how to write or add unit tests, so haven't done any of that here.

I've added a line that, when trying to generate a new scenario on a Monday, as well as Nick's original checks of whether a unit is already deployed and is assigned to a track, also checks if the force is flagged as combat.

I've seen it step over a force that was flagged as non-combat in a very simple test campaign using the debugger so I think this works, but this needs careful testing to make sure.

I've attached a save file to this PR that people can use to test by generating some contracts and then seeing if you ever get 'lance-truck' picked as the seed lance.

It has one combat force and one non-combat force - lance-mek (combat) and lance-truck (non-combat).

NonCombatTestUnit-Base.cpnx.gz

Suggestion from Illiani in the discord that might get non-combat flag in TO&E to work.
Also added some comments, and another bit of code suggested by Illiani
With some help and use of debugger, we found the bit where it steps through each force to check the flag.

I've tested this with the debugger and it looks to work in a test campaign but needs solid testing.
@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.47%. Comparing base (64f8158) to head (4e72790).
Report is 46 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5010      +/-   ##
============================================
+ Coverage     10.46%   10.47%   +0.01%     
- Complexity     6022     6029       +7     
============================================
  Files           951      950       -1     
  Lines        133345   133397      +52     
  Branches      19378    19384       +6     
============================================
+ Hits          13955    13974      +19     
- Misses       118041   118076      +35     
+ Partials       1349     1347       -2     

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

@HammerGS HammerGS merged commit 032b771 into MegaMek:master Oct 11, 2024
4 checks passed
@HammerGS
Copy link
Member

I know we have issues that should be identfied and closed with this. Can someone reference those for closure.

@UlyssesSockdrawer
Copy link
Contributor Author

UlyssesSockdrawer commented Oct 11, 2024 via email

@UlyssesSockdrawer
Copy link
Contributor Author

@HammerGS looks like:

#2749 - can definitely be closed by this

#3168 - only the 'non-combat' part of this, so might not be fully closed here but might be closed by the work @IllianiCBT did around ForceGen3?

@IllianiCBT
Copy link
Collaborator

I'm going to leave #3168 open as this is part of a bigger issue where legacy ATB functionality causes issues with StratCon. For StratCon users shouldn't be deploying from the TO&E and shouldn't be manually un-deploying forces as that will (as illustrated in the cited report) cause issues.

@UlyssesSockdrawer UlyssesSockdrawer deleted the NonCombatFun branch October 13, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants