OXCE (OpenXcom Extended) main thread

[Shiroi Bara]

You need at least 7.5.14 to get the fixes that went in in Apr 24.

Oh God I'm really blind, instead of checking list carefully I picked just bottom version from  it. The latest in the middle. No button anymore. Thank you! And my apologizes, Meridian.

[Yankes]

Yeah, I know game always used 1.5. I'm just arguing that 1.5 is a simplification of √2 and that √2 is what should have been used as per standard hypotenuse equation. But programmers were lazy and now we have to deal with these rounding errors haha

According to IEEE 754 floating point standard, 4.5 rounds to 4, so I don't know what you mean with "standard roundup".
Your second, what I would call "extreme" example of 1.5, according to the standard, that would be rounded to 2.

Anyway, can you change it back so that default diagonal running cost is the same as before? Thanks.

No, current behavior is caused by refactor that allow custom move costs. More probable is that OXCE will stay with current behavior.
I will look on code to see how to handle it, but if it can't be easy fix then will stay as is.


btw if 1.5 was only "lazy" solution then you should accept fact that diagonal move should cost 1 when horizontal was 1 as `round(1.41) == 1`.
1.5 is only same solution when you work with discrete move costs.

[Delian]

if it can't be easy fix then will stay as is.

Should be easy.
https://en.cppreference.com/w/cpp/numeric/math/rint
rint() would round correctly (so would nearbyint()), assuming FE_TONEAREST (default) is used.

[Yankes]

Its not easy, first of all you contradicted yourself, one time you require cost to be 3 -> 4, another time 1 -> 2, you can't have both as one use 1.41 another 1.5.

Beside to round up I use integer div, and even more this is used to all rounding in move cost even for horizontal paths, this is why its hard, change to it will change other cases too.

[Delian]

Sorry about the confusion. I think you should keep the 1.5x diagonal multiplier, because of backwards compatibility. And only change the rounding.

change to it will change other cases too.

Which other cases? As far as I can tell, changing rounding would only change behavior back to how it worked in version <=7.5.5

[Yankes]

Change to what exactly? You use in your initial example 1.41 that give you 3 -> 4, but if code use 1.5 then 3 -> 5 because 5 is closest to 4.5
And this is current behavior.

And I stress again, I did not change rounding in that version I changed WHOLE calculating of move cost. I can't revert it without removing whole new functionality.

[Delian]

Code: (from Pathfinding::getTuCost()) [Select]

...
else if (bam == BAM_RUN)
{
timeCost *= 0.75;
energyCost *= 1.5;
}
...
if timeCost is 6, then 6 * 0.75 = 4.5 -> conversion to int rounds it down to 4.
if energyCost is 3, then 3 * 1.5 = 4.5 -> conversion to int rounds it down to 4.
This is how it worked before. It was being rounded down. Or, to be precise, floor was being used.

Code: [Select]

const auto timeCost = (cost.TimePercent + (costDiv / 2)) / costDiv;
const auto energyCost = cost.EnergyPercent / costDiv;
This is the new code. Here you're rounding tu cost up, but rounding energy cost down.  In one of your commits you fixed the energy cost. Why didn't you fix the tu cost as well, if you were aware of the problem?

The solution? Change

Code: [Select]

const auto timeCost = (cost.TimePercent + (costDiv / 2)) / costDiv;into

Code: [Select]

const auto timeCost = cost.TimePercent / costDiv;and the problem should be solved -> rounding will be same as before.

[Yankes]

Yes, you find place when rounding is done, but this rounding handle ALL rounding, as I point previously now cost 1 will change to cost 1 for diagonal as it is not rounding but floor.

This will cause that if move cost is 99% then every move will cost 1TU less as intermeddle cost will be like 4.96 and this have floor equal 4.

Probably more correct solution would be subtract 1 and this will case that exactly 0.5 will be always round down to 0 not to 1.

[Delian]

That's a valid point. Yeah, I'd like it if 4.5 is rounded to 4, and 4.96 is rounded to 5.

[Yankes]

I did push this change to main branch 7.5.16 should have exactly 0.5 round to 0.0 for TU

[Delian]

I think you could've done the same for energy
const auto energyCost = (cost.EnergyPercent - 1 + costDiv / 2) / costDiv;

[Yankes]

For now Meridian said that for now leave it as is.
https://github.com/Yankes/OpenXcom/commit/66d70f0ca7dd751e7c34cf296aaa0fe5d1f6839f
there some potential changes I plan to add in future.

[Delian]

Ok, I tested it and running diagonally now costs 4 tu as before. Thanks.

While testing, I found another change, unrelated, but still I think it's problematic.
Pathfinding in version 7.5.16 is worse than in 7.5.3

.sav file piratez v.M5.2

[Yankes]

I do not plan change it if whole path only differ by 1TU, at some point you need accept new behavior.

[Delian]

What about 3TU? So you're saying that bad pathfinding isn't an issue?

Edit: I should note that running and walking produces two different paths. Walking produces the same path as before (like in 7.5.3, a better path), but running produces a different, less-optimal path, so maybe the issue is there somewhere.