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 1429: mounted.getLinked() is null #1433

Merged

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Feb 19, 2024

This is the MML addendum to the MM fix to #1429.
Technically it isn't necessary as the offending calls will be handled correctly once MegaMek/megamek#5176 goes in, but this PR cleans the original calls up as passing null as a weapon value is ugh.

This PR also adds unit tests to:

  • verify that every column of the EquipmentTableModel of each unit type can be populated and read;
  • verify that EquipmentTableModel.getDamageString() (used to populate the damage field in the table) works correctly with Aerospace, CI, and Artillery;

said unit tests to be run via the "Verification/test" gradle task prior to any PR submissions.

There are still many, many areas within MML that need unit test coverage, but at least this gets the ball rolling.

NOTE:
The tests in this PR will continue to fail until MegaMek/megamek#5176 is pulled.

@SJuliez
Copy link
Member

SJuliez commented Feb 20, 2024

This PR contains a range of units. Is this intentional?

@Sleet01
Copy link
Collaborator Author

Sleet01 commented Feb 20, 2024

This PR contains a range of units. Is this intentional?

Yes, since the root issue only impacted the Aero equipment view, I wanted to add coverage for the full range of unit types in the new test to catch any similar single-type equipment view issues.

@SJuliez SJuliez merged commit b246f4e into MegaMek:master Feb 20, 2024
1 of 4 checks passed
@neoancient neoancient mentioned this pull request Feb 21, 2024
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