From b0b17f46333f387d0b650f43e56078705b98c680 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Tue, 6 Jan 2026 20:13:51 -0500 Subject: [PATCH] timer fixes: timers managed by engine can run in the background. Closes #180 --- src/McRFPy_API.cpp | 51 +++++++++++++++++++++++++++++++++++++++------- src/PyTimer.cpp | 16 ++++++--------- src/Timer.cpp | 4 ++-- src/Timer.h | 8 +++++--- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/McRFPy_API.cpp b/src/McRFPy_API.cpp index db33395..813bea9 100644 --- a/src/McRFPy_API.cpp +++ b/src/McRFPy_API.cpp @@ -911,6 +911,32 @@ PyObject* McRFPy_API::_setScene(PyObject* self, PyObject* args) { return Py_None; } +// #180: Create a Python wrapper for an existing C++ Timer (for orphaned timers) +static PyObject* createTimerWrapper(std::shared_ptr timer) +{ + // Allocate new PyTimerObject + PyTypeObject* type = &mcrfpydef::PyTimerType; + PyTimerObject* obj = (PyTimerObject*)type->tp_alloc(type, 0); + if (!obj) return nullptr; + + // Initialize - use placement new for std::string + new(&obj->name) std::string(timer->name); + obj->data = timer; // Share the existing C++ Timer + obj->weakreflist = nullptr; + + // Register in cache for future lookups + if (timer->serial_number == 0) { + timer->serial_number = PythonObjectCache::getInstance().assignSerial(); + } + PyObject* weakref = PyWeakref_NewRef((PyObject*)obj, NULL); + if (weakref) { + PythonObjectCache::getInstance().registerObject(timer->serial_number, weakref); + Py_DECREF(weakref); + } + + return (PyObject*)obj; +} + // #173: Get all timers as a tuple of Python Timer objects PyObject* McRFPy_API::api_get_timers() { @@ -918,15 +944,25 @@ PyObject* McRFPy_API::api_get_timers() return PyTuple_New(0); } - // Count timers that have Python wrappers std::vector timer_objs; for (auto& pair : game->timers) { auto& timer = pair.second; - if (timer && timer->serial_number != 0) { - PyObject* timer_obj = PythonObjectCache::getInstance().lookup(timer->serial_number); - if (timer_obj && timer_obj != Py_None) { - timer_objs.push_back(timer_obj); - } + if (!timer) continue; + + PyObject* timer_obj = nullptr; + + // Try to find existing Python wrapper + if (timer->serial_number != 0) { + timer_obj = PythonObjectCache::getInstance().lookup(timer->serial_number); + } + + // #180: If no wrapper exists (orphaned timer), create a new one + if (!timer_obj || timer_obj == Py_None) { + timer_obj = createTimerWrapper(timer); + } + + if (timer_obj && timer_obj != Py_None) { + timer_objs.push_back(timer_obj); } } @@ -934,7 +970,8 @@ PyObject* McRFPy_API::api_get_timers() if (!tuple) return NULL; for (Py_ssize_t i = 0; i < static_cast(timer_objs.size()); i++) { - Py_INCREF(timer_objs[i]); + // timer_objs already has a reference from lookup() or createTimerWrapper() + // PyTuple_SET_ITEM steals that reference, so no additional INCREF needed PyTuple_SET_ITEM(tuple, i, timer_objs[i]); } diff --git a/src/PyTimer.cpp b/src/PyTimer.cpp index 9d43b66..b59df69 100644 --- a/src/PyTimer.cpp +++ b/src/PyTimer.cpp @@ -80,8 +80,8 @@ int PyTimer::init(PyTimerObject* self, PyObject* args, PyObject* kwds) { current_time = Resources::game->runtime.getElapsedTime().asMilliseconds(); } - // Create the timer with start parameter - self->data = std::make_shared(callback, interval, current_time, (bool)once, (bool)start); + // Create the timer with start parameter and name (#180) + self->data = std::make_shared(callback, interval, current_time, (bool)once, (bool)start, self->name); // Register in Python object cache if (self->data->serial_number == 0) { @@ -112,18 +112,14 @@ void PyTimer::dealloc(PyTimerObject* self) { PyObject_ClearWeakRefs((PyObject*)self); } - // Remove from game engine if still registered - if (Resources::game && !self->name.empty()) { - auto it = Resources::game->timers.find(self->name); - if (it != Resources::game->timers.end() && it->second == self->data) { - Resources::game->timers.erase(it); - } - } + // #180: DON'T remove from game->timers - timer keeps running even without Python wrapper. + // The engine's shared_ptr keeps the Timer alive. When timer.stop() is called or + // a one-shot timer fires, testTimers() removes it from the map. // Explicitly destroy std::string self->name.~basic_string(); - // Clear shared_ptr - this is the only place that truly destroys the Timer + // Clear our shared_ptr - if engine still has one, Timer lives on self->data.reset(); Py_TYPE(self)->tp_free((PyObject*)self); diff --git a/src/Timer.cpp b/src/Timer.cpp index cda7775..47bfb45 100644 --- a/src/Timer.cpp +++ b/src/Timer.cpp @@ -4,9 +4,9 @@ #include "McRFPy_API.h" #include "GameEngine.h" -Timer::Timer(PyObject* _target, int _interval, int now, bool _once, bool _start) +Timer::Timer(PyObject* _target, int _interval, int now, bool _once, bool _start, const std::string& _name) : callback(std::make_shared(_target)), interval(_interval), last_ran(now), - paused(false), pause_start_time(0), total_paused_time(0), once(_once), stopped(!_start) + paused(false), pause_start_time(0), total_paused_time(0), once(_once), stopped(!_start), name(_name) {} Timer::Timer() diff --git a/src/Timer.h b/src/Timer.h index 1c87b46..2667b05 100644 --- a/src/Timer.h +++ b/src/Timer.h @@ -2,6 +2,7 @@ #include "Common.h" #include "Python.h" #include +#include class PyCallable; class GameEngine; // forward declare @@ -12,7 +13,7 @@ private: std::shared_ptr callback; int interval; int last_ran; - + // Pause/resume support bool paused; int pause_start_time; @@ -23,12 +24,13 @@ private: // Stopped state: timer is not in engine map, but callback is preserved bool stopped; - + public: uint64_t serial_number = 0; // For Python object cache + std::string name; // Store name for creating Python wrappers (#180) Timer(); // for map to build - Timer(PyObject* target, int interval, int now, bool once = false, bool start = true); + Timer(PyObject* target, int interval, int now, bool once = false, bool start = true, const std::string& name = ""); ~Timer(); // Core timer functionality