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 #5252: critical assignment tab eq list missing keywords like "Ammo" and "[Half]" or "[Full]" #1484

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Apr 15, 2024

Don't use the .getShortName() value for AmmoType instances in the Equipment List of the Crits tab.

Testing:

  • Added offending ammo types from the bug to new units.
  • Ran all three projects' unit tests.

Close MegaMek/megamek#5252

@Sleet01 Sleet01 requested a review from SJuliez April 15, 2024 07:25
@HammerGS
Copy link
Member

We’ll need to test with record sheets. As short name is used to make sure they print without some of the crazy l long names.

I can do that latter today.

@Sleet01
Copy link
Collaborator Author

Sleet01 commented Apr 15, 2024

We’ll need to test with record sheets. As short name is used to make sure they print without some of the crazy l long names.

There's an alternative fix where we add "Ammo" and "[Half]" or "[Full]" back to the short names as needed that I also tested; if this one isn't producing the desired results I can go back to that approach.

@Sleet01
Copy link
Collaborator Author

Sleet01 commented Apr 15, 2024

@HammerGS Here's a JPG of a PostScript file saved from the MML print dialog:

pdf_output

The update doesn't seem to impact how the printout displays Ammo on sheets, just whether Ammo equipment in the Critical equipment list is shortened or not.

@Sleet01
Copy link
Collaborator Author

Sleet01 commented Apr 15, 2024

@HammerGS Worst case scenario (200 ton SH Tripod loaded with as many ammos as possible):

Screenshot_20240415_122738 (All the ammo using regular names in the Crit Tab's equipment list)

pdf_output2
(Record Sheet is mostly legible, if a bit crowded)

@HammerGS
Copy link
Member

That's one intense unit. But that should work, mutators are always nasty for names and CGL doesn't use special ammos. So I'm good with it.

@Sleet01 Sleet01 merged commit 551f8ef into MegaMek:master Apr 15, 2024
4 checks passed
@Thom293
Copy link

Thom293 commented Apr 15, 2024

Does this solve MegaMek/megamek#5159 too? just saw it when looking for something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants