libsdl-org / SDL-1.2

Simple Directmedia Layer, 1.2 branch ... ***DEPRECATED***, please use https://github.com/libsdl-org/SDL for new projects!
https://libsdl.org
GNU Lesser General Public License v2.1
98 stars 81 forks source link

Include SDL_iconv.obj in OS/2 builds #867

Closed ccawley2011 closed 1 year ago

ccawley2011 commented 1 year ago

This has only been compile tested.

sezero commented 1 year ago

Don't know about this one: It was never included in the build, maybe for a reason but I don't know, and I don't have the motivation to test.

So let's not do this one for now.

ccawley2011 commented 1 year ago

Don't know about this one: It was never included in the build, maybe for a reason but I don't know, and I don't have the motivation to test.

README.Watcom claims that testiconv doesn't work in Windows OpenWatcom builds, but I just tried that and it works fine, so I'd assume that whatever was broken back in 2006 is fixed now.

sezero commented 1 year ago

Windows: OK. But OS/2 is weird. Look at core/os2/geniconv in SDL2. It may require more to do for OS/2 than plain SDL_iconv.c (but just guessing and without testing.)

sezero commented 1 year ago

If you can test and fix iconv stuff for os/2 though, that'd be really welcome :)

ccawley2011 commented 1 year ago

I'm assuming that the OS/2 iconv code in SDL2 is there to support additional codepages that would normally require a full implementation of iconv, while the built-in fallback in SDL only supports ASCII, ISO-8859-1 and Unicode. I'd imagine that this will be sufficient to ensure that the iconv functions are exported in the DLL (like they are in sdl12-compat), and that any SDL 1.2 programs that need additional code pages will need to be adapted for Windows as well, since I think the official Windows builds of SDL 1.2 (and SDL2 as well) also use the built-in conversion routines.

sezero commented 1 year ago

That sounds correct.

ccawley2011 commented 1 year ago

I (eventually) figured out how to get this running under eComStation, and testiconv seems to work fine.

sezero commented 1 year ago

No changes needed, yes? (Reopened the PR.)

ccawley2011 commented 1 year ago

No changes needed, yes? (Reopened the PR.)

Just the ones in this PR.

sezero commented 1 year ago

OK then, applied. Thanks!