Old env vars as fallback for SDL_VIDEO_DRIVER + SDL_AUDIO_DRIVER

[DanielGibson]

Especially SDL_VIDEODRIVER is commonly used to use the native Wayland backend, so I think it's a good idea to keep supporting the old name instead of forcing users to find out that they now have to add an underscore..
Not sure how popular SDL_AUDIODRIVER is, but with all the audio backends that exist on Linux alone I'm sure some people use it to work around sound issues.

Description

I'm not 100% sure what the best way to do this is, but the easiest way to do it cleanly seemed to be to implement a small wrapper around SDL_getenv() in SDL_hints.c that, if SDL_getenv() returned NULL for the given name (i.e. the "proper" SDL3 hint string), checks whether it was SDL_HINT_VIDEO_DRIVER or SDL_HINT_AUDIO_DRIVER and tries again with "SDL_VIDEODRIVER" and "SDL_AUDIODRIVER".

This is not overly generic and might in theory get a bit ugly if someone wanted to add fallbacks for many more environment variables, but:

• Doing it here makes sure that all the hint priority logic works as it should.
  For example, if the program calls SDL_SetHintWithPriority(SDL_HINT_VIDEO_DRIVER, "x11", SDL_HINT_OVERRIDE), that will override the environment variable, no matter how it's called - this would be a lot harder to do when implementing the fallback at the call-sites of SDL_GetHint(SDL_HINT_VIDEO_DRIVER) - but on the other hand, if the program uses normal SDL_SetHint() (or a priority < SDL_HINT_OVERRIDE), the fallback environment variable will override that just like the "proper" environment variable would
• Doing it here makes sure that SDL_ResetHint() takes the fallback env var into account
• It's simple
• I don't think that too many similar cases where one would want a fallback will turn up, so I guess it's good enough - and if not it can still be refactored

Existing Issue(s)

Fixes #11115

---

Marking this as a draft until I've tested it a bit more

[DanielGibson]

Ok, this seems to work and is ready for review :)

[icculus]

I think I just want to special case audio and video variables and not make it generic.

diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c
index b232bef59..a987276bf 100644
--- a/src/audio/SDL_audio.c
+++ b/src/audio/SDL_audio.c
@@ -905,6 +905,9 @@ bool SDL_InitAudio(const char *driver_name)
     // Select the proper audio driver
     if (!driver_name) {
         driver_name = SDL_GetHint(SDL_HINT_AUDIO_DRIVER);
+        if (!driver_name) {
+            driver_name = SDL_getenv("SDL_AUDIODRIVER");  // check legacy env variable to help end users.
+        }
     }
 
     bool initialized = false;
diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c
index a0a147cae..7d71cabed 100644
--- a/src/video/SDL_video.c
+++ b/src/video/SDL_video.c
@@ -596,6 +596,9 @@ bool SDL_VideoInit(const char *driver_name)
     video = NULL;
     if (!driver_name) {
         driver_name = SDL_GetHint(SDL_HINT_VIDEO_DRIVER);
+        if (!driver_name) {
+            driver_name = SDL_getenv("SDL_VIDEODRIVER");  // check legacy env variable to help end users.
+        }
     }
     if (driver_name && *driver_name != 0) {
         const char *driver_attempt = driver_name;
@@ -5447,6 +5450,10 @@ bool SDL_ShowMessageBox(const SDL_MessageBoxData *messageboxdata, int *buttonID)
     } else {
         // It's completely fine to call this function before video is initialized
         const char *driver_name = SDL_GetHint(SDL_HINT_VIDEO_DRIVER);
+        if (!driver_name) {
+            driver_name = SDL_getenv("SDL_VIDEODRIVER");  // check legacy env variable to help end users.
+        }
+
         if (driver_name && *driver_name != 0) {
             const char *driver_attempt = driver_name;
             while (driver_attempt && (*driver_attempt != 0) && !result) {

(although that can be an SDL_GetHint instead of SDL_getenv if we want to be forgiving to existing programs and not just users.)

[DanielGibson]

SDL_HINT_VIDEO_DRIVER is also checked in src/video/dummy/SDL_nullvideo.c and src/video/offscreen/SDL_offscreenvideo.c

Does SDL_GetHint() return NULL or a default value if the hint (or corresponding env var) hasn't been set by the user?

[DanielGibson]

@icculus Also, doesn't that change the behavior?

For example, imagine a game setting SDL_SetHint(SDL_HINT_VIDEO_DRIVER, "wayland").
Now if a user uses export SDL_VIDEO_DRIVER=x11, that will override the hint, because the application set the hint with normal priority.

If the user uses export SDL_VIDEODRIVER=x11 instead, then with my code it would work exactly the same, i.e. the environment variable would override the hint - but with your suggested changes, the fallback environment variable is never checked, because SDL_GetHint(SDL_HINT_VIDEODRIVER) didn't return NULL, but "wayland".

[slouken]

Touché! @icculus, your move. :)

[DanielGibson]

What slightly annoys me: I'm pretty sure I had already foreseen this exact problem with just using getenv() at the places the hints are used when I wrote #11115:

or somehow handle this in the callers of SDL_GetHint(SDL_VIDEO_DRIVER)?
The latter is probably harder to do correctly if I want to respect hint priorities..

By the way, the (public) Hint API is missing a getter for a hints priority.

But two days later when icculus suggested this I'd already forgotten about that and only after implementing the changes he suggested, while writing the commit message, I rediscovered it (-:

[DanielGibson]

Ok, I did all the requested changes.

I just realized you have a clang-format script in build-scripts/ - should I use that before committing in the future?

[slouken]

Ok, I did all the requested changes.

I just realized you have a clang-format script in build-scripts/ - should I use that before committing in the future?

Sure, just ignore any changes in third party code or obvious bad formatting. It's not perfect. :)

[slouken]

I'm going to go ahead and merge this. @icculus and I can arm wrestle over it later. :)

Thanks!

[icculus]

Eh, it's fine with me.

[DanielGibson]

Thanks for reviewing and merging!