Sonic & Knuckles Collection C port

According to the wiki, all the data in S&KC is byteswapped. Maybe the compressed data is an exception? Because the x86 ASM does read like the C code you wrote for a big endian value.

I think you're looking in mapdevr.c, which has the Engima decompression code. The Nemesis decompression code is in bitdevr.c.

But corrections for mapdevr are welcome, too, so, thanks!

In the meantime I fixed another bug in the bitdevr codetable generation. Looking at the output in a hex editor has it looking even more off compared to the reference, but in-game it looks a little better.

 

Sorry, my bad. I meant Nemesis.

Maybe I should compare Enigma output next because at the moment I don't seem to be getting anywhere with Nemesis. The codetable generation should be correct now, yet the first repeat count retrieved during decompression is still wrong.

 

That causes a segmentation fault because the variable data no longer has the last read value, which it needs at the top of the outer loop.

I just reverted a bugfix because I now remember why I didn't subtract 1 at the end of this line:

Code (C):

1. unsigned short cnt = 1 << 8 - codebits_cnt;

The subtraction was there specifically so the developer could satisfy his obsession with the dbf instruction. Without it, I can count down to zero.

 

That's okay; you were only trying to help.

Any ideas for how I could test the Enigma decompression? I can't find an x86 implementation to compare my output with.

 

This just keeps getting frustratingly weirder.

I installed KENSSharp and tried to decompress the map. It threw up an error as it tried to read past the end of the data. I looked at my input data, and it's 459 bytes, even though the array I wrote it from has only 428 characters!

EDIT: Found it. I have to write in binary mode. Evidently, it's not because I've decompiled tons of C code that I have a lot of experience writing it.

...But KENS still tries to read past the stream.
...Except if you pick Big Endian. Looking more closely, only the header seems to be little endian.

EDIT2: The output, however, is little endian. That's going to make it harder to compare outputs.

I went as far as to replicate the x86 ASM. The first values of Enigma data remain wrong when compared to KENS's (byte-swapped) output. I'm out of ideas.

Expected output is

Code (Text):

1. 00 01 00 02 00 03 00 04 00 05 00 00 00 01 00 02 00

Actual output:

Code (Text):

1. 01 00 02 00 03 00 04 00 05 00 06 00 07 00 08 00 09

The code:

Code (C):

