- Notifications
You must be signed in to change notification settings - Fork3.8k
SDL_UCS4ToUTF8 may exist from SDL_keyboard.c#3349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
test for remove function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Another alternative is to just use thepg_ prefix for this, and not rely on SDL at all on WASM (since this is an internal SDL util function, might not be wise to do)
i think using the internal may gain some space ( .wasm are not compressed unlike .data) it's worth the risk and easy to fix later since function code will stay just above. |
What about using |
That would not work because |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM! Thanks 😎
This has no affect on our non-static builds, but if this makes things better for WASM lets merge this in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM 👍
| #else | ||
| /* Taken from SDL_iconv() */ | ||
| char* | ||
| SDL_UCS4ToUTF8(Uint32ch,char*dst) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ps. We should never define our own "SDL_" functions. This has bitten us in the past. Instead we should create pg_XXX functions which inside may call the SDL_XXX function, or include the function body. This way if someone links something with the SDL_XXX function in the future we have no problems.
Created an issue for this here:#3417
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
test CI for removing function