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 for RAC and UAC ammo bingo state hanging game #4865

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Nov 1, 2023

I'm not sure if this has been around for a while, or is the fallout of recent changes, but we're seeing the game hang whenever two or more variable-rate ammo-based weapons (UACs and RACs, possibly others) end up with 0 remaining ammo during the weapon action handling phase of a turn. The root cause was that we explicitly assumed we'd never see an attack finding 0 rounds remaining when actually executed, so we'd have an attack set to make 1 shot with 0 remaining ammo... which made the game sad.

I don't want to go into much detail, but this was pretty hairy to debug due to the way that we call the same To-Hit calculation code all over the place in order to:

  • Show to-hit chance in the weapon pane of the UI,
  • Let Princess determine what things to shoot with which weapons,
  • Actually handle attacks,
  • Generate the turn-end report (?! this was bonkers to me)

Suffice to say, the issue comes because, while we try to detect the zero-ammo situation before declaring or handling attacks, weapons that have variable firing rates don't currently do any up-front accounting. The player-selected mode of a RAC/5, for instance, is not taken into account until we actually handle the attack and begin consuming the weapon's linked ammo. At that point it is far too late to display a warning to the user or prevent a 2nd weapon's attack action from being created, because all the attacks have already been created and we're just totting up the numbers behind the scenes.

I've implemented a fix that will prevent the hanging, and consolidated some of the duplicate code between the RAC and UAC handlers. But there's still a lot that could work better here:

  1. As it stands we just silently fail to execute any attacks where the ammo ran out prior to handling them. There's no indicator in the turn log that an issue has occurred. Given how long it took to debug this, I don't want to spend even more time trying to figure out a way to bubble the ToHit result with the "out of ammo!" message up to the logger and get it in the right place in the log messages.
  2. It looks like we may execute the attacks and post the results to the log in different orders; e.g. with 7 RAC/2 ammo and two RAC/2s set to 4 shots each, we will see the attack that gets its full 4 shots at the bottom, and the attack that only gets 3 shots (due to ammo starvation) above it in the turn-end log.
  3. We are doing a ton of extraneous ammo polling because we wait until the attack is actually resolved to consume ammunition; this would be much quicker if we had a transactional system to "debit" ammo from bins prior to creating the attack actions, and only return ammo in the rare case (e.g. Streaks) where an attack failed to use its allocated projectiles.

close #4861

Copy link
Member

@NickAragua NickAragua left a comment

Choose a reason for hiding this comment

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

Just one minor formatting change, the logic looks good.

megamek/src/megamek/common/weapons/RACHandler.java Outdated Show resolved Hide resolved
Fix formatting on switch/case
@HammerGS HammerGS merged commit 7de99f3 into MegaMek:master Nov 12, 2023
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.

[49.15] Hang on use of RAC/5 when insufficient ammo is available
3 participants