search

FAForever / fa

Lua code for FAF
226 stars 232 forks source link

Fix `AttackGroundTries` and Othuy Lightning Storm #6166

Closed lL1l1 closed 4 months ago

lL1l1 commented 4 months ago

Description of the proposed changes

Fixes the ability of AttackGroundTries for bombers and fixes an off by one issue for other units. Annotates the field. As a bonus, fixes lightning storm spawned from Ythotha too (cheat menu spawn and Ythotha spawn act differently).

Testing done on the proposed changes

Checklist

lL1l1 commented 4 months ago

The main change is the salvo retargeting feature. An issue that was being solved before this PR is that salvos can be restarted from shot 0 if you cancel the firing through OnLostTarget in the middle of the FiringState. Timeline:

  1. Before this PR, HaltFireOrdered solved restarting salvos, but that causes AttackGroundTries to be off by one since the last shot isn't fired because it goes ReadyState.OnFire -> FiringState.OnLostTarget immediately (halt fire is ordered immediately) so the firing state doesn't get time to shoot.
  2. In the state at 039dfe76b, this isn't possible, but an issue that pops up is that salvos will keep shooting even if the target is lost.
  3. This makes me create the salvo retargeting feature which has to:
    • keep track of how much of a salvo has been fired across states
    • if there's an incomplete salvo, allow the weapon to restart the salvo immediately when it is allowed to fire instead of waiting for OnFire
    • To be honest I'm not sure if tracking targets in lua is required by the retargeting feature. I think I implemented that because before FAF's target resetting fix weapons were allowed to stay in FireReadyState forever. FAF's fix uses ResetTarget which calls OnLostTarget which changes the weapon state to idle/packing, unlike the vanilla behavior. That state change can happen constantly if the weapon has a target in tracking radius but not firing range, which I suppose can lead to unexpected animation or FX.

The retargeting feature does solve being able to restart salvos while still shooting on the last shot of AttackGroundTries. It also allows cool retargeting in the middle of a salvo with different orders, so you can't lose an entire salvo to a misclick (for example scathis). The only remaining issue is that weapons do not call OnLostTarget when they retarget due to target priorities or the target dying. This causes a ravager to not stop firing when it switches from a striker tank to another striker tank (because the old one died) or to an Othuum (because the othuum has higher priority). I think it's a pretty minor issue though.

lL1l1 commented 4 months ago

I'm going to have to split the various changes in this PR.