1. void mapdevr(unsigned short* p_destination, unsigned char* p_source, unsigned short offset) {
2.   mapdevrwk wk = { p_source };
3.   unsigned short a5, d0;
4.   a5 = d0 = *p_source++;
5.   //unsigned char inline_copy_bit_cnt = *p_source++;
6.   unsigned char d4 = *p_source++ << 3;
7.   //unsigned char bitfield = *p_source++ << 3;
8.   unsigned short a2 = *(unsigned short*)p_source++ + offset;
9.   //unsigned short incremental_copy = *(unsigned short*)p_source++ + offset;
10.   unsigned short a4 = *(unsigned short*)p_source++ + offset;
11.   //unsigned short literal_copy = *(unsigned short*)p_source++ + offset;
12.   wk.data = *p_source++ << 8; // d5
13.   wk.data |= *p_source++;
14.   wk.shift = 16; // d6
15.   bool done = false;
16.  
17.   do {
18.     d0 = 7;
19.     wk.entry_bit_cnt = 7;
20.     unsigned char d7 = wk.shift; // d7 = d6
21.     d7 -= d0;
22.     unsigned short d1 = wk.data;
23.     d1 >>= d7;
24.     d1 &= 0x7F;
25.     // unsigned char entry = (wk.data >> wk.shift - wk.entry_bit_cnt) & 0x7F;
26.     unsigned short d2 = d1;
27.     // unsigned char type = entry >> 4; (combination with later operation)
28.     if (d1 < 64) {
29.     //if (entry < 64) {
30.     //if (!(entry & 0x40)) { // entry < 64 ?
31.       d0 = 6;
32.       wk.entry_bit_cnt = 6;
33.       d2 >>= 1;
34.       //entry >>= 1;
35.     }
36.     mapdevr_nextbyte(&wk);
37.     d2 &= 0xF;
38.     //unsigned char repeat_cnt = entry & 0xF;
39.     d1 >>= 4;
40.  
41.     switch (d1) {
42.     //switch (type) {
43.       case 0:
44.       case 1:
45.         do {
46.           *p_destination++ = a2;
47.           ++a2;
48.           //*p_destination++ = incremental_copy++;
49.         } while (d2-- != 0);
50.         //} while (repeat_cnt-- != 0);
51.         break;
52.       case 2:
53.       case 3:
54.         do {
55.           *p_destination++ = a4;
56.           //*p_destination++ = literal_copy;
57.         } while (d2-- != 0);
58.         //} while (repeat_cnt-- != 0);
59.         break;
60.       case 4: {
61.         unsigned short inline_copy = mapdevr_getinline(&wk, offset, d4, a5);
62.         //unsigned short inline_copy = mapdevr_getinline(&wk, offset, bitfield, inline_copy_bit_cnt);
63.  
64.         do {
65.           *p_destination++ = inline_copy;
66.         } while (d2-- != 0);
67.         //} while (repeat_cnt-- != 0);
68.         break;
69.       }
70.       case 5: {
71.         unsigned short inline_copy = mapdevr_getinline(&wk, offset, d4, a5);
72.         //unsigned short inline_copy = mapdevr_getinline(&wk, offset, bitfield, inline_copy_bit_cnt);
73.  
74.         do {
75.           *p_destination++ = inline_copy++;
76.         } while (d2-- != 0);
77.         //} while (repeat_cnt-- != 0);
78.         break;
79.       }
80.       case 6: {
81.         unsigned short inline_copy = mapdevr_getinline(&wk, offset, d4, a5);
82.         //unsigned short inline_copy = mapdevr_getinline(&wk, offset, bitfield, inline_copy_bit_cnt);
83.  
84.         do {
85.           *p_destination++ = inline_copy--;
86.         } while (d2-- != 0);
87.         //} while (repeat_cnt-- != 0);
88.         break;
89.       }
90.       case 7:
91.         if (d2 == 0xF) {
92.         //if (repeat_cnt == 0xF) {
93.           done = true;
94.         }
95.         else {
96.           unsigned short inline_copy = mapdevr_getinline(&wk, offset, d4, a5);
97.           //unsigned short inline_copy = mapdevr_getinline(&wk, offset, bitfield, inline_copy_bit_cnt);
98.  
99.           do {
100.             *p_destination++ = inline_copy;
101.           } while (d2-- != 0);
102.           //} while (repeat_cnt-- != 0);
103.         }
104.         break;
105.     }
106.   } while (!done);
107. }

I'm going to cry in a corner now.

 

I noticed the same mistake yesterday in bitdevr. At least I'm consistent?

C's precedence rules strike again.

Unfortunately, while adding parentheses makes sense to us, the compiler complains that now I'm trying to increment something that's not an "lvalue". I've moved the incrementing to the next line.

Good catch. I admittedly wouldn't have looked.

That's not just a quick fix; it's the correct fix, because the x86 ASM does the same thing.

I've been doing lots of editing myself the past few days to make sure I don't get scolded by Overlord.

As bit rotations are frequently used in the pixelbuffer code, I've since made a macro for that. It doesn't have the issue you describe, so I'll replace that code with it. Here it is, for reference:

Code (C):

1. #define ROTATE_LEFT(value, bits) ((value = value << bits | value >> 32 - bits) & 0xFFFFFFFF)

Ah, I guess I missed it because the other calls weren't inside the loop.

Thank you so much for all of these corrections! :D The outputs now seem to match, save for the fact that SK&C's version is byte-swapped. Behold:



Some things to note:

• bitdevr isn't fixed yet, so I turned KENS's valid output into an inline byte array. Couldn't do the same trick to circumvent mapdevr, as it takes an offset that stand-alone tools don't account for.
• Yes, the colours look weird. The original program actually exhibits the same behaviour in full screen at the level select and in select zones. I don't know why. I tried switching the RGB values around when the palette is being copied (like redirecting the green channel to the blue channel, and vice-versa), but that had no effect.

This afternoon I looked into why the graphics don't display in window mode, without much luck. DirectDraw really doesn't like to be called while you're stepping through code with a debugger, making me think that calls were failing. The only call that actually fails is SetPalette because there's no 8-bit palette support. It then falls back to GDI. I continued following the code, but nothing looks wrong to me. But I have little experience with graphics programming, so I must be missing something.

 

