Diagonal Screen Scrolling Crashes

[ZoriaRPG]

@DarkDragon
I had to make a slight adjustment to this commit:

https://github.com/ArmageddonGames/Z...017509e0892f14

it is now:

https://github.com/ArmageddonGames/Z...ae979aea78d26c

I think that it fixes the crash that Lut reported, but I do not know if it un-fixes the crash that you found.

What was the crash that you fixed, and how do I replicate it, to test if this reintroduces it?



Here is a build with the change:
http://timelord.insomnia247.nl/zc/zc...in_Beta_11.zip

[DarkDragon]

I don't think that's the problem.

The data[] array has size 176; the upper four bits come from cy and the lower four from cx. (160 & 0xF0) is the same as (168 & 0xF0) so changing the bound from 160 to 168 won't make any difference.

The best way to find the bug, if you are able to reproduce it, is to run ZC in a debugger and get a stack trace at the moment the out-of-bounds access occurs. I don't think the bug could be in either of the lookahead functions: it's easy to check that the maximum possible value of "combo" is

(160 & 0xF0) + (240 >> 4) = 175 < 176

as required, and clearly combo >= 0.

[ZoriaRPG]

I don't think that's the problem.

The data[] array has size 176; the upper four bits come from cy and the lower four from cx. (160 & 0xF0) is the same as (168 & 0xF0) so changing the bound from 160 to 168 won't make any difference.

The best way to find the bug, if you are able to reproduce it, is to run ZC in a debugger and get a stack trace at the moment the out-of-bounds access occurs. I don't think the bug could be in either of the lookahead functions: it's easy to check that the maximum possible value of "combo" is

(160 & 0xF0) + (240 >> 4) = 175 < 176

as required, and clearly combo >= 0.

It did in fact, actually fix it, and 168 is the maximum y value in a number of other functions throughout link.cpp. Compare to WalfFlagInfo(), blits of the playing field, and the pixel shift for red_shift(). Why did you try to bound it to 160?

I still do not know what bugs you originally intended to fix with the clamp change:

Fixed crashes when moving too fast.

What crashes? How do I reproduce them?

[DarkDragon]

It did in fact, actually fix it,

Nope (I already explained the math in my previous post). Think carefully about what the code is doing and which bits of y affect the final result.

If the bug is gone it is due to an unrelated change you made (or insufficient testing).

Don't believe me? Try to find a single value of y for which the old and new versions of the functions behave differently (or look again at the code and think about the math.)

I still do not know what bugs you originally intended to fix with the clamp change:

It's a short function and the only relevant line is:

int combo = (cy&0xF0)+(cx>>4);
return TheMaps[currmap*MAPSCRS+s].data[combo];            // entire combo code

Think about what this code is doing and then answer these questions:

1. What happens if the value of cx is greater than 255?
2. What happens if the value of cy is greater than 175?
3. How should cx and cy be clamped to fix these issues (there are multiple possible correct solutions; 160 and 168 behave exactly the same for cy, for instance, as explained above.)

Don't make changes to the code based on guesses or "values in a number of other functions." Think about the code and be sure.