Only consider non-paused armaments (if any exist) when determining max range for attack

[darkademic]

Fixes #20901

AttackBase has this:

// Use valid armament with highest range out of those that have ammo
// If all are out of ammo, just use valid armament with highest range
armaments = armaments.OrderByDescending(x => x.MaxRange());
var a = armaments.FirstOrDefault(x => !x.IsTraitPaused);
if (a == null)
	a = armaments.First();

Whereas in the Attack activity paused armaments are still considered when calculating the max range, which means, if a paused armament has less range than the active armaments, the unit will stop when reaching the real max range, but not shoot because the Attack activity is using the max range of a paused armament.

[PunkPun]

Tbh this feels more like a hack. It's intended for paused armament ranges to be considered. Perhaps we need to rethink how we combine the ranges of multiple weapons instead?

[darkademic]

Tbh this feels more like a hack. It's intended for paused armament ranges to be considered. Perhaps we need to rethink how we combine the ranges of multiple weapons instead?

Hm, can you elaborate?

AttackBase ignores paused armaments when calculating min/max range in GetMinimumRange(), GetMaximumRange(), GetMinimumRangeVersusTarget() and GetMaximumRangeVersusTarget(), as well as in CanTargetActor() and CanTargetLocation(), so this is just changing Attack to do the same, as per the comment:

// We want actors to use only weapons with ammo for this, except when ALL weapons are out of ammo,
// then we use the paused, valid weapon with highest range.

[PunkPun]

I see. So the current system is already hacky. Only GetMaximumRangeVersusTarget has a fallback on non-paused weapons that isn't 0.

If we were to follow that logic, we should fallback on 0 for minimum range, and then as fallback for max range, we should use the highest max range instead of the lowest max range.

[PunkPun]

also a comment should be added for this weirdly specific logic, perhaps the one you just quoted

[darkademic]

I see. So the current system is already hacky. Only GetMaximumRangeVersusTarget has a fallback on non-paused weapons that isn't 0.

If we were to follow that logic, we should fallback on 0 for minimum range, and then as fallback for max range, we should use the highest max range instead of the lowest max range.

I wondered if AttackBase.ChooseArmamentsForTarget() should actually be excluding the paused armaments, but the armaments returned by that are also used in AttackFollow to determine whether the target can be aimed at, so that'd need further changes.

Falling back to zero for minimum range seems wrong, as you'd expect a unit to try to make sure it's at the correct range for when the paused condition expires, but happy to keep it consistent with AttackBase so this remains a targeted fix and doesn't become more of a rework.

[PunkPun]

LGTM

[dnqbob]

I think it is intended, as for Armament we have pause and disable (RequiresCondition), which means when "pause", the Armament is not abandoned by actor, and actor will abandon this Armament when it is "disabled". I believe this setting is part of the game logic.

For example, an unit has long range weapon1 and short range weapon2 (like a artillery with a gunner on it), and it perfer weapon1 when attack, only use short range weapon when long range weapon is considered dead by it. In order to do that, we can use "PauseOnCondition" when it get emp and cannot use its weapon1 temporarily, and "RequiresCondition" when its barrel is destroyed to force it use weapon2.

[abcdefg30]

Changelog