-
Notifications
You must be signed in to change notification settings - Fork 287
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 1552 rfe add ada air defense arrow iv munition #4762
Fix 1552 rfe add ada air defense arrow iv munition #4762
Conversation
(Rebased on later master)
(Rebased to a later master)
(rebased from later master)
…ler code (rebased from later master)
…rect-fire Artillery Flak
(rebased from later master)
…rgets and ADA both available
In cases where Princess has ADA missiles and is fighting forces with airborne units, prefer ADA over all else, including artillery attacks. Updated estimation of damage and to-hit chances when using Flak or ADA missiles. Added some extra belt-and-suspenders checks to damage application, in case ADA missiles get handled as Artillery (they should not, but still). Updated some tests to be disabled under Java 17 or later, due to issues with spying classes containing certain classes or list types. Close MegaMek#1152
megamek/src/megamek/common/weapons/ArtilleryCannonWeaponHandler.java
Fixed
Show resolved
Hide resolved
@@ -4496,7 +4508,7 @@ | |||
*/ | |||
private static ToHitData compileTerrainAndLosToHitMods(Game game, Entity ae, Targetable target, int ttype, int aElev, int tElev, | |||
int targEl, int distance, LosEffects los, ToHitData toHit, ToHitData losMods, int toSubtract, int eistatus, | |||
WeaponType wtype, Mounted weapon, int weaponId, AmmoType atype, long munition, boolean isAttackerInfantry, | |||
WeaponType wtype, Mounted weapon, int weaponId, AmmoType atype, EnumSet<AmmoType.Munitions> munition, boolean isAttackerInfantry, |
Check notice
Code scanning / CodeQL
Useless parameter Note
@@ -3192,7 +3204,7 @@ | |||
* @param isINarcGuided flag that indicates whether the target is broadcasting an iNarc beacon | |||
*/ | |||
private static ToHitData compileAmmoToHitMods(Game game, Entity ae, Targetable target, int ttype, ToHitData toHit, | |||
WeaponType wtype, Mounted weapon, AmmoType atype, long munition, boolean bApollo, boolean bArtemisV, | |||
WeaponType wtype, Mounted weapon, AmmoType atype, EnumSet<AmmoType.Munitions> munition, boolean bApollo, boolean bArtemisV, |
Check notice
Code scanning / CodeQL
Useless parameter Note
@@ -3192,7 +3204,7 @@ | |||
* @param isINarcGuided flag that indicates whether the target is broadcasting an iNarc beacon | |||
*/ | |||
private static ToHitData compileAmmoToHitMods(Game game, Entity ae, Targetable target, int ttype, ToHitData toHit, | |||
WeaponType wtype, Mounted weapon, AmmoType atype, long munition, boolean bApollo, boolean bArtemisV, | |||
WeaponType wtype, Mounted weapon, AmmoType atype, EnumSet<AmmoType.Munitions> munition, boolean bApollo, boolean bArtemisV, |
Check notice
Code scanning / CodeQL
Useless parameter Note
I'll test this later this week. Exciting to see. |
I'm very excited, both for ADAs specifically and the capacity for adding more munition types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Needs a little cleanup and I've got a couple of minor suggestions.
Also, and this is a requirement, please check that MekHQ and MML at least compile with these changes.
megamek/src/megamek/client/bot/princess/ArtilleryTargetingControl.java
Outdated
Show resolved
Hide resolved
megamek/src/megamek/client/ui/swing/unitDisplay/WeaponPanel.java
Outdated
Show resolved
Hide resolved
megamek/src/megamek/client/ui/swing/unitDisplay/WeaponPanel.java
Outdated
Show resolved
Hide resolved
megamek/src/megamek/common/weapons/ArtilleryCannonWeaponHandler.java
Fixed
Show resolved
Hide resolved
Shit. I added a PR for MML's lone usage of Munitions but missed MekHQ's somehow. I'll put up a dependent PR. Edit: PR for MekHQ fix is #4762 |
Not to rush anyone, but the dependent PR for MML already went in, so it's broken until this gets pulled. |
I think all the requested changes are done right? |
I think I got everything. I asked Nick to check it over one more time. |
This PR adds two new features:
Also includes:
close #1552