search

NagyD / SDLPoP

An open-source port of Prince of Persia, based on the disassembly of the DOS version.
GNU General Public License v3.0
1.1k stars 140 forks source link

A possible sequence table bug #282

Closed dstarosta closed 2 years ago

dstarosta commented 2 years ago

The number is outside the "dx" macro brackets. If that's a bug would it affect the byte integrity of the sequence?

https://github.com/NagyD/SDLPoP/blob/96fae3987f532551cf5d14319e6c13e72842b418/src/seqtbl.c#L784

NagyD commented 2 years ago

Good catch!

I fixed it: 5d9d7d3c71216d84fbe8a62cc9d7c27b213506ef I also added parentheses to macro definitions to catch these kind of mistakes.

If that's a bug would it affect the byte integrity of the sequence?

I don't think the fix changes the resulting byte sequence. With the original definition of dx, both dx(-)2 and dx(-2) expand to SEQ_DX, (byte) -2. With the new dx, dx(-2) expands to SEQ_DX, (byte) (-2) which means the same. dx(-)2 expands to SEQ_DX, (byte) (-)2 which is a syntax error.

But to be sure, I recompiled with CHECK_SEQTABLE_MATCHES_ORIGINAL enabled (and USE_SUPER_HIGH_JUMP disabled). SDLPoP says this on the console on startup:

Checking that the sequence table matches the original DOS version...
All good, no differences found!