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

Fix 3870 and 4524: no quirks / SPAs applied to Artillery weapons #4792

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Sep 18, 2023

It looks like the Quirk and SPA check paths got broken for Artillery back when the artillery handling was split out from regular attacks.

I've combined all Attacker Quirks, Attacker SPAs, and Defender SPAs into functions and consolidated the calls to these functions to three locations:

  • compileTargetToHitMods()
  • handleArtilleryAttacks()
  • artilleryDirectToHit() (just the ADA section, as this section creates an entirely new toHit object from scratch)

I believe this will apply all relevant modifiers to all weapon types, provided the game options to enable Quirks and SPAs are set correctly.
Also cleaned up some NPE paths:

  • NPE while moving through the unit configuration options if Quirks were enabled but the selected / next unit was Infantry;
  • NPE in packet handling during start of Indirect phase while checking Quirks and SPAs (weapon or target might be null)
  • NPE while Princess traverses enemy unit -> target probabilities

Testing:

  1. Created a game with various positive and negative quirks and SPAs on several AAA Artillery platforms;
  2. Added several different types of ground, VTOL, and ASF targets with their own quirks and SPAs.
  3. Aimed at various targets and recorded reported toHit mods, then fired and confirmed mods were applied as projected.

Edit:
close #3870
close #4524

public static ToHitData processDefenderSPAs(ToHitData toHit, Entity ae, Entity te, Mounted weapon, Game game){

if (null == te) {
return toHit;

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'weapon' is never used.
@NickAragua NickAragua merged commit 027a6d3 into MegaMek:master Sep 21, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants