SDLPoP; David's open-source port of PoP

[David]

I had to change the argument order (b,g,r instead of r,g,b) in two calls to SDL_MapRGB()

This is fixed if I change the buffer to 32 bits.
For the palace walls: in make_offscreen_buffer()
For flashing: in set_gr_mode()
I don't know why this happens.

- The onscreen and offscreen surfaces are now 24 bpp again instead of 32. Unfortunately, I cannot get this surface to the screen correctly, so for now it is still converted to 32 bpp every time there is a screen update (which is at least less horrible than the way it used to be).

It appears correctly if I specify SDL_PIXELFORMAT_RGB24 in SDL_CreateTexture.
But then the previous problem comes back...
(SDL_PIXELFORMAT_RGB888 would be 32-bit, but different from RGBX8888. Maybe it should be called XRGB8888?)

[David]

One strange side effect is that I had to change the argument order (b,g,r instead of r,g,b) in two calls to SDL_MapRGB().

It seems to be a bug in SDL.
I made a testcase (attached) that fills a surface with orange (255,192,128), using SDL_FillRect().
The surface is saved to a BMP file, it is not shown.

If the target surface is 24 bits-per-pixel, then the orange becomes blue (128,192,255).
If it's 16 or 32, then this does not happen.
If I recompile the testcase with SDL 1.2, then this does not happen.

The testcase also copies an image (of the kid) onto the background color.
This is to show that the problem is not with loading/saving of images, but with SDL_FillRect().
It also puts a red pixel into top-left corner manually using surface->pixels and surface->format.

[Norbert]

This is the output I get under Linux.
First SDL2, then SDL1.

$ gcc testcase.c -o testcase `sdl2-config --cflags --libs` -I/usr/include/SDL2

$ ./testcase
BitsPerPixel=24
BytesPerPixel=3
Rmask=0x00FF0000
Gmask=0x0000FF00
Bmask=0x000000FF
Amask=0x00000000
color=0x00FFC080

- - - - -

$ gcc testcase.c -o testcase `sdl-config --cflags --libs` -I/usr/include/SDL

./testcase
BitsPerPixel=0
BytesPerPixel=0
Rmask=0x00FF0000
Gmask=0x0000FF00
Bmask=0x000000FF
Amask=0x00000000
color=0x00FFC080

[David]

I get the same output under Windows.
But the important part is: What does the program put into testcase_out.bmp in the two cases?

This bug is not yet reported:
https://bugzilla.libsdl.org/buglist.cgi ... hangeddate
So I will have to report it.
Should I keep the image of the kid and/or the red pixel?

EDIT:
Here is the 24-bit case of SDL_FillRect: https://hg.libsdl.org/SDL/file/b8d8f967 ... ect.c#l196
It looks like somebody else already noticed this: http://comments.gmane.org/gmane.comp.lib.sdl/53072
"r" and "b" should be swapped, but I'm afraid it depends on endianness of the CPU.

[David]

I reported the bug in the SDL Bugzilla: https://bugzilla.libsdl.org/show_bug.cgi?id=2986
I attached a smaller testcase (no loaded image or red pixel), and my proposed fix.

[Norbert]

It's nice that you reported this and even included a proposed fix.
Hopefully they will see the value of your work, and do something with it soon.

I hope you don't mind I mailed this:

Spoiler: show

To: icculus TA icculus.org, slouken TA libsdl.org
Subject: SDL
Date: Sun, 17 May 2015 23:42:37 +0200

Hi Ryan, Sam,

Maybe it is time for a new SDL release?
Version 2.0.3 was released over a year ago.

You/we need new releases to bring bug fixes to the people.

For example, in June 2014 this was fixed...
https://bugzilla.libsdl.org/show_bug.cgi?id=2389
...but most developers (and thus end users) are still working with
2.0.3.

The person who created the above bug report and fix recently created
another bug report and fix...
https://bugzilla.libsdl.org/show_bug.cgi?id=2986
...but if you don't release new versions of SDL, what's the point?

Best regards,
Norbert

[Falcury]

It appears correctly if I specify SDL_PIXELFORMAT_RGB24 in SDL_CreateTexture.
But then the previous problem comes back...
(SDL_PIXELFORMAT_RGB888 would be 32-bit, but different from RGBX8888. Maybe it should be called XRGB8888?)

It seems to be a bug in SDL.

Great that you found the problem! Until SDL gets updated, I suppose we can safely leave the SDL_FillRect workaround in place? With 24-bit surfaces (magically?) working with SDL_PIXELFORMAT_RGB24, I guess the cleanest solution is to simply use that as the buffer format...