You're right, I was too quick to jump for my macro. I reverted my fix and implemented your suggestion instead.

Fixed.

I don't like that solution, as that's not what the original ASM does. I modified BYTESWAP so it acts like function instead of a statement so I get to decide where to store the result:

Code (C):

1. *p_wk->p_destination++ = BYTESWAP(p_wk->prev_pixels);

Done.

I've verified the outputs, and they match! Thank you so much!

In the meantime, I've fixed some input bugs and some level select bugs. All that's left is the background animating too fast. The speed at which the current selection updates is the same as the original, so thankfully it's not an issue with the timing code. It's probably my port of the animation script code.

Speaking of animation scripts, they have a format that, when translated to a C type, can't be part of an array:

Code (C):

1. typedef struct animscript {
2.   unsigned char dummy1;
3.   unsigned char dummy2;
4.   unsigned char dummy3;
5.   signed char tim;
6.   unsigned char* p_rom;
7.   unsigned short vram_adr;
8.   unsigned char cnt;
9.   unsigned char size;
10.   signed char data[];
11. }
12. animscript;

The dummy variables are exclusive to S&KC. I suspect they're padding to place p_rom on a 4-byte boundary.

Anyway, right at the end there's a variable amount of data, dependent on the number of animation frames and whether there's frame-specific durations or not. This can be expressed in C99 as a flexible array that's at the end of a struct, which in this case also fits the original data. You cannot, however, make an array of this struct type because the size of an array's members needs to be known at compile time.

The level select has only one animation script, so it's not a problem there. But when there are several, I need a way to loop over them. I can't just place the structs next to each other in code because there's no guarantee that they'll also be next to each other in memory. I can't keep them as byte data, either, because there's a pointer to tile data in there. I suspect the original program hard-coded addresses to assets.

 

From what you tell me, this is your start-up context:

• g_startGameMode is set to 2 because debug mode is set in the INI file
• g_sonicGameMode is set to 1 because you chose Sonic & Knuckles in the launcher

After initialisation, the game decides on the start-up routine based on g_startGameMode. There are three of them:

• Zero launches one of the three games based on g_sonicGameMode
• 1 launches Special Stage Mode (aka Blue Spheres)
• 2 launches debug mode

The start-up code for debug mode doesn't care about the game you've selected, and sets g_mdRam.cartridge to zero, which is the value for Sonic 3 & Knuckles.

Now, when you soft reset the game, g_startGameMode is set to zero, restoring normal operation (read: no more debug mode). The normal start-up code does care about the value of g_sonicGameMode, which is still set to 1, so Sonic & Knuckles is started.

Hope that clears it up!

In other news, I've fixed the animation timing! I had to deviate a little bit from the original ASM because copying it literally would've involved casting an unsigned char to a signed char, and that's not portable.

Now I just need to fix window mode.

 

Today I solved as many compiler warnings as possible. When that didn't lead anywhere, I checked return values. Those are all fine.

Then I got the idea of overriding the palette values by assinging 128 to all channels. The display portion of the window became gray, so at least I knew there was an active surface there and that it was tied to a palette. Next I tried viewing memory, but that didn't lead anywhere.

Hey, why not try hard-coding the palette to a diverse set of colours? That would allow me to check if any pixels are actually written to it instead of it being blank.



Finally, I'm getting somewhere. Now I have to find out why the palette doesn't make its way to the surface.

 

Of course I can do that! That's a good idea.

As for my palette hunt...

If I zero the palette data when the program starts (which seems to have been the plan at one point, as I found an unreferenced function doing exactly that), the palette data is never updated because it never changes. Yet full-screen mode still seems to have colour data. I'm starting to think that it having colour is a glitch and that the palette it's using is uninitialised data.

@MainMemory do you have any idea where the palette data is loaded into the virtual Mega Drive's RAM? The function at 0x00426c05 loads the palette data for both level selects from colortbl (006704a6), and stores it at 0x9000000 + 0x80 (using the pointer at 008549d4). That thing has more than 1000 references. I went through a lot of them, but have yet to find where is retrieved.