Since there are a lot of repeated ifdefs relates to functions that have simply been renamed or only have minor changes to the parameters, I'd recommend using wrapper functions where possible to make the changes easier to read and less fragile to maintain.

Since there are a lot of repeated ifdefs relates to functions that have simply been renamed or only have minor changes to the parameters, I'd recommend using wrapper functions where possible to make the changes easier to read and less fragile to maintain.

Thanks for your advice.
Do you prefer #define, static functions or member functions?

Here is an example with static functions:

Before:

#if SDL_VERSION_ATLEAST(3, 0, 0)
    if (!SDL_AddGamepadMappingsFromFile(file.getPath().toString(Common::Path::kNativeSeparator).c_str()))
#else
    if (SDL_GameControllerAddMappingsFromFile(file.getPath().toString(Common::Path::kNativeSeparator).c_str()) < 0)
#endif
        error("File %s not valid: %s", file.getPath().toString(Common::Path::kNativeSeparator).c_str(), SDL_GetError());

After with static functions:

static bool gameControllerAddMappingsFromFile(const char *file) {
#if SDL_VERSION_ATLEAST(3, 0, 0)
	return SDL_AddGamepadMappingsFromFile(file);
#else
	return SDL_GameControllerAddMappingsFromFile(file) >= 0;
#endif
}

...

if (!gameControllerAddMappingsFromFile(file.getPath().toString(Common::Path::kNativeSeparator).c_str()))
        error("File %s not valid: %s", file.getPath().toString(Common::Path::kNativeSeparator).c_str(), SDL_GetError());
    

Regarding the imgui update, did you add the files from the matching git commit as rest of the code?

Regarding the imgui update, did you add the files from the matching git commit as rest of the code?

Yes I used the commit ocornut/imgui@cb16568 corresponding to the tag https://github.com/ocornut/imgui/releases/tag/v1.91.3

About this, I see that there is a backends/imgui/scummvm.patch file, I should update this file too, no?

Since there are a lot of repeated ifdefs relates to functions that have simply been renamed or only have minor changes to the parameters, I'd recommend using wrapper functions where possible to make the changes easier to read and less fragile to maintain.

Tell me if it's OK now or did you think something different?

About this, I see that there is a backends/imgui/scummvm.patch file, I should update this file too, no?
Yes. I use it for updating imgui from time to time. Just make a diff against the imgui git commit.

About this, I see that there is a backends/imgui/scummvm.patch file, I should update this file too, no?
Yes. I use it for updating imgui from time to time. Just make a diff against the imgui git commit.

I updated the patch file, but I have a lot more changes than the previous one, can you have a look @sev- please?

Since there are a lot of repeated ifdefs relates to functions that have simply been renamed or only have minor changes to the parameters, I'd recommend using wrapper functions where possible to make the changes easier to read and less fragile to maintain.

Tell me if it's OK now or did you think something different?

There is no definite answer here... use whatever makes the code easier to read and maintain.

From the three options you provided:

• static wrapper functions would be a very straightforward way
• defines are a bit uglier than function wrappers, IMHO
• member functions are also a nice idea, we could keep all of these functions neatly placed inside classes, and instantiate a different class per SDL version

So, from the options above, we could either use a C-style implementation (static wrapper functions) or a C++-style implementation (different class per SDL version, inherited from a common parent class, with member functions). I personally like the C++ approach better, but function wrappers would also work just fine :)

Since there are a lot of repeated ifdefs relates to functions that have simply been renamed or only have minor changes to the parameters, I'd recommend using wrapper functions where possible to make the changes easier to read and less fragile to maintain.

Tell me if it's OK now or did you think something different?

There is no definite answer here... use whatever makes the code easier to read and maintain.

From the three options you provided:

• static wrapper functions would be a very straightforward way
• defines are a bit uglier than function wrappers, IMHO
• member functions are also a nice idea, we could keep all of these functions neatly placed inside classes, and instantiate a different class per SDL version

So, from the options above, we could either use a C-style implementation (static wrapper functions) or a C++-style implementation (different class per SDL version, inherited from a common parent class, with member functions). I personally like the C++ approach better, but function wrappers would also work just fine :)

I made my choice already, sorry I'm not a huge fan of inheritance and I don't see the benefit of creating 1 class by version.

I made my choice already, sorry I'm not a huge fan of inheritance and I don't see the benefit of creating 1 class by version.

That's fine :) Nice implementation!

Thank you for looking into this!

I wanted to give it a try on Mingw64, and currently it seems that Win32 isn't fully supported yet:

In file included from backends/platform/sdl/win32/win32-window.cpp:27:
./backends/platform/sdl/win32/win32-window.h:32:9: error: 'HWND' does not name a type
   32 |         HWND getHwnd();
      |         ^~~~
backends/platform/sdl/win32/win32-main.cpp: In function 'int WinMain(HINSTANCE, HINSTANCE, LPSTR, int)':
backends/platform/sdl/win32/win32-main.cpp:54:16: error: 'main' was not declared in this scope
   54 |         return main(__argc, __argv);
      |                ^~~~
make: *** [Makefile.common:176: backends/platform/sdl/win32/win32-main.o] Error 1
make: *** Waiting for unfinished jobs....
    C++      engines/scumm/boxes.o
    C++      engines/scumm/camera.o
backends/platform/sdl/win32/win32-window.cpp: In member function 'virtual void SdlWindow_Win32::setupIcon()':
backends/platform/sdl/win32/win32-window.cpp:36:29: error: 'getHwnd' was not declared in this scope; did you mean 'getwc'?
   36 |                 HWND hwnd = getHwnd();
      |                             ^~~~~~~
      |                             getwc
backends/platform/sdl/win32/win32-window.cpp: At global scope:
backends/platform/sdl/win32/win32-window.cpp:53:6: error: no declaration matches 'HWND__* SdlWindow_Win32::getHwnd()'
   53 | HWND SdlWindow_Win32::getHwnd() {
      |      ^~~~~~~~~~~~~~~
backends/platform/sdl/win32/win32-window.cpp:53:6: note: no functions named 'HWND__* SdlWindow_Win32::getHwnd()'
./backends/platform/sdl/win32/win32-window.h:29:7: note: 'class SdlWindow_Win32' defined here
   29 | class SdlWindow_Win32 final : public SdlWindow {
      |       ^~~~~~~~~~~~~~~
make: *** [Makefile.common:176: backends/platform/sdl/win32/win32-window.o] Error 1

I quickly went through the release notes and indeed it looks like backends/platform/sdl/win32/win32-window.cpp is missing something - I'll check if I can adapt it from the other changes.

I quickly went through the release notes and indeed it looks like backends/platform/sdl/win32/win32-window.cpp is missing something - I'll check if I can adapt it from the other changes.

That's nice of you, thank you.

Colossal work, thank you!

Have number of comments, mainly on formatting and suggestion to add more wrappers.

I am concerned that the code now becomes much harder to maintain with this amount of ifdefs, so maybe splitting files/adding more wrappers could make it easier to read and less fragile?

Sorry I've been sick for a few days, as soon as I feel better I'll take care of it.