I have found a couple of new issues:
- The blinking sound (sound 38) does not play when the game prompts to respawn after dying
- CTRL+R is not instantaneous, because the opening music has to be loaded from disk again. Can we stop the program from freeing sounds after they have been loaded once? (we have enough RAM anyway)
- Guard palettes do not seem to get set correctly on Mac OS X, causing a crash (I think the guard images are not loaded as 8-bit for some reason - I haven't thoroughly researched this)
- The game still stutters under some conditions on my system - I think this might be a timer-related problem, I'm trying to pin it down...

I would also like to propose a new optional feature. I've been thinking about how a quicksave system might be implemented in SDLPoP - this might be very useful for difficult mods, because endlessly redoing the entire level loses its charm quickly and encourages people (including me) to cheat.
- F6 to quicksave, F9 to quickload
- Quicksaving displays the message "QUICKSAVE" at the bottom of the screen
- Time keeps running out when you quickload, similar to dying or pressing CTRL+A.

[David]

- The blinking sound (sound 38) does not play when the game prompts to respawn after dying

That sound exists only in pcspeaker format. SDLPoP does not play those.

- CTRL+R is not instantaneous, because the opening music has to be loaded from disk again. Can we stop the program from freeing sounds after they have been loaded once? (we have enough RAM anyway)

Good idea.
But to make my own changes, I first need to merge yours...

- Guard palettes do not seem to get set correctly on Mac OS X, causing a crash (I think the guard images are not loaded as 8-bit for some reason - I haven't thoroughly researched this)

StaticReturn has already ran into this: viewtopic.php?p=15661#p15661 (and following posts)
And your guess is right: viewtopic.php?p=15676#p15676
A solution/workaround is to use GUARD.DAT instead of the PNGs.

I would also like to propose a new optional feature. I've been thinking about how a quicksave system might be implemented in SDLPoP - this might be very useful for difficult mods, because endlessly redoing the entire level loses its charm quickly and encourages people (including me) to cheat.
- F6 to quicksave, F9 to quickload
- Quicksaving displays the message "QUICKSAVE" at the bottom of the screen
- Time keeps running out when you quickload, similar to dying or pressing CTRL+A.

So this would be something like a savestate in an emulator, right?
Except that time-related variables would not be saved.
(F6 and F9 as in PoP3D?)
We need to look through the global variables in data.h and decide what must be saved, and what not.

[David]

I merged Falcury's changes on GitHub.

[Falcury]

That sound exists only in pcspeaker format. SDLPoP does not play those.

That's a pity. Perhaps we could somehow convert the sound to a waveform version, so that emulation of the pc speaker won't be necessary.

So this would be something like a savestate in an emulator, right?
Except that time-related variables would not be saved.
(F6 and F9 as in PoP3D?)
We need to look through the global variables in data.h and decide what must be saved, and what not.

Yes, PoP3D was the inspiration :-) It may be a bit of a challenge to make it work properly; the state of the entire level will need to be saved, including the exact positions of the guards, gates, debris et cetera. I think it will be fun to implement.

I merged Falcury's changes on GitHub.

How did you manage to do it? I will probably get in trouble once I try to update my local copy (which has uncommitted changes). I suppose the easiest option will be to back up my changes and manually merge them afterwards...

[David]

How did you manage to do it?

After the success, I saved the whole contents of the console window.
The commands are: (only those that do something)

Code: Select all

git checkout -b Falcury-master master
git pull https://github.com/Falcury/SDLPoP.git master

git checkout master
git merge --no-ff Falcury-master
<fix seg009.c by hand>
git add *
git commit
git checkout master
git merge Falcury-master
git push

(Actually the first two commands are from the GitHub instructions. They scrolled out of the console buffer by the end.)

the state of the entire level will need to be saved, including the exact positions of the guards, gates, debris et cetera

I already made a list of all global variables, and noted yes/no next to each.

[segra]

That sound exists only in pcspeaker format. SDLPoP does not play those.

That's a pity. Perhaps we could somehow convert the sound to a waveform version, so that emulation of the pc speaker won't be necessary.

Perhaps this can be of some help, its the Dos PC Speaker handling for ScummVM (decoding speaker sound from old DOS LucasArts/Film adventures). It should output to an SDL compatible format.
https://github.com/scummvm/scummvm/blob ... yer_v1.cpp


P.S. I've just submitted a GitHub Pull request to add joystick support, as the main conversation about this project is occurring here, figured i should sign up

Keep up the good work, its much appreciated

[David]

Today I removed lots of unused stuff.
Most of these were leftovers from the disassembly.
One of them is that "method_7_alloc(-1);" that segra also noticed.

I also merged the newer pull request by kees.

I will also merge the pull request with the joystick support.
I don't have a joystick so I can't test it.

[segra]

David, just wondering... whats your intentions for SDLPoP ?

- Clean it up completely / migrate functions out of the generic seg* files ?
- POP2 Support?
- Apple / Amiga data support?

[David]

- Clean it up completely / migrate functions out of the generic seg* files ?

Maybe, but it's not urgent.

- POP2 Support?

PoP2 is very different from PoP1, so that would be a separate project.
The current status of PoP2 disassembly is here: viewtopic.php?p=14786#p14786

- Apple / Amiga data support?

Amiga graphics is identical to the DOS, so why?
Apple: It makes sense, but is it worth?


I merged your changes and also fixed the fullscreen bug that you noted.
Ctrl-J steps one room right in cheat mode. Modifier keys should be handled more consistently in process_key(). Or maybe in idle()?
(Maybe last_key_scancode should contain the currently pressed modifiers somehow.)