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

Handle one-shot ammo weapons like RLs appropriately #5433

Merged

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Apr 29, 2024

After ammo selection was added for Princess, she no longer fires Rocket Launchers, for two reasons:

  1. The to-hit threshold for weapons with only one remaining shot of ammo was far too low, near 50% / 6+ TN.
  2. OS weapons, although using linked ammo bins like regular weapons, use a fake location that causes checks of ammo validity to fail.

Since Princess verifies if prospective ammo for a weapon can be loaded into it when planning attacks, and OS ammo (like RL's single shot) the end result is that Princess skips OS weapons when actually creating her firing plans.

The first issue has already been fixed by adding specific thresholds for OS weapons based on Princess's aggression setting, which I've confirmed are generating the appropriate thresholds.

The final fix is to only check if a OS weapon's ammo still has a shot left and that it is of the appropriate type, rather than running the full gamut of checks used for regular ammo (shots left, validity, whether it is missing, damaged, or dumping, as well as valid location) since most of those states don't apply.

Testing:

  • Ran existing unit tests for all three projects (MHQ tests currently broken by other changes)
  • Ran Princess vs. Princess matches using RLs on a variety of unit types and confirmed firing

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.

Sometimes when I look at changes like this I wonder, how has this ever worked?

@SJuliez SJuliez merged commit 46b581f into MegaMek:master Apr 30, 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.

2 participants