Added the timerID to the SDL timer callback

Fixes https://github.com/libsdl-org/SDL/issues/2593

docs/README-migration.md

@@ -1677,6 +1677,13 @@ If you were using this macro for other things besides SDL ticks values, you can
 #define SDL_TICKS_PASSED(A, B)  ((Sint32)((B) - (A)) <= 0)
 ```
 
+The callback passed to SDL_AddTimer() has changed parameters to:
+```c
+Uint32 SDLCALL TimerCallback(void *userdata, SDL_TimerID timerID, Uint32 interval);
+````
+
+The return value of SDL_RemoveTimer() has changed to the standard int error code.
+
 ## SDL_touch.h
 
 SDL_GetTouchName is replaced with SDL_GetTouchDeviceName(), which takes an SDL_TouchID instead of an index.

include/SDL3/SDL_timer.h

@@ -124,6 +124,13 @@ extern SDL_DECLSPEC void SDLCALL SDL_Delay(Uint32 ms);
  */
 extern SDL_DECLSPEC void SDLCALL SDL_DelayNS(Uint64 ns);
 
+/**
+ * Definition of the timer ID type.
+ *
+ * \since This datatype is available since SDL 3.0.0.
+ */
+typedef Uint32 SDL_TimerID;
+
 /**
  * Function prototype for the timer callback function.
  *
@@ -132,9 +139,9 @@ extern SDL_DECLSPEC void SDLCALL SDL_DelayNS(Uint64 ns);
  * the one passed in, the periodic alarm continues, otherwise a new alarm is
  * scheduled. If the callback returns 0, the periodic alarm is cancelled.
  *
+ * \param userdata an arbitrary pointer provided by the app through SDL_AddTimer, for its own use.
+ * \param timerID the current timer being processed
  * \param interval the current callback time interval.
- * \param param an arbitrary pointer provided by the app through SDL_AddTimer,
- *              for its own use.
  * \returns the new callback time interval, or 0 to disable further runs of
  *          the callback.
  *
@@ -146,14 +153,7 @@ extern SDL_DECLSPEC void SDLCALL SDL_DelayNS(Uint64 ns);
  *
  * \sa SDL_AddTimer
  */
-typedef Uint32 (SDLCALL *SDL_TimerCallback)(Uint32 interval, void *param);
-
-/**
- * Definition of the timer ID type.
- *
- * \since This datatype is available since SDL 3.0.0.
- */
-typedef Uint32 SDL_TimerID;
+typedef Uint32 (SDLCALL *SDL_TimerCallback)(void *userdata, SDL_TimerID timerID, Uint32 interval);
 
 /**
  * Call a callback function at a future time.
@@ -179,7 +179,7 @@ typedef Uint32 SDL_TimerID;
  * \param interval the timer delay, in milliseconds, passed to `callback`
  * \param callback the SDL_TimerCallback function to call when the specified
  *                 `interval` elapses
- * \param param a pointer that is passed to `callback`
+ * \param userdata a pointer that is passed to `callback`
  * \returns a timer ID or 0 if an error occurs; call SDL_GetError() for more
  *          information.
  *
@@ -189,22 +189,20 @@ typedef Uint32 SDL_TimerID;
  *
  * \sa SDL_RemoveTimer
  */
-extern SDL_DECLSPEC SDL_TimerID SDLCALL SDL_AddTimer(Uint32 interval,
-                                                 SDL_TimerCallback callback,
-                                                 void *param);
+extern SDL_DECLSPEC SDL_TimerID SDLCALL SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *userdata);
 
 /**
  * Remove a timer created with SDL_AddTimer().
  *
  * \param id the ID of the timer to remove
- * \returns SDL_TRUE if the timer is removed or SDL_FALSE if the timer wasn't
- *          found.
+ * \returns 0 on success or a negative error code on failure; call
+ *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_AddTimer
  */
-extern SDL_DECLSPEC SDL_bool SDLCALL SDL_RemoveTimer(SDL_TimerID id);
+extern SDL_DECLSPEC int SDLCALL SDL_RemoveTimer(SDL_TimerID id);
 
 
 /* Ends C function definitions when using C++ */

src/dynapi/SDL_dynapi_procs.h

