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

Implement 4397 internal bomb bay redux #5003

Merged

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Jan 2, 2024

(Note: this is a re-submission of PR 4974, due to merge issues I caused by fixing things out of order)

This has some pretty big changes:

New gameplay features:

  1. Aerospace fighters, Conventional Fighters, Fixed Wing Support aircraft, Small Craft, and Dropships with the "Internal Bomb Bay" quirk can now mount bomb-type munitions internally. The Bombs section of the configuration dialog will indicate Internal and/or External bomb options as appropriate. When bombing, the user will be prompted separately for Internal and External bomb stores to utilize (this is necessary for accounting reasons, and to keep the UI simple).
  2. Units that utilize their internal stores to make attacks (or to TAG, if a TAG pod is mounted) chance ground fire touching off any other stores in their internal bays: on a 10+ roll at the end of a round in which a unit has taken damage from ground fire, all remaining internal bombs explode, dealing their full damage directly to SI!

Changes/Additions

  1. Re-organized Aero class hierarchy slightly:
|Entity
|- Aero
    |- TeleMissile
    |- AeroSpaceFighter                 // Inserted between Aero and subclasses
    |   |- ConventionalFighter
    |   |   |- FixedWingSupport
    |   |- FighterSquadron             // Now made up of ASFs, not base Aeros
    |- SmallCraft
    |   |- Dropship
    |   |- EscapePods                     // This should probably go under Aero, but out of scope for this PR.
    |- Jumpship
         |- Warship
         |- SpaceStation   

in service of reducing the number of classes that needed updates for IBB. This should also allow reducing the code in Aero considerably and moving ASF-only functionality into that class, if desired.

  1. Updated the Internal Bomb Bay quirk tooltip and text.
  2. Added handling for Internal and External Bombs, including updates to configuration and bomb targeting dialogs.
  3. Added IBB-related End Phase report, with relevant messages and explosion check / damage application.
  4. Added ephemeral Cargo Bay generation for Aeros that mount the Cargo Bay miscellaneous equipment, as opposed to actual bays. These are not saved, but are used when calculating bomb space for IBB (and, perhaps later, for cargo?).
  5. Fixed "Invalid Design" checks for A) extra weight on ASFs with internal bombs and ephemeral bays, and B) "BA ECM" on Small Craft (MekHQ Issue #1970
  6. Added accounting for ground-fire-only damage per entity; this is required for 4) above.
  7. Added separate listing of Internal and External munitions for summary view and MUL saving/loading.
  8. Added ability for VTOLs to bomb their current hex if a "bomb" hex was not added during the Move phase - this appears to have caused some confusion, and VTOLs should be able to bomb their final location in the Attack phase.
  9. Fixed a timeout error that would cause Princess to hang when attempting to move ground units in the presence of a large number of bombs. This may be the root of some of the recent Princess hangs we have not been able to track down; I will investigate further.

TO-DO / Not Yet Implemented

  1. Separately accounting for each bomb in each disparate cargo bay: Cargo Bays need to be upgraded to a real Class with internal logic and accounting for this to be feasible. Currently we offer no way to account for location of bombs until they are instantiated after leaving the lobby; everything just gets thrown into "NOS". This unfortunately means that cheesing bombers with multiple 6-bomb bays won't protect IBB users.
  2. Counting cargo space filled with bombs as "used": see 1).
  3. Effect of a Cargo crit on aerospace units mounting internal bomb stores and using IBB: forum question here has been answered and is provisionally implemented the same way as standard external bombs. It looks like adding user choice of bombs to destroy will require wiring up some special event handling, which I feel is out of scope.
  4. Verification of Fighter Squadron bombing capabilities; this came up as a question during development and is apparently not yet settled.

Testing

  1. Ran MM tests in Idea
  2. Ran MHQ tests in Idea
  3. Tested configuration, saving, loading, and MUL reinforcement with all valid Internal Bomb Bay unit types.
  4. Verified operation of all valid bomber types, without and (where allowed) with Internal Bomb Bay.
  5. Verified Princess operations with Internal, External, and mixed munition loads.
  6. Verified MHQ compatibility (after concomitant MHQ patches)

Note:

  1. I updated the JVM options in build.gradle to use 4GB of RAM and disable AWT grabbing (source of a lot of freezing while debugging AWT windows). I can remove this if folks don't feel it's necessary for them.

  2. Pulling this PR will also require pulling the matching MekHQ PR here

Close #4397

Sleet01 added 21 commits January 2, 2024 10:53
This was intended for a new option in the top-level Exception handler dialog, but extraneous here.
Copy link
Member

@SJuliez SJuliez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments.
Did you also check compatibility with MML?
I noticed some code that could be omitted when there are no units left that are pure Aero class.

@@ -154,8 +154,12 @@ public String[] getLocationNames() {
// fixed and pod-mounted.
private int podHeatSinks;

protected int maxBombPoints = 0;
protected int[] bombChoices = new int[BombType.B_NUM];
protected int maxIntBombPoints = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not about this line)

Since fighters are no longer directly Aero, do we ever instantiate Aero? It seems that the Aero class could be abstract.

Copy link
Collaborator Author

@Sleet01 Sleet01 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not about this line)

Since fighters are no longer directly Aero, do we ever instantiate Aero? It seems that the Aero class could be abstract.

It could be, although I wasn't planning to at this juncture. It would let us move a lot of stuff out of Aero.
Edit: we do not ever instantiate Aero directly now.

megamek/src/megamek/common/AeroSpaceFighter.java Outdated Show resolved Hide resolved
megamek/src/megamek/common/Entity.java Outdated Show resolved Hide resolved
megamek/src/megamek/common/IBomber.java Show resolved Hide resolved
megamek/src/megamek/common/battlevalue/BVCalculator.java Outdated Show resolved Hide resolved
@Sleet01
Copy link
Collaborator Author

Sleet01 commented Jan 2, 2024

A few comments. Did you also check compatibility with MML? I noticed some code that could be omitted when there are no units left that are pure Aero class.

I did check compatibility in MML, although there should be no impact there: bombs are only added in MM / MHQ, and the added "virtual" cargo space for ASFs (used to calculate internal bomb capacity for ASFs with "Cargo" equipment) is not utilized by MML.

@SJuliez SJuliez merged commit 330199b into MegaMek:master Jan 3, 2024
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
Development

Successfully merging this pull request may close these issues.

RFE: Implement Internal Bomb Bay quirk
2 participants