Fix units with multiple weapons not attacking when attack-moving.

[pchote]

The underlying problem in #18935 was caused by inconsistent target validation logic in AutoTarget versus AttackFrontal's Attack activity: AutoTarget considers a target valid if any weapon is in range, but Attack only considers it valid if all valid weapons are in range.

This PR removes the all-weapon restriction if the unit isn't able to move, where it is clearly bogus.

As the comment notes, the existing logic is also bogus for units that define non-overlapping weapon ranges. I don't know how we want to solve this (its a game design issue, not a code bug), so I have left a TODO documenting the problem for now. If there is a clear consensus on how to solve that, then I can update the PR to implement it here.

[deleted-user-1]

If target is in range of any valid weapon, fire.
If target is not in range of any valid weapon, move into weapon range of any valid weapon.
If the unit could use another weapon at closer/further range but can't use that one now but another, let the player decide.

[reaperrr]

If the unit could use another weapon at closer/further range but can't use that one now but another, let the player decide.

I was going to write something along those lines. If a one-size-fits-all solution cannot be accomplished in code only, leave the choice to the modder.

From a modders' perspective, I'd want support for both:
Artillery units that stop moving closer once in range of their main weapon even if they have short-range secondary armaments for close combat defense, as well as tanks with powerful short-range cannon and weak long-range missiles that fire their missiles while continuing to move into cannon range.

[pchote]

Artillery units that stop moving closer once in range of their main weapon even if they have short-range secondary armaments for close combat defense, as well as tanks with powerful short-range cannon and weak long-range missiles that fire their missiles while continuing to move into cannon range.

This is exactly what I was thinking too, but i'm not sure how to best expose this to yaml. My current idea is to use the armament names listed on Attack*.Armaments as a priority list: so if a unit defines Armaments: primary, secondary it will always try to move so that at least one primary armament is in range.

[deleted-user-1]

Or imagine a unit with long range splash damage weapon and short range direct damage. Players should IMO have to move their units into a position that's best for a given situation, instead of relying on the engine to automate this task for them.

[pchote]

That would mean that Mammoth tanks would treat their missiles as their primary weapon, and require micro if you want them to use their main cannon. That's a deal breaker, IMO.

[deleted-user-1]

That would mean that Mammoth tanks would treat their missiles as their primary weapon, and require micro if you want them to use their main cannon. That's a deal breaker, IMO.

Not sure if I understand. Missiles are invalid against ground vehicles so if you target a tank it would need to move into cannon range anyway. I don't know if cannons are valid against infantry, but yes, if they are and the mammoth is in missile range, you'd need to move closer. I can't really see how that's a dealbreaker tbh, especially since you don't want to move into cannon range against infantry with mammoths anyway.

But I see that designers might want to have that automation in their games, so perhaps it could be another configuration setting.

[reaperrr]

This is exactly what I was thinking too, but i'm not sure how to best expose this to yaml. My current idea is to use the armament names listed on Attack*.Armaments as a priority list: so if a unit defines Armaments: primary, secondary it will always try to move so that at least one primary armament is in range.

That might be good enough. Modders who need the priority to be toggleable on the fly could switch between Attack* traits with different priorities via conditions, I guess.

[pchote]

I don't see any complaints or further feedback, so have rebased and implemented the idea above.

[pchote]

Rebased.

[teinarss]

Got a crash on the shellmap in RA

System.InvalidOperationException
  HResult=0x80131509
  Message=Sequence contains no elements
  Source=System.Linq
  StackTrace:
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.Max[TSource,TResult](IEnumerable`1 source, Func`2 selector)
   at OpenRA.Mods.Common.Activities.Attack.TickAttack(Actor self, AttackFrontal attack) in C:\projects\OpenRA\OpenRA.Mods.Common\Activities\Attack.cs:line 203
   at OpenRA.Mods.Common.Activities.Attack.Tick(Actor self) in C:\projects\OpenRA\OpenRA.Mods.Common\Activities\Attack.cs:line 145
   at OpenRA.Activities.Activity.TickOuter(Actor self) in C:\projects\OpenRA\OpenRA.Game\Activities\Activity.cs:line 114
   at OpenRA.Traits.ActivityUtils.RunActivity(Actor self, Activity act) in C:\projects\OpenRA\OpenRA.Game\Traits\ActivityUtils.cs:line 37
   at OpenRA.Actor.Tick() in C:\projects\OpenRA\OpenRA.Game\Actor.cs:line 262
   at OpenRA.World.Tick() in C:\projects\OpenRA\OpenRA.Game\World.cs:line 422
   at OpenRA.Game.InnerLogicTick(OrderManager orderManager) in C:\projects\OpenRA\OpenRA.Game\Game.cs:line 632
   at OpenRA.Game.LogicTick() in C:\projects\OpenRA\OpenRA.Game\Game.cs:line 658
   at OpenRA.Game.Loop() in C:\projects\OpenRA\OpenRA.Game\Game.cs:line 823
   at OpenRA.Game.Run() in C:\projects\OpenRA\OpenRA.Game\Game.cs:line 864
   at OpenRA.Game.InitializeAndRun(String[] args) in C:\projects\OpenRA\OpenRA.Game\Game.cs:line 274
   at OpenRA.Launcher.Program.Main(String[] args) in C:\projects\OpenRA\OpenRA.Launcher\Program.cs:line 24

[reaperrr]

var armaments only includes weapons valid against this target.

In cases like Mammoth tanks, only secondary might be valid, resulting in primaryArmaments being null.

Maybe just change the above to FirstOrDefault and fall back to armaments if primaryArmaments is still null?

Alternatively, we could work on the assumption that primary and secondary will remain the only levels used in our official mods and many 3rd-party mods, and do something like this to try to fall back to secondary before falling back to bleed behavior:

var armamentsToDetermineRange = armaments.Where(a => a.Info.Name == attack.Info.Armaments.FirstOrDefault());
if (armamentsToDetermineRange == null)
	armamentsToDetermineRange = armaments.Where(a => a.Info.Name == attack.Info.Armaments.Skip(1).FirstOrDefault());

if (armamentsToDetermineRange == null)
	armamentsToDetermineRange = armaments;

[pchote]

I've lost motivation to try and get back into this. Parking on the future milestone until someone wants to take over or we encounter another bug where this would be an efficient fix.