@@ -662,7 +662,7 @@ SDL_DYNAPI_PROC(int,SDL_ReleaseCameraFrame,(SDL_Camera *a, SDL_Surface *b),(a,b)
 SDL_DYNAPI_PROC(int,SDL_ReloadGamepadMappings,(void),(),return)
 SDL_DYNAPI_PROC(int,SDL_RemovePath,(const char *a),(a),return)
 SDL_DYNAPI_PROC(int,SDL_RemoveStoragePath,(SDL_Storage *a, const char *b),(a,b),return)
-SDL_DYNAPI_PROC(SDL_bool,SDL_RemoveTimer,(SDL_TimerID a),(a),return)
+SDL_DYNAPI_PROC(int,SDL_RemoveTimer,(SDL_TimerID a),(a),return)
 SDL_DYNAPI_PROC(int,SDL_RenamePath,(const char *a, const char *b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_RenameStoragePath,(SDL_Storage *a, const char *b, const char *c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_RenderClear,(SDL_Renderer *a),(a),return)

src/timer/SDL_timer.c

@@ -31,7 +31,7 @@ typedef struct SDL_Timer
 {
     SDL_TimerID timerID;
     SDL_TimerCallback callback;
-    void *param;
+    void *userdata;
     Uint64 interval;
     Uint64 scheduled;
     SDL_AtomicInt canceled;
@@ -161,7 +161,7 @@ static int SDLCALL SDL_TimerThread(void *_data)
                 interval = 0;
             } else {
                 /* FIXME: We could potentially support sub-millisecond timers now */
-                interval = SDL_MS_TO_NS(current->callback((Uint32)SDL_NS_TO_MS(current->interval), current->param));
+                interval = SDL_MS_TO_NS(current->callback(current->userdata, current->timerID, (Uint32)SDL_NS_TO_MS(current->interval)));
             }
 
             if (interval > 0) {
@@ -269,7 +269,7 @@ void SDL_QuitTimers(void)
     }
 }
 
-SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *param)
+SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *userdata)
 {
     SDL_TimerData *data = &SDL_timer_data;
     SDL_Timer *timer;
@@ -299,7 +299,7 @@ SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *para
     }
     timer->timerID = SDL_GetNextObjectID();
     timer->callback = callback;
-    timer->param = param;
+    timer->userdata = userdata;
     timer->interval = SDL_MS_TO_NS(interval);
     timer->scheduled = SDL_GetTicksNS() + timer->interval;
     SDL_AtomicSet(&timer->canceled, 0);
@@ -329,7 +329,7 @@ SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *para
     return entry->timerID;
 }
 
-SDL_bool SDL_RemoveTimer(SDL_TimerID id)
+int SDL_RemoveTimer(SDL_TimerID id)
 {
     SDL_TimerData *data = &SDL_timer_data;
     SDL_TimerMap *prev, *entry;
@@ -357,7 +357,11 @@ SDL_bool SDL_RemoveTimer(SDL_TimerID id)
         }
         SDL_free(entry);
     }
-    return canceled;
+    if (canceled) {
+        return 0;
+    } else {
+        return SDL_SetError("Timer not found");
+    }
 }
 
 #else
@@ -371,7 +375,7 @@ typedef struct SDL_TimerMap
     int timeoutID;
     Uint32 interval;
     SDL_TimerCallback callback;
-    void *param;
+    void *userdata;
     struct SDL_TimerMap *next;
 } SDL_TimerMap;
 
@@ -385,7 +389,7 @@ static SDL_TimerData SDL_timer_data;
 static void SDL_Emscripten_TimerHelper(void *userdata)
 {
     SDL_TimerMap *entry = (SDL_TimerMap *)userdata;
-    entry->interval = entry->callback(entry->interval, entry->param);
+    entry->interval = entry->callback(entry->userdata, entry->timerID, entry->interval);
     if (entry->interval > 0) {
         entry->timeoutID = emscripten_set_timeout(&SDL_Emscripten_TimerHelper,
                                                   entry->interval,
@@ -410,7 +414,7 @@ void SDL_QuitTimers(void)
     }
 }
 
-SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *param)
+SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *userdata)
 {
     SDL_TimerData *data = &SDL_timer_data;
     SDL_TimerMap *entry;
@@ -421,7 +425,7 @@ SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *para
     }
     entry->timerID = SDL_GetNextObjectID();
     entry->callback = callback;
-    entry->param = param;
+    entry->userdata = userdata;
     entry->interval = interval;
 
     entry->timeoutID = emscripten_set_timeout(&SDL_Emscripten_TimerHelper,
@@ -434,7 +438,7 @@ SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *para
     return entry->timerID;
 }
 
-SDL_bool SDL_RemoveTimer(SDL_TimerID id)
+int SDL_RemoveTimer(SDL_TimerID id)
 {
     SDL_TimerData *data = &SDL_timer_data;
     SDL_TimerMap *prev, *entry;
@@ -455,10 +459,10 @@ SDL_bool SDL_RemoveTimer(SDL_TimerID id)
     if (entry) {
         emscripten_clear_timeout(entry->timeoutID);
         SDL_free(entry);
-
-        return SDL_TRUE;
+        return 0;
+    } else {
+        return SDL_SetError("Timer not found");
     }
-    return SDL_FALSE;
 }
 
 #endif /* !defined(SDL_PLATFORM_EMSCRIPTEN) || !SDL_THREADS_DISABLED */

