Mounted rework #5406
Mounted rework #5406
Conversation
# Conflicts: # megamek/src/megamek/client/bot/princess/ArtilleryTargetingControl.java # megamek/src/megamek/client/bot/princess/FireControl.java # megamek/src/megamek/client/bot/princess/MultiTargetFireControl.java # megamek/src/megamek/client/bot/princess/Princess.java # megamek/src/megamek/client/bot/princess/WeaponFireInfo.java # megamek/src/megamek/common/Aero.java # megamek/src/megamek/common/Compute.java # megamek/src/megamek/common/Entity.java # megamek/src/megamek/common/Mounted.java # megamek/src/megamek/common/WeaponType.java # megamek/src/megamek/common/actions/WeaponAttackAction.java # megamek/src/megamek/common/weapons/AmmoWeapon.java # megamek/src/megamek/common/weapons/bayweapons/BayWeapon.java # megamek/src/megamek/common/weapons/infantry/InfantryWeapon.java # megamek/src/megamek/server/GameManager.java # megamek/unittests/megamek/client/bot/princess/FireControlTest.java
There was a problem hiding this comment.
Considering how central Mounted is, the amount of change is rather lower than I had expected. We have some equipment with non-obvious type, e.g. C3M is a weapon and (iirc) Radical Heat Sinks are an AmmoType but I imagine if it worked before then it'll work now.
|
But there's a test failure :) |
|
I ran the tests but now I suspect that I actually had master checked out at the time. |
… and the code used getLinkedAmmo()
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -309,17 +311,11 @@ | |||
| boolean isPointblankShot, List<ECMInfo> allECMInfo, boolean evenIfAlreadyFired, | |||
| int ammoId) { | |||
| final Entity ae = game.getEntity(attackerId); | |||
| final Mounted weapon = ae.getEquipment(weaponId); | |||
| final Mounted linkedAmmo = (ammoId == -1) ? weapon.getLinked() : ae.getEquipment(ammoId); | |||
| final WeaponMounted weapon = (WeaponMounted) ae.getEquipment(weaponId); | |||
Check warning
Code scanning / CodeQL
Dereferenced variable may be null Warning
| @@ -4361,7 +4357,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, Mounted ammo, EnumSet<AmmoType.Munitions> munition, boolean isAttackerInfantry, | |||
| WeaponType wtype, WeaponMounted weapon, int weaponId, AmmoType atype, AmmoMounted ammo, EnumSet<AmmoType.Munitions> munition, boolean isAttackerInfantry, | |||
Check notice
Code scanning / CodeQL
Useless parameter Note
| @@ -2724,8 +2727,8 @@ | |||
| continue; | |||
| } | |||
|
|
|||
| final Mounted suggestedAmmo = info.getAmmo(); | |||
| final Mounted mountedAmmo = getPreferredAmmo(shooter, info.getTarget(), currentWeapon, suggestedAmmo); | |||
| final AmmoMounted suggestedAmmo = info.getAmmo(); | |||
Check notice
Code scanning / CodeQL
Unread local variable Note
| @@ -4238,17 +4251,18 @@ | |||
| // One entry per ammo type that is currently usable by this weapon. | |||
| return ammos; | |||
| } | |||
| public ArrayList<Mounted> getMisc() { | |||
|
|
|||
| public List<MiscMounted> getMisc() { | |||
Check notice
Code scanning / CodeQL
Exposing internal representation Note
| @@ -4214,22 +4228,21 @@ | |||
| return false; | |||
| } | |||
|
|
|||
| public ArrayList<Mounted> getAmmo() { | |||
| public List<AmmoMounted> getAmmo() { | |||
Check notice
Code scanning / CodeQL
Exposing internal representation Note
| return weaponList; | ||
| } | ||
|
|
||
| public ArrayList<Mounted> getWeaponList() { | ||
| public List<WeaponMounted> getWeaponList() { |
Check notice
Code scanning / CodeQL
Exposing internal representation Note
| @@ -3853,26 +3854,41 @@ | |||
| /** | |||
| * Returns an enumeration of all equipment | |||
| */ | |||
| public ArrayList<Mounted> getEquipment() { | |||
| public List<Mounted<?>> getEquipment() { | |||
Check notice
Code scanning / CodeQL
Exposing internal representation Note
| } | ||
|
|
||
| @Override | ||
| public List<Integer> getBayWeapons() { |
Check notice
Code scanning / CodeQL
Exposing internal representation Note
This overhaul of Mounted is the next step in my equipment overhaul. The vast majority of the changes involve type checking, and errors would be caught by the compiler.
What it does:
Benefits:
Note that the calculation of ammo shots is still in the base class rather than AmmoMounted because it is also used by mines, which are MiscType. It can probably be simplified but I left that for the future, as this is already very extensive.
Also I handled the movement of bay weapons and ammo to WeaponMounted in a separate PR that builds on this one, since that change also touches a lot of code.