search

PascalGameDevelopment / SDL2-for-Pascal

Unit files for building Free Pascal and Delphi applications using the SDL2 library
https://pascalgamedevelopment.github.io/SDL2-for-Pascal/
Mozilla Public License 2.0
103 stars 20 forks source link

Add iconv functions #109

Closed suve closed 1 year ago

suve commented 1 year ago

This changeset adds SDL_iconv* functions and related types.

Note: Since many other symbols found in SDL_stdinc.h are missing from our sdlstdinc.inc, some changes have been made:

  1. FPC's built-in Strings.strlen() is called instead of SDL_strlen(), where approriate.
  2. The SDL_iconv_wchar_utf8() macro is not included in this changeset, as it requires SDL_wcslen().
Free-Pascal-meets-SDL-Website commented 1 year ago

Thanks!

Sorry for the delayed response.

The following points I like to hint to or ask about:

  1. I'd prefer lower-case const keyword (general code style convention).
  2. The identation for the Result statements in the three SDLiconv* macros is not equal.
  3. I'd like to ask why you go for the Strings unit implementation of strlen instead of adding/using SDL_StrLen? For me the latter solution seems more appropriate here.

Best regards Matthias

suve commented 1 year ago

I'd like to ask why you go for the Strings unit implementation of strlen instead of adding/using SDL_StrLen? For me the latter solution seems more appropriate here.

We already do something similar for SDL_FRectEqualsEpsilon(), where we call FPC's System.Abs() instead of SDL_fabsf(). (link)

I've asked this in #62 but didn't really receive an answer - do we want to go all-in and add definitions for all the cstdlib-like SDL functions? If so, then I'll follow up this PR with another one (or more) adding stdinc.h symbols and fixing any relevant macros.

Free-Pascal-meets-SDL-Website commented 1 year ago

We already do something similar for SDL_FRectEqualsEpsilon(), where we call FPC's System.Abs() instead of SDL_fabsf(). (link)

I've asked this in #62 but didn't really receive an answer - do we want to go all-in and add definitions for all the cstdlib-like SDL functions? If so, then I'll follow up this PR with another one (or more) adding stdinc.h symbols and fixing any relevant macros.

Thanks for the elaborated answer.

I see your point, the main reason why I stumbled about this, is the introduction of the Strings unit, which could be prevented when using SDL's own function. This would be desirable to have less dependency on certain units and higher compatibility between compilers. It seems Delphi has no Strings unit but a AnsiStrings unit for StrLen. Also, FPC and Delphi have strlen as part of their SysUtils unit (see FPC SysUtils.strlen and Delphi SysUtils.StrLen), which would be more compatible as we don't have to distinguish between the used units based on the compiler. - Unfortunately Delphi marked the SysUtils.StrLen as deprecated.

Please fix this PR to be Delphi-compatible.

I'm sorry, I forgot about your reply in #62 . Let's discuss this in general there. But to answer your question

do we want to go all-in and add definitions for all the cstdlib-like SDL functions? here, which is highly related to the fix-up,

I would suggest to go for addition of cherry-picked, useful SDL functions if it is worth it. To my mind it is worth it, if we can prevent adding new units (dependencies).

Best regards Matthias

suve commented 1 year ago

Thanks for the insight regarding Delphi compatibility. I've pushed one more commit that should include the proper unit based on the compiler used. Sorry for the long wait.

I'll follow this up with a couple of PRs introducing some cherry-picked functions from SDL_stdint.h.