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

Offline Delian

  • Colonel
  • ****
  • Posts: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1110 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: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1111 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: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1112 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: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1113 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: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1114 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: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1115 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: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1116 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: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1117 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: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1118 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: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1119 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: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1120 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: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1121 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: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1122 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 »

Offline Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 2873
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1123 on: June 16, 2022, 06:06:22 pm »
Paths spend more than 50TU, 3TU diff is 6%, if it was 20TU or more then I could worry and consider this bug.

Simply as there is new way calculate cost there could be +/- 1 difference in some cases. And simply old paths now cost more because of this and game choose other.
« Last Edit: June 16, 2022, 07:01:10 pm by Yankes »

Offline Delian

  • Colonel
  • ****
  • Posts: 228
    • View Profile
Re: OXCE (OpenXcom Extended) main thread
« Reply #1124 on: June 25, 2022, 05:00:49 pm »
I don't think you understand. The A-Star algorithm, Dijkstra etc., these are supposed to be optimal path algorithms. It means that they always produce the shortest/optimal path from point A to point B. There is no "only 6% difference". Either they find the shortest path, or they don't. And if they don't, it means they're broken/bugged/incorrectly implemented.

I spent an hour looking at the code, but I couldn't find what change caused the bug...
So for now, the best I can help is with providing a better save file and a few more screenshots.