Prince of Persia

I am trying to see if we can remove the dependencies on SDL_image and SDL_mixer.
Then we could get rid of all of the DLLs, except SDL2.dll.

So far I have been able to replace SDL_image with stb_image.h (and for saving screenshots, stb_image_write.h).
These also make things simpler, because they are single-file/header only libraries (basically, you just #include them and use them).

I created a new branch to try this out:
https://github.com/Falcury/SDLPoP/tree/stbi

But why?

Norbert wrote:But why?

I don't really have a good reason...
Mostly, I just wanted to see if it was possible, and learn things in the process.
I know I should really be doing other stuff instead...

Anyway, I've attached a proof of concept version that doesn't require SDL_image or SDL_mixer.
The possible advantages that I can see, would be:
- Less setup needed for people who want to compile SDLPoP from sources.
- Less cluttered base folder, slightly smaller download size. (no DLLs except SDL2.dll)
- For audio playback, a bit more fine-grained/low-level control over the sound routines from a programming perspective (though this can also be a disadvantage of course). Plus, the USE_MIXER ifdefs can be removed from the codebase.

That said, maybe the upsides don't outweigh the possible downsides.

(The attachment also has a build script for Visual Studio in the src folder; that's related to this pull request)

Branch with experimental replacement of SDL_mixer by stb_vorbis.c:
https://github.com/Falcury/SDLPoP/tree/stbv

Falcury wrote:Anyway, I've attached a proof of concept version that doesn't require SDL_image or SDL_mixer.

I tried it.
Your precompiled exe requires at least Windows Vista, so I had to recompile it for myself.
But it works well apart from that.

It seems that the GUARD.DAT file is now required even on Windows, otherwise the game crashes on meeting a guard.
It seems stb_image automatically converts paletted PNG images to RGB.
[I think that happens in stbi__expand_png_palette().]
And, while SDL_SetPaletteColors() does accept a NULL palette, Falcury changed set_chtab_palette() so that it reads current_palette->ncolors, and *that* is what causes the crash.

For the same reason, the background behind story texts is all black.

David wrote:Your precompiled exe requires at least Windows Vista, so I had to recompile it for myself.
But it works well apart from that.

I have a hunch what the reason might be.
It's probably because I used the linker flag /SUBSYSTEM:WINDOWS instead of /SUBSYSTEM:WINDOWS,5.01
This page has a description of the problem (if this is indeed the problem):
http://www.tripleboot.org/?p=423

I compiled it with MSVC, because for some reason that generates smaller executables compared to compiling with CLion (gcc/MinGW-w64).

David wrote:It seems that the GUARD.DAT file is now required even on Windows, otherwise the game crashes on meeting a guard.
It seems stb_image automatically converts paletted PNG images to RGB.
[I think that happens in stbi__expand_png_palette().]
And, while SDL_SetPaletteColors() does accept a NULL palette, Falcury changed set_chtab_palette() so that it reads current_palette->ncolors, and *that* is what causes the crash.

Then it seems I messed up there, not realizing that the palette could be NULL.
Hopefully this is easy to fix.

Some other possible issues that got introduced:
- There may be a memory leak when surfaces are created with SDL_CreateRGBSurfaceFrom(), because according to the documentation, SDL_FreeSurface() does not free the pixel data from which the surface was created.
- Turning the sound off now also turns off the music, which wasn't the case with SDL_mixer.
- If the music is stopped abruptly, there is an audible 'clicking' sound. I don't hear this on the SDL_mixer version. (From what I found on Google, this is apparently a problem that sound apps work around by doing a 'very fast fadeout' instead of an abrupt cutoff.)
- PNG files created with stb_image_write.h may have a worse compression ratio compared to libpng. See for example this blog post:
https://danielgibson.github.io/2015/07/ ... nd-libpng/

Falcury wrote: I have a hunch what the reason might be.
It's probably because I used the linker flag /SUBSYSTEM:WINDOWS instead of /SUBSYSTEM:WINDOWS,5.01
This page has a description of the problem (if this is indeed the problem):
http://www.tripleboot.org/?p=423

Well, the subsystem version in the PE header of your EXE looks exactly like on their screenshot.
However, hex-editing your prince.exe was not enough, because it also uses VCRUNTIME140.dll and some api-ms-win-crt-*.dll files that I don't have.

Falcury wrote: - There may be a memory leak when surfaces are created with SDL_CreateRGBSurfaceFrom(), because according to the documentation, SDL_FreeSurface() does not free the pixel data from which the surface was created.

After reading this, I thought that my original code has the same problem when loading images from DAT files.
But I use SDL_CreateRGBSurface() there: https://github.com/NagyD/SDLPoP/blob/ma ... 009.c#L518

A thing to be careful of: you should manually free the pixel data *only* if it was allocated separately.
But sometimes you can't tell in advance whether you need to do that at a certain place.
For example, in free_chtab(), SDL_FreeSurface() might be called on images loaded from either PNGs or DATs, but the latter are *not* created using SDL_CreateRGBSurfaceFrom().

How to tell the difference?

There is a flag in each surface that tells SDL_FreeSurface() whether to free the pixel data.
https://hg.libsdl.org/SDL/file/b5cf1e85 ... ce.c#l1177
So when freeing a surface you should check that flag, and if it's set then free the pixel data manually.

Or maybe just clear the flag after creating the surface with SDL_CreateRGBSurfaceFrom().
Although I'm not sure if SDL_free() (in SDL_FreeSurface()) is compatible with regular malloc() (used in STB).
However, STB has an option to define what functions it should use for allocating and freeing memory.

Falcury wrote: - Turning the sound off now also turns off the music, which wasn't the case with SDL_mixer.

Ctrl+S *should* turn off the music as well.
So the STB version is correct, and there is a bug in the original SDLPoP.
In the original SDLPoP, the intro music or the level 1 starting music will play even if sound is off. But the death music won't play.

And the original PoP for DOS has the opposite bug: Ctrl+S turns off only the music.
See here: viewtopic.php?p=14208#p14208

David wrote: Ctrl+S *should* turn off the music as well.
So the STB version is correct, and there is a bug in the original SDLPoP.

Fixed: https://github.com/NagyD/SDLPoP/commit/ ... 97bd9ece34

David wrote: In the original SDLPoP, the intro music or the level 1 starting music will play even if sound is off. But the death music won't play.

That's because the death music is explicitly disabled if the sound is off: https://github.com/NagyD/SDLPoP/blob/ma ... 06.c#L1193