virtual joystick attach / detach code wrong
sezero opened this issue · 15 comments
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
I think this gets ok with: 29bf28e
but not well tested: only triggering the automation virtual joystick stuff.
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 ?
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 think this gets ok with: 29bf28e but not well tested: only triggering the automation virtual joystick stuff.
Thanks - but can't test myself though.
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: #48
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 :)
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
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" {
+#endifI think the "if 0 / if 1" is easier to check for now...
I think the "if 0 / if 1" is easier to check for now...
Yes - I was late to see your commit
Is there anything else to be done for this? (Other than testing.)
What else is left to do for this one?
I think it's ok !, so closing