Skip to content

Conversation

@suve
Copy link
Collaborator

@suve suve commented Nov 16, 2022

This patch adds a sdlhidapi.inc, a translation of SDL_hidapi.h. It also adds a definition for the cwchar_t type.

@Free-Pascal-meets-SDL-Website
Copy link
Collaborator

Great update! Thanks! I wonder though, if cwchar_t definition shouldn't actually go into ctypes unit. Anyway, why do you hide cwchar_t behind the compiler symbol?

Minor hint: Still don't like the capital U in the middle of unsigned var types though.

Best regards

@suve
Copy link
Collaborator Author

suve commented Nov 20, 2022

The reason for hiding cwchar_t behind a compile-type symbol is that:

  1. For FPC + UNIX, the cwchar_t definition I made simply aliases it to UnixType.wchar_t, with UnixType being added to the uses clause in sdl2.pas.

  2. We {$INCLUDE ctypes.inc} in all the companion libraries (i.e. sdl2_gfx.pas, sdl2_image.pas, sdl2_mixer.pas, sdl2_net.pas and sdl2_ttf.pas)

This leads to the companion library units failing to compile because UnixType.wchar_t cannot be resolved. Check this CI run for an example: https://github.com/PascalGameDevelopment/SDL2-for-Pascal/actions/runs/3479499875/jobs/5818123841

As far as I could see, there were two ways this could be solved:

  • A) Use the UnixType unit in all companion libraries

  • B) Make it so that the cwchar_t type is defined only for the main SDL2 unit and not the companion libraries

I opted for solution B; however, if you think that A is preferable, it's a trivial change.

@Free-Pascal-meets-SDL-Website
Copy link
Collaborator

I see, good solution and great PR!

Tried to quickly fix a tiny conflict by GitHub's web interface. I do not like the resulting (final) merge commit it generates. I wouldn't do that again, sorry for that.

Hint: The failing windows CI run is caused by the remote windows setup. Nothing to worry about.

Best regards
Matthias

@Free-Pascal-meets-SDL-Website Free-Pascal-meets-SDL-Website merged commit 433e7da into PascalGameDevelopment:master Dec 15, 2022
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