aliens

Author Topic: OXCE (OpenXcom Extended) main thread  (Read 445881 times)

Offline Shiroi Bara

  • Colonel
  • ****
  • Posts: 325
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1095 on: June 15, 2022, 10:59:36 pm »
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.

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1096 on: June 15, 2022, 11:02:42 pm »
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.

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1097 on: June 15, 2022, 11:34:49 pm »
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.
« Last Edit: June 15, 2022, 11:41:13 pm by Delian »

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1098 on: June 15, 2022, 11:55:22 pm »
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.

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1099 on: June 16, 2022, 01:02:49 am »
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
« Last Edit: June 16, 2022, 01:08:26 am by Delian »

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1100 on: June 16, 2022, 01:37:07 am »
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.

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1101 on: June 16, 2022, 02:05:51 pm »
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.
« Last Edit: June 16, 2022, 02:27:45 pm by Delian »

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1102 on: June 16, 2022, 02:23:40 pm »
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.

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1103 on: June 16, 2022, 02:45:47 pm »
That's a valid point. Yeah, I'd like it if 4.5 is rounded to 4, and 4.96 is rounded to 5.

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1104 on: June 16, 2022, 04:29:28 pm »
I did push this change to main branch 7.5.16 should have exactly 0.5 round to 0.0 for TU

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1105 on: June 16, 2022, 04:36:33 pm »
I think you could've done the same for energy
const auto energyCost = (cost.EnergyPercent - 1 + costDiv / 2) / costDiv;

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1106 on: June 16, 2022, 04:48:51 pm »
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.

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1107 on: June 16, 2022, 04:56:33 pm »
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


« Last Edit: June 16, 2022, 05:10:41 pm by Delian »

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 3194
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1108 on: June 16, 2022, 05:25:18 pm »
I do not plan change it if whole path only differ by 1TU, at some point you need accept new behavior.

Offline Delian

  • Colonel
  • ****
  • Posts: 240
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1109 on: June 16, 2022, 05:49:30 pm »
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.
« Last Edit: June 16, 2022, 06:04:55 pm by Delian »