Fix #2178: testLineAgainstWater works incorrectly outside of game boundaries

[Allerek]

Just added a check for water level outside Game Area, and commened out a check if both points are outside Game Area(why would we check for that lol).

Fixes #2178

[forkerer]

The commented out check is an early out if both ends are on the same side of the world border, thus skipping all the further calculations, i think that the GetZonesIntersecting return zones at the world border even when both points are ourside of the -3000 : 3000 range
Red arrows show check that will be skipped, while the black one will go through

[forkerer]

I don't see a reason to remove that check, and i doubt you tested those cases or checked what happens in the code further down

[Allerek]

I don't see a reason to remove that check, and i doubt you tested those cases or checked what happens in the code further down

Because thats what this PR is all about, if both of the ends will be outside area, it will automatically return false, we dont want that to happen, we want to be able to check if line is hitting the water in any case, and yeah - i tested that and it worked just fine.

Some users might want to get out of Game Area(for some reason, i'm not here to judge), and we should allow that.

[forkerer]

And they still can, the code you commented out doesn't change anything about that, it just skips unnecessary calculations later on. No water other than ocean water will ever be collided outside of -3000 : 3000 range because You can't create custom water outside that

[forkerer]

Check for the GetZonesIntersecting behaviour when you check line starting from (-3050, -3050, 10) to (3050, -3050, 10) when ocean water level is 0, it will do unnecessary checks around the world border if I'm not mistaken after You commented the code out

[Allerek]

And they still can, the code you commented out doesn't change anything about that, it just skips unnecessary calculations later on. No water other than ocean water will ever be collided outside of -3000 : 3000 range because You can't create custom water outside that

Yeah, but the code returns false, it doesnt even check if the line really collides with water, as far as i see it only checks for X and Y.

-Are both points outside of map?
-If yes then check if they're on the same side of the map
-If so, just return false for some reason? < That doesnt make any sense, whats the reason to suggest theres no water in its way what soever?

[forkerer]

You literally check for ocean water collision few lines up, no other water will ever exist there, as it can't be created

[forkerer]

That's the only water you can encounter outside of game zone

[Allerek]

After thinking it out, you might be right, further code checks for waves as i see? Forgot the ocean water doesnt have waves at all, so that can make sense. Getting commented code back, leaving the higher fix tho.

[Allerek]

After all the checks are done, i guess we're ready to merge, as the change itself worked well in my tests.

[Allerek]

watertest.zip
Test resource.

(first number-current water level, second is endz for testLineAgainstWater)

[Allerek]

For some reason it returns true if water level is lower than 0, and we check at Z=0(or lower), i guess its due to
GetWaterLevel(CVector(3000,3000,1), &waterHeight, true, nullptr);
Where i check for Z level '1', i guess we could just set it to a very low number, but this aint the best solution i guess. Any ideas? @forkerer

[patrikjuvonen]

For some reason it returns true if water level is lower than 0, and we check at Z=0(or lower), i guess its due to
GetWaterLevel(CVector(3000,3000,1), &waterHeight, true, nullptr);
Where i check for Z level '1', i guess we could just set it to a very low number, but this aint the best solution i guess. Any ideas? @forkerer

Did you check his suggested fix in the original issue #2178?

[Allerek]

For some reason it returns true if water level is lower than 0, and we check at Z=0(or lower), i guess its due to
GetWaterLevel(CVector(3000,3000,1), &waterHeight, true, nullptr);
Where i check for Z level '1', i guess we could just set it to a very low number, but this aint the best solution i guess. Any ideas? @forkerer

Did you check his suggested fix in the original issue #2178?

i got some help, i'll introduce it soon but for now i have some work to do

[Pirulax]

LGTM

[Pirulax]

Well I hope you've tested it.