test/testautomation_timer.c

@@ -101,7 +101,7 @@ static int timer_delayAndGetTicks(void *arg)
 }
 
 /* Test callback */
-static Uint32 SDLCALL timerTestCallback(Uint32 interval, void *param)
+static Uint32 SDLCALL timerTestCallback(void *param, SDL_TimerID timerID, Uint32 interval)
 {
     g_timerCallbackCalled = 1;
 
@@ -121,7 +121,7 @@ static Uint32 SDLCALL timerTestCallback(Uint32 interval, void *param)
 static int timer_addRemoveTimer(void *arg)
 {
     SDL_TimerID id;
-    SDL_bool result;
+    int result;
     int param;
 
     /* Reset state */
@@ -136,13 +136,13 @@ static int timer_addRemoveTimer(void *arg)
     /* Remove timer again and check that callback was not called */
     result = SDL_RemoveTimer(id);
     SDLTest_AssertPass("Call to SDL_RemoveTimer()");
-    SDLTest_AssertCheck(result == SDL_TRUE, "Check result value, expected: %i, got: %i", SDL_TRUE, result);
+    SDLTest_AssertCheck(result == 0, "Check result value, expected: 0, got: %i", result);
     SDLTest_AssertCheck(g_timerCallbackCalled == 0, "Check callback WAS NOT called, expected: 0, got: %i", g_timerCallbackCalled);
 
     /* Try to remove timer again (should be a NOOP) */
     result = SDL_RemoveTimer(id);
     SDLTest_AssertPass("Call to SDL_RemoveTimer()");
-    SDLTest_AssertCheck(result == SDL_FALSE, "Check result value, expected: %i, got: %i", SDL_FALSE, result);
+    SDLTest_AssertCheck(result < 0, "Check result value, expected: <0, got: %i", result);
 
     /* Reset state */
     param = SDLTest_RandomIntegerInRange(-1024, 1024);
@@ -162,7 +162,7 @@ static int timer_addRemoveTimer(void *arg)
     /* Remove timer again and check that callback was called */
     result = SDL_RemoveTimer(id);
     SDLTest_AssertPass("Call to SDL_RemoveTimer()");
-    SDLTest_AssertCheck(result == SDL_FALSE, "Check result value, expected: %i, got: %i", SDL_FALSE, result);
+    SDLTest_AssertCheck(result < 0, "Check result value, expected: <0, got: %i", result);
     SDLTest_AssertCheck(g_timerCallbackCalled == 1, "Check callback WAS called, expected: 1, got: %i", g_timerCallbackCalled);
 
     return TEST_COMPLETED;

test/testlock.c

@@ -100,7 +100,7 @@ Run(void *data)
 }
 
 #ifndef _WIN32
-static Uint32 hit_timeout(Uint32 interval, void *param) {
+static Uint32 hit_timeout(void *param, SDL_TimerID timerID, Uint32 interval) {
     SDL_Log("Hit timeout! Sending SIGINT!");
     (void)raise(SIGINT);
     return 0;

test/testtimer.c

@@ -50,16 +50,18 @@ static int test_sdl_delay_within_bounds(void) {
 static int ticks = 0;
 
 static Uint32 SDLCALL
-ticktock(Uint32 interval, void *param)
+ticktock(void *param, SDL_TimerID timerID, Uint32 interval)
 {
     ++ticks;
     return interval;
 }
 
 static Uint32 SDLCALL
-callback(Uint32 interval, void *param)
+callback(void *param, SDL_TimerID timerID, Uint32 interval)
 {
-    SDL_Log("Timer %" SDL_PRIu32 " : param = %d\n", interval, (int)(uintptr_t)param);
+    int value = (int)(uintptr_t)param;
+    SDL_assert( value == 1 || value == 2 || value == 3 );
+    SDL_Log("Timer %" SDL_PRIu32 " : param = %d\n", interval, value);
     return interval;
 }
 
@@ -182,7 +184,7 @@ int main(int argc, char *argv[])
 
     start_perf = SDL_GetPerformanceCounter();
     for (i = 0; i < 1000000; ++i) {
-        ticktock(0, NULL);
+        ticktock(NULL, 0, 0);
     }
     now_perf = SDL_GetPerformanceCounter();
     SDL_Log("1 million iterations of ticktock took %f ms\n", (double)((now_perf - start_perf) * 1000) / SDL_GetPerformanceFrequency());