virtual joystick attach / detach code wrong

[sezero]

As of commit ae4b84a, I made some type / prototype fixes to SDL3 calls, the issues about which were detectable if one does the following change to test:

--- a/src/sdl2_compat.c
+++ b/src/sdl2_compat.c
@@ -108,5 +108,5 @@ extern "C" {
 #define SDL3_SYM(rc,fn,params,args,ret) \
     typedef rc (SDLCALL *SDL3_##fn##_t) params; \
-    static SDL3_##fn##_t SDL3_##fn = NULL;
+    static SDL3_##fn##_t SDL3_##fn = IGNORE_THIS_VERSION_OF_SDL_##fn;
 #include "sdl3_syms.h"

There are three important ones remaining, namely virtual joystick attach and detach calls which can not be pass-throughs:

sdl3_syms.h:727: warning: initialization from incompatible pointer type
sdl3_syms.h:728: warning: initialization from incompatible pointer type
sdl3_syms.h:729: warning: initialization from incompatible pointer type

I added a FIXME about them, here: https://github.com/libsdl-org/sdl2-compat/blob/main/src/sdl3_syms.h#L726-L729

We already have a FIXME in the C code about that I think: https://github.com/libsdl-org/sdl2-compat/blob/main/src/sdl2_compat.c#L4071

Possibly a job for @slouken

[1bsyl]

I think this gets ok with: https://github.com/libsdl-org/sdl2-compat/commit/29bf28e06316eb140325d4e019e0dc4e00bcbffb but not well tested: only triggering the automation virtual joystick stuff.

[1bsyl]

using: + static SDL3_##fn##_t SDL3_##fn = IGNORE_THIS_VERSION_OF_SDL_##fn; is a good trick, maybe should add it so that we can trigger it easily ?

[sezero]

using: + static SDL3_##fn##_t SDL3_##fn = IGNORE_THIS_VERSION_OF_SDL_##fn; is a good trick, maybe should add it so that we can trigger it easily ?

Will result in link failure because we don't link to SDL3, only dlopen it.

[sezero]

I think this gets ok with: 29bf28e but not well tested: only triggering the automation virtual joystick stuff.

Thanks - but can't test myself though.

[1bsyl]

using: + static SDL3_##fn##_t SDL3_##fn = IGNORE_THIS_VERSION_OF_SDL_##fn; is a good trick, maybe should add it so that we can trigger it easily ?

Will result in link failure because we don't link to SDL3, only dlopen it.

I mean, something we can enable just to have the warning!

also, here a PR for changing event instance to id: https://github.com/libsdl-org/sdl2-compat/pull/48

[sezero]

Will result in link failure because we don't link to SDL3, only dlopen it.

I mean, something we can enable just to have the warning!

Well, be my guest :)

[sezero]

I think this gets ok with: 29bf28e but not well tested: only triggering the automation virtual joystick stuff.

Added some notes to the commit about the issues I saw

[1bsyl]

yep, I fixed that thanks ! also added the defines here: 7361441c9b3927a6e5ef53790c22d67c7150e9f2

[sezero]

using: + static SDL3_##fn##_t SDL3_##fn = IGNORE_THIS_VERSION_OF_SDL_##fn; is a good trick, maybe should add it so that we can trigger it easily ?

Will result in link failure because we don't link to SDL3, only dlopen it.

I mean, something we can enable just to have the warning!

I could think of the following: not the prettiest, but should work: make -f Makefile.linux test-protos

diff --git a/src/Makefile.linux b/src/Makefile.linux
index b1f6a2d..07583a7 100644
--- a/src/Makefile.linux
+++ b/src/Makefile.linux
@@ -33,6 +33,10 @@ $(SHLIB): $(OBJ)
    $(CC) $(CFLAGS) $(CPPFLAGS) $(INCLUDES) -o $@ -c $<

 distclean: clean
-   $(RM) *.so*
+   $(RM) *.so* test-protos
 clean:
    $(RM) *.o dynapi/*.o
+
+test-protos: test-protos.c sdl3_include_wrapper.h sdl2_compat.h
+   $(CC) $(CFLAGS) $(CPPFLAGS) $(INCLUDES) -c test-protos.c 2> $@
+   cat $@
diff --git a/src/test-protos.c b/src/test-protos.c
new file mode 100644
index 0000000..850c8c1
--- /dev/null
+++ b/src/test-protos.c
@@ -0,0 +1,23 @@
+#include "sdl3_include_wrapper.h"
+#include "sdl2_compat.h"
+
+#if defined(DisableScreenSaver)
+/*
+This breaks the build when creating SDL_ ## DisableScreenSaver
+/usr/include/X11/X.h:#define DisableScreenSaver    0
+*/
+#undef DisableScreenSaver
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define SDL3_SYM(rc,fn,params,args,ret) \
+    typedef rc (SDLCALL *SDL3_##fn##_t) params; \
+    SDL3_##fn##_t SDL3_##fn = IGNORE_THIS_VERSION_OF_SDL_##fn;
+#include "sdl3_syms.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif

[1bsyl]

I think the "if 0 / if 1" is easier to check for now...

[sezero]

I think the "if 0 / if 1" is easier to check for now...

Yes - I was late to see your commit

[sezero]

Is there anything else to be done for this? (Other than testing.)

[1bsyl]

there is the PR #48 and so that would fixes #22

[sezero]

What else is left to do for this one?

[1bsyl]

I think it's ok !, so closing