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

Stratcon Hidden Facility Modifier Fix #3665

Merged
merged 1 commit into from
May 20, 2023

Conversation

Thom293
Copy link
Contributor

@Thom293 Thom293 commented May 10, 2023

This PR changes one line of code to activate the Hidden Facility Modifiers so they are applied as intended.

Obviously, Stratcon is amazing and I defer to Nick on this. This was just discussed in discord this morning and I had a little time.

Discussing this with @PhoenixHeart512 this morning and he pointed out the bug. Enemy Facility modifers do not apply until they are scouted. This is a long standing issue, although there is no bug report on it that we could find. I had a little time today so poked around at it.

Per the code comments and @NickAragua, hidden SC facilities should apply their modifier on a %chance based on an "aggro rating." However, it appears the %chance aggro variable was never triggered for some reason (or the chance was so low as to be imperceptable) and so hidden modifiers are never applied.

This activates the Hidden Facility modifiers so they now work. It changes one line of code and changes the untriggered variable to a fixed 75% chance. Tested it over a few months, and it works. Per the original code, it works on a per facility per track basis. e.g.: A track with 2 hidden facilities will 75% of the time apply their modifiers to other battles. You will sometimes get both mods, 1, or none. Track 0 facilities will not apply their modifiers to Track 1 battles, etc.

This might later be a canidate for a Campaign Stratcon Option where the user can custom set the % chance to apply the hidden modifiers from between 1-100%. Like the "Chance for battle by role" option. But someone else would have to add that, as it is beyond my personal ability. But in the meantime, this gets them working and playable when they were not before. Thank you.

…osed to apply their modifier on a %chance basis. However, the %chance was never triggered and so the modifier was never applied. This activates the Hidden Facility modifiers. It changes the untriggered variable to a fixed 75% chance.
@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (4874b63) 10.66% compared to head (5d5e28c) 10.65%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3665      +/-   ##
============================================
- Coverage     10.66%   10.65%   -0.01%     
  Complexity     5472     5472              
============================================
  Files           830      830              
  Lines        113489   113500      +11     
  Branches      17156    17156              
============================================
  Hits          12099    12099              
- Misses       100182   100194      +12     
+ Partials       1208     1207       -1     
Impacted Files Coverage Δ
MekHQ/src/mekhq/AtBGameThread.java 0.00% <0.00%> (ø)
...hq/campaign/mission/AtBDynamicScenarioFactory.java 0.75% <0.00%> (-0.01%) ⬇️
.../mekhq/campaign/stratcon/StratconRulesManager.java 0.00% <0.00%> (ø)
MekHQ/src/mekhq/gui/view/AtBScenarioViewPanel.java 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@HammerGS HammerGS merged commit 4b88d6d into MegaMek:master May 20, 2023
HammerGS added a commit that referenced this pull request May 20, 2023
@Thom293 Thom293 deleted the HiddenFacilityModFix branch May 25, 2023 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants