Issue #1084 - Allow choosing target tile for meleeing adjacent targets

[Iridar]

Fixes #1084

Also adds a couple helper methods into CHHelpers to be used by this issue or mods.

---

I've traced the problem to X2MeleePathingPawn::UpdateMeleeTarget(), which calls very native class'X2AbilityTarget_MovingMelee'.static.SelectAttackTile().

When melee-ing adjacent targets, SelectAttackTile() only writes one tile into the PossibleTiles out array.

I suggest addressing this issue by inserting custom unreal script logic that will run in case SelectAttackTile() gives only one tile. Said custom logic will simply take a look at the target, and insert more tiles into the PossibleTiles array.

If it sounds good, I could use some help in setting up that custom logic so that we only insert tiles that are actually reachable by the soldier.

[Iridar]

Hmm I just realized class'X2AbilityTarget_MovingMelee'.static.IsValidAttackTile() exists, I'll see if that can be used here.

[Iridar]

Okay, I generally got it working, the only problem is that now I'm unable to attack from the tile the attacker is standing on. I.e. the situation is reversed, previously you could only attack from the original tile, now you can attack from every tile except the original one.

The tile the attacker is standing on is selected by default, but once you move the cursor from it, you cannot return, until you cancel out ability target.

I think some piece of logic somewhere disallows attacking from the original tile, because that tile is occupied, and doesn't check that it's occupied by the unit itself. Currently looking for that logic.

[Iridar]

Traced the problem to XComPathingPawn::RebuildPathingInformation() but oh boy there's a lotta code there.

[Iridar]

Apparently Cursor.GetCursorFeetLocation() raises the Z of the returned tile if the cursor is hovering on a tile occupied by a unit (the melee attacker). So CursorTile will never be directly equal to AttackerUnit.TileLocation, which borks the game's logic in at least two separate places.

[Iridar]

Disregard the previous comment, it was only one place. Should be working now. Note: I've added support only for mouse targeting, I don't know if the controller targeting requires special handling in this regard and don't plan to do a single thing about it.

[Iridar]

Note to self: investigate maybe using this:

// Find all adjacent tiles to target that are valid for melee attacking. This only looks at tiles adjacent to the target.
// ( Currently written only for attackers of UnitSize 1. )
static native function bool FindTilesForMeleeAttack(XComGameState_Unit Target, out Array Tiles);

[Iridar]

static native function bool FindTilesForMeleeAttack(XComGameState_Unit Target, out Array Tiles);

The moment I started looking into it, I realized that this function only takes a unit as the potential target, so it wouldn't work for destructible objects like alien relay. The code that is currently in this PR does work in this case.

In theory I could try using the FindTilesForMeleeAttack() against units and my own code against destructible objects, but I don't think it's worth the effort and added complexity. My code is simple and not calculation-intensive, and the calling function itself is not hot code, it runs once when the targeting method selects the specific target.

As such, I think current implementation should stand.

[Iridar]

Current version of this PR does not properly interact with large units, like Sectopods, working on a fix.

[Iridar]

Current code can probably be further optimized, particularly this cycle:

foreach DestructibleActor.AssociatedTiles(AssociatedTile)
{
	AdjacentTiles = class'CHHelpers'.static.GetTilesAdjacentToTile(AssociatedTile);

	// There is no FindTilesForMeleeAttack() equivalent for non-unit targets, so we have to manually test each potential attack tile.
	foreach AdjacentTiles(AdjacentTile)
	{
		if (class'Helpers'.static.FindTileInList(AdjacentTile, PossibleTiles) != INDEX_NONE)
			continue;

		if (class'X2AbilityTarget_MovingMelee'.static.IsValidAttackTile(UnitState, AdjacentTile, AssociatedTile, ActiveCache))
		{	
			PossibleTiles.AddItem(AdjacentTile);
		}
	}
}

The IsValidAttackTile() ends up getting called A LOT, since for every AssociatedTile, the GetTilesAdjacentToTile() returns 8 tiles, and some of those adjacent tiles may be inside the Destructible itself (in the AssociatedTiles array), so it would be pointless to test them as valid tiles to attack from.

There is also some overlap, as one adjacent tile can be adjacent to multiple associated tiles.

If the Destructible occupies several levels vertically, then we end up collecting and testing adjacent tiles that hang in the air as well.

I'm inclined to keep the code as is, as it seems to work just fine, and arguably the native function will run faster than any pre-validation logic we can run in Unreal Script anyway.

[Iridar]

@Xymanek done everything you asked.

[Xymanek]

Additional test cases to verify:

• Attacking large units (sectopods, etc)
• Ramps. There are "small" ramps (~1 half cover elevation change) and "big" ramps (1 floor elevation change). Need to test cases when either the attacker, the target or both are standing on a ramp tile

[Iridar]

Targeting on very mild slopes like this roof works fine: https://imgur.com/SnuqQat

And I already tested it multiple times for Sectopods, but here's a retest: https://imgur.com/a/e8AWSN3

However, you were right to be concerned about slopes: https://imgur.com/a/3mheYs2