FAForever / fa

Lua code for FAF
221 stars 228 forks source link

Fix bombers not keeping track of attack ground tries #6277

Closed lL1l1 closed 1 week ago

lL1l1 commented 1 week ago

Description of the proposed changes

Stops bomb dropping weapons from resetting their target mid-bombing run, since that caused them to reset their AttackGroundTries counter, which meant they would never move on to the groundfire target.

Testing done on the proposed changes

Give a bomber two groundfire orders, it should switch to the next order after attacking 3 times. Note: It won't drop a bomb on the third pass, but this is due to a separate bug and does not impact what is being fixed here.

Additional context

Cherry pick from #6166.

Checklist

Garanas commented 1 week ago

I'm wondering why we should reset the target in general. It appears to be introducing a lot of (difficult to spot) issues in general.

In the past we've already reduced the tracking radius of weapons. See also:

That means weapons will (in general) not try and track targets that are far outside of their range as it defaults to 105%.

lL1l1 commented 1 week ago

I know of only 2 issues target resetting causes: this issue with bombers resetting, and the constant FireReady -> Idle/Packing -> FireReady state cycling (I fixed this in that closed PR, I'll cherry pick soon). I think having these small issues is better than ever having a scenario where weapons lock on to a target at 105% range (or more for modded units with old code style, since we do not override blueprint tracking range) and then never switching to something nearby.

Garanas commented 1 week ago

I think having these small issues is better than ever having a scenario where weapons lock on to a target at 105% range (or more for modded units with old code style, since we do not override blueprint tracking range) and then never switching to something nearby.

But how often does this actually happen in practice? And does this reset actually fix it?

And we can override the track radius if we deem it harmful, just like some other values are overwritten regardless of what is in the blueprint (target check interval, size of air units, etc)

Garanas commented 1 week ago

Before we do that discussion, let's first focus on merging this in. All improvements are improvements afterall 👍