Skip to content

Conversation

@corentin35000
Copy link
Contributor

@corentin35000 corentin35000 commented Sep 12, 2025

On Android, assets are always packaged inside the assets/ folder of the APK.
There is no need to force a leading / in the path — on the contrary, having "/images/foo.png" makes AAssetManager fail, because it expects relative paths like "images/foo.png".

Previously:

Calling SDL_OpenTitleStorage("") built a base path of "/", producing lookups like "/images/..." that fail on Android.

Calling SDL_OpenTitleStorage(NULL, …) relied on SDL_GetBasePath(), but on Android this always returns NULL, so the call failed with “No available title storage driver”.

This patch fixes both issues by:

Treating override == "" as an empty base path (no leading /).

Falling back to an empty base path when SDL_GetBasePath() returns NULL on Android.

This allows both SDL_OpenTitleStorage("") and SDL_OpenTitleStorage(NULL, …) to work correctly on Android without any extra changes.
On other platforms, behavior remains unchanged.

Tested on macOS, Linux, Windows, and Android — everything works as expected.

Related issues: #13929, #13933

@corentin35000
Copy link
Contributor Author

let me know if this is okay with you @slouken :)

@slouken slouken added this to the 3.4.0 milestone Sep 12, 2025
@slouken slouken requested a review from icculus September 12, 2025 19:45
@icculus
Copy link
Collaborator

icculus commented Sep 15, 2025

I'm wondering if it's better to have Android not return NULL from SDL_GetBasePath()...if "assets" is guaranteed to be a real path and we can access files in it with fopen(), I'd rather do that...but if it only works with SDL_IOStream (which does a lot of tapdancing for assets on Android), SDL_GetBasePath()!=NULL is only going to cause different problems.

@icculus
Copy link
Collaborator

icculus commented Nov 11, 2025

I dunno, I'd rather this have a cleaner solution than this, but that's likely not worth the time investment and this patch seems harmless enough.

Going to merge this, I think. @flibitijibibo, any opinions on this before I click The Big Green Button?

@flibitijibibo
Copy link
Collaborator

Nothing from me!

@icculus icculus merged commit 2079517 into libsdl-org:main Nov 12, 2025
79 of 84 checks passed
@smcv
Copy link
Contributor

smcv commented Nov 13, 2025

I'm wondering if it's better to have Android not return NULL from SDL_GetBasePath()

That's #13933

@smcv
Copy link
Contributor

smcv commented Nov 13, 2025

Did this change fix #13929?

@icculus
Copy link
Collaborator

icculus commented Nov 14, 2025

I'm wondering if it's better to have Android not return NULL from SDL_GetBasePath()...if "assets" is guaranteed to be a real path

(It's not a real path, so if you try to fopen() something in "/assets" it'll fail (but SDL_IOStream, which can use the AssetManager interface behind the scenes, will not.)

So that's a non-starter. For now, leaving it as NULL is probably the right thing (or at least, changing it now is probably not the least harm). I can be convinced otherwise, though!

Did this change fix #13929?

I would also like to know this.

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.

5 participants