fix: canTargetUnit() scanning algorithm

[narical]

Overview:

1. Fix inconsistent target centre calculation
2. Fix inconsistent number of horizontal slices
3. Fix incorrect calculation of top/bottom boundaries
4. Fix small bug with bottom boundary
5. Is that true to the original XCom algorithm?

Details

From the current version of the code:

center height = (minHeight + maxHeight) / 2
number of horizontal slices = heightRange / 2
number of horizontal slices = MAX( 10, number of horizontal slices )

Visualizing canTargetUnit in its current state gives us this picture:

This means that 5 of 16 types of units in the base game are significally shorter than they should be, in terms of visibility and detection, with soldier being the leader - 4 voxels shorter, ~18% of its height. Another 2 units are 2 voxels shorter, and another 4 units are 1 voxel shorter each.

How it looks like (mutons should always be visible): download video

In order to fix that, I split the issue to three parts.

1. We need to make sure that the difference between max unit height and it's centre is an even number. We scan voxels "in pairs", assuming that if we hit a voxel - there's another one underneath it (with exception for bottom plane of floaters and cyberdisks). In that regard, we need the topmost voxel of units to be "scannable". So, the very first step is to increment centre Z coordinate by 1 in case we can't hit the top voxel while gradually incrementing centre coordinate by 2.

2. The most glaring issue - we can see that the code misses two offsets: -10 and +10. That's why it just misses tops/bottoms of units entirely. Fix is trivial.

3. We need to make sure that we have enough scan levels. Logically, number of those levels above the centre should be equal to the one below it - considering some variations between the centre and min/max points. So, just increment levels by 1 if we get odd number of them. For example, cyberdisk on the picture above is totally fine with its centre level but as it has height range of 15, it gets (15/2=7.5 rounded down to) 7 scan levels, and that's 1 level less than needed to scan all its height including topmost voxel.

My suggested fix looks like this:

center height = (minHeight + maxHeight) / 2
center height += (maxHeight - center height) % 2 // add 1 if (max-center) is odd
number of horizontal slices = heightRange / 2
number of horizontal slices += number of horizontal slices % 2 // add 1 if odd
number of horizontal slices = MAX( 12, number of horizontal slices ) // was 10 

Vizualisation looks like this:

The next issue is "misses". They are not the problem by themselves, cause they just skip a loop cycle and that's it. The real issue is - according to current code logic, they are used to determine if some voxel lies on top or bottom plane of target's cylinder. Why is that important?

Here's an example (red voxels - current version, ignore the commit pls).

From Volutar's words, it has the following logic. There are three vertical lines. The game ignores front and back lines (red voxels) most of the time, and scans point on the unit's axis. For front/back lines, it scans only their top/bottom points. They are needed to check visibility "in 3D", when you're looking to target at the steep angle from above or below. In that case, all of target's frontal section (including axis) could be hidden behind an obstacle, but the point on the back line could still be visible.

But it never worked the way it was intended.

	// scan ray from top to bottom  plus different parts of target cylinder
	for ( int i = 0; i <= heightRange; ++i )
	{
		scanVoxel->z = targetCenterHeight + heightFromCenter[ i ];
		for ( int j = 0; j < 5; ++j )
		{
			if ( i < (heightRange - 1) && j > 2 ) break; //skip unnecessary checks
	......

Game decides to check front/back lines only for "heightRange-1" and "heightRange" indexes, that corresponds to "outermost" height values, which are displayed as "misses" on the first picture. Which effectively turns 3D-Muton into 2D one.

My suggested solution for this issue - directly compare the actural voxel height against unit top/bottom voxels.

	// Skip unnecessary checks
	if (j > 2 && scanVoxel->z > targetMinHeight+2 && scanVoxel->z < targetMaxHeight) break;

Here comes "small bug with a bottom boundary". targetMinHeight doesn't represent its lowest voxel. For non-floating units, standing on Z-level 0 (with zero terrain elevation), targetMinHeight is 0. It's the level of the floor, or ceiling of a lower tile. Checking for unit boundaries should start from targetMinHeight+1.

In my code above, I check agains targetMinHeight+2. If you look at second picture - any lowest scan hits either 1st or 2nd voxel. That's why we should exclude them both.

About "true original X-Com algorithm"

In OG, algorithm was different. It was fixed N-S-E-W cross, with 4 horizontal slices - for top, bottom, 1/4 and 3/4 height. Current implementation was intended to check top and bottom of the unit, like the original one did, but fails to do so. Fixing it (by making it work as intended) will make it closer to OG.

P.S. English is not my language, I haven't had any sleep tonight, and I apologize for possible mistakes.

[killermosi]

Is that screenshot taken from the soldier's "eyes" or "gun"? IIRC, in the vanilla XCom there were situations when your soldier could see an enemy, but not shoot at it.

[narical]

Is that screenshot taken from the soldier's "eyes" or "gun"?

3D-view is from eyes, as far as I know

IIRC, in the vanilla XCom there were situations when your soldier could see an enemy, but not shoot at it.

function canTargetUnit() is used both for line of sight and line of fire, the former one begins at the eyes level, the latter is from weapon. That's nothing unusual in "can see, cannot shoot" situation. Fixing canTargetUnit() will improve both unit detection and finding line of fire.