Skip to content

Add SDL3 as a platform#116

Merged
icculus merged 2 commits into
icculus:mainfrom
RobLoach:sdl3
May 28, 2026
Merged

Add SDL3 as a platform#116
icculus merged 2 commits into
icculus:mainfrom
RobLoach:sdl3

Conversation

@RobLoach
Copy link
Copy Markdown
Contributor

@RobLoach RobLoach commented May 28, 2026

This adds SDL3 as a platform to allow using the SDL3 File System when interacting with PhysFS. I'm not 100% sure this is a good idea, so let me know if this is a backwards approach.

Since the platform changes across SDL builds, I've exposed a PHYSFS_PLATFORM_SDL3 CMake option to allow explicitly stating that you want to target SDL3 instead of one of the other platforms.

  • I confirm that I am the author of this code and release it to the PhysicsFS project under the Zlib license. This contribution does not contain code from other sources, including code generated by a Large Language Model ("AI").

@icculus
Copy link
Copy Markdown
Owner

icculus commented May 28, 2026

I've actually been thinking of doing this for awhile, honestly.

I'll look at this more closely in the morning (and probably merge it).

@RobLoach
Copy link
Copy Markdown
Contributor Author

Sounds good with me. I ran it through my own test suite, and things seem to be working, but feel free to change anything around. Some thoughts...

  1. Enumeration system could likely improve with further safety checks, but seemed okay to me
  2. I wasn't 100% on if SDL_GetPrefPath() issues a SDL_SetError(), so that's the only SDL call that doesn't do an SDL_BAIL_IF()

Overall though, it seemed to be pretty solid.

Comment thread src/physfs_platform_sdl3.c Outdated
*
* When finished, the result will need SDL_free().
*/
static char *platformCopyWithSeparator(const char *path)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDL_GetUserFolder, SDL_GetBasePath, and SDL_GetPrefPath all promise the returned string will end with a path separator, so this function probably can go away in favor of a call to __PHYSFS_strdup().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only difference I found between them was that on SDL_GetPrefPath(), we don't need to call __PHYSFS_strdup() since it allocs the memory for us, while the others don't. Should be good now. Thanks!

Comment thread src/physfs_platform_sdl3.c Outdated
/**
* Ignores any issued SDL errors prior to sending the report off to PhysFS.
*
* This ensures we don't pollute SDL_GetError().
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to pollute SDL_GetError(). If one didn't query the string immediately upon return from a failing SDL API call, it's not reasonable to assume it didn't get replaced by something else (including a PhysicsFS that was explicitly built to use SDL under the hood).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed SDL_BAIL_IF entirely. Does clean it up.

@icculus icculus merged commit c45bb5d into icculus:main May 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants