Squashed commit of the following: [raii_pyobjects]

closes #4

commit 8f060dc87b
Author: John McCardle <mccardle.john@gmail.com>
Date:   Fri Mar 15 22:20:03 2024 -0400

    Removing std::cout debugging statements

commit c9d5251c71
Author: John McCardle <mccardle.john@gmail.com>
Date:   Fri Mar 15 20:00:57 2024 -0400

    In-place map modification worked

commit 0a8f67e391
Author: John McCardle <mccardle.john@gmail.com>
Date:   Thu Mar 14 23:13:13 2024 -0400

    Stress test is failing: By removing a timer to a function (from inside that function?) I can immediately cause a segfault.

commit 05d9f6a882
Author: John McCardle <mccardle.john@gmail.com>
Date:   Tue Mar 12 22:27:12 2024 -0400

    wow, good test of Key and Click Callable classes. Cleanup, squash, and merge after I give it a lookover in daylight, though.

commit 972768eb26
Author: John McCardle <mccardle.john@gmail.com>
Date:   Tue Mar 12 21:02:48 2024 -0400

    inital PyCallable work; isolate very well behaved usage of PyObject references behind RAII

src/GameEngine.cpp

@@ -69,14 +69,14 @@ void GameEngine::run()
 
 void GameEngine::manageTimer(std::string name, PyObject* target, int interval)
 {
-    //std::cout << "Manage timer called. " << name << " " << interval << std::endl;
     auto it = timers.find(name);
     if (it != timers.end()) // overwrite existing
     {
-        if (target == NULL || target == Py_None) // delete
+        if (target == NULL || target == Py_None)
         {
-            Py_DECREF(timers[name].target);
-            timers.erase(it);
+            // Delete: Overwrite existing timer with one that calls None. This will be deleted in the next timer check
+            // see gitea issue #4: this allows for a timer to be deleted during its own call to itself
+            timers[name] = std::make_shared<PyTimerCallable>(Py_None, 1000, runtime.getElapsedTime().asMilliseconds());
             return;
         }
     }
@@ -85,16 +85,23 @@ void GameEngine::manageTimer(std::string name, PyObject* target, int interval)
         std::cout << "Refusing to initialize timer to None. It's not an error, it's just pointless." << std::endl;
         return;
     }
-    timers[name] = Timer(target, interval, runtime.getElapsedTime().asMilliseconds());
-    Py_INCREF(target);
+    timers[name] = std::make_shared<PyTimerCallable>(target, interval, runtime.getElapsedTime().asMilliseconds());
 }
 
 void GameEngine::testTimers()
 {
     int now = runtime.getElapsedTime().asMilliseconds();
-    for (auto& [name, timer]: timers)
+    auto it = timers.begin();
+    while (it != timers.end())
     {
-        timer.test(now);
+        it->second->test(now);
+        
+        if (it->second->isNone())
+        {
+            it = timers.erase(it);
+        }
+        else
+            it++;
     }
 }
 
@@ -157,8 +164,10 @@ void GameEngine::sUserInput()
             std::string name = currentScene()->action(actionCode);
             currentScene()->doAction(name, actionType);
         }
-        else if (currentScene()->key_callable != NULL && currentScene()->key_callable != Py_None)
+        else if (currentScene()->key_callable)
         {
+            currentScene()->key_callable->call(ActionCode::key_str(event.key.code), actionType);
+            /*
             PyObject* args = Py_BuildValue("(ss)", ActionCode::key_str(event.key.code).c_str(), actionType.c_str());
             PyObject* retval = PyObject_Call(currentScene()->key_callable, args, NULL);
             if (!retval)
@@ -170,6 +179,7 @@ void GameEngine::sUserInput()
             {   
                 std::cout << "key_callable returned a non-None value. It's not an error, it's just not being saved or used." << std::endl;
             }
+            */
         }
         else
         {

src/GameEngine.h

@@ -5,6 +5,7 @@
 #include "McRFPy_API.h"
 #include "IndexTexture.h"
 #include "Timer.h"
+#include "PyCallable.h"
 
 class GameEngine
 {
@@ -20,7 +21,8 @@ class GameEngine
     std::string window_title;
 
     sf::Clock runtime;
-    std::map<std::string, Timer> timers;
+    //std::map<std::string, Timer> timers;
+    std::map<std::string, std::shared_ptr<PyTimerCallable>> timers;
     void testTimers();
 
 public:

src/McRFPy_API.cpp

@@ -453,6 +453,7 @@ PyObject* McRFPy_API::_createScene(PyObject* self, PyObject* args) {
 PyObject* McRFPy_API::_keypressScene(PyObject* self, PyObject* args) {
     PyObject* callable;
 	if (!PyArg_ParseTuple(args, "O", &callable)) return NULL;
+    /*
     if (game->currentScene()->key_callable != NULL and game->currentScene()->key_callable != Py_None)
     {
         Py_DECREF(game->currentScene()->key_callable);
@@ -460,6 +461,8 @@ PyObject* McRFPy_API::_keypressScene(PyObject* self, PyObject* args) {
     Py_INCREF(callable);
     game->currentScene()->key_callable = callable;
     Py_INCREF(Py_None);
+    */
+    game->currentScene()->key_callable = std::make_unique<PyKeyCallable>(callable);
     return Py_None;		
 }
 

src/McRFPy_API.h

@@ -12,12 +12,6 @@ private:
         texture_width = 12, texture_height = 11, // w & h sprite/frame count
         texture_sprite_count = 11 * 12; // t_width * t_height, minus blanks?
 
-    // TODO: this is wrong, load resources @ GameEngineSprite sprite;
-    // sf::Texture texture;
-
-    //std::vector<PyMethodDef> mcrfpyMethodsVector;
-    //static PyObject* PyInit_mcrfpy();
-
     McRFPy_API();
 
 public:

src/PyCallable.cpp

@@ -0,0 +1,114 @@
+#include "PyCallable.h"
+
+PyCallable::PyCallable(PyObject* _target)
+{
+    target = Py_XNewRef(_target);
+}
+
+PyCallable::~PyCallable()
+{
+    if (target)
+        Py_DECREF(target);
+}
+
+PyObject* PyCallable::call(PyObject* args, PyObject* kwargs)
+{
+    return PyObject_Call(target, args, kwargs);
+}
+
+bool PyCallable::isNone()
+{
+    return (target == Py_None || target == NULL);
+}
+
+PyTimerCallable::PyTimerCallable(PyObject* _target, int _interval, int now)
+: PyCallable(_target), interval(_interval), last_ran(now)
+{}
+
+PyTimerCallable::PyTimerCallable()
+: PyCallable(Py_None), interval(0), last_ran(0)
+{}
+
+bool PyTimerCallable::hasElapsed(int now)
+{
+    return now >= last_ran + interval;
+}
+
+void PyTimerCallable::call(int now)
+{
+    PyObject* args = Py_BuildValue("(i)", now);
+    PyObject* retval = PyCallable::call(args, NULL);
+    if (!retval)
+    {
+        PyErr_Print();
+        PyErr_Clear();
+    } else if (retval != Py_None)
+    {
+        std::cout << "timer returned a non-None value. It's not an error, it's just not being saved or used." << std::endl;
+        std::cout << PyUnicode_AsUTF8(PyObject_Repr(retval)) << std::endl;
+    }
+}
+
+bool PyTimerCallable::test(int now)
+{
+    if(hasElapsed(now))
+    {
+        call(now);
+        last_ran = now;
+        return true;
+    }
+    return false;
+}
+
+PyClickCallable::PyClickCallable(PyObject* _target)
+: PyCallable(_target)
+{}
+
+PyClickCallable::PyClickCallable()
+: PyCallable(Py_None)
+{}
+
+void PyClickCallable::call(sf::Vector2f mousepos, std::string button, std::string action)
+{
+    PyObject* args = Py_BuildValue("(iiss)", (int)mousepos.x, (int)mousepos.y, button.c_str(), action.c_str());
+    PyObject* retval = PyCallable::call(args, NULL);
+    if (!retval)
+    {
+        std::cout << "ClickCallable has raised an exception. It's going to STDERR and being dropped:" << std::endl;
+        PyErr_Print();
+        PyErr_Clear();
+    } else if (retval != Py_None)
+    {
+        std::cout << "ClickCallable returned a non-None value. It's not an error, it's just not being saved or used." << std::endl;
+        std::cout << PyUnicode_AsUTF8(PyObject_Repr(retval)) << std::endl;
+    }
+}
+
+PyObject* PyClickCallable::borrow()
+{
+    return target;
+}
+
+PyKeyCallable::PyKeyCallable(PyObject* _target)
+: PyCallable(_target)
+{}
+
+PyKeyCallable::PyKeyCallable()
+: PyCallable(Py_None)
+{}
+
+void PyKeyCallable::call(std::string key, std::string action)
+{
+    if (target == Py_None || target == NULL) return;
+    PyObject* args = Py_BuildValue("(ss)", key.c_str(), action.c_str());
+    PyObject* retval = PyCallable::call(args, NULL);
+    if (!retval)
+    {
+        std::cout << "KeyCallable has raised an exception. It's going to STDERR and being dropped:" << std::endl;
+        PyErr_Print();
+        PyErr_Clear();
+    } else if (retval != Py_None)
+    {
+        std::cout << "KeyCallable returned a non-None value. It's not an error, it's just not being saved or used." << std::endl;
+    }
+}

src/PyCallable.h

@@ -0,0 +1,45 @@
+#pragma once
+#include "Common.h"
+#include "Python.h"
+
+class PyCallable
+{
+protected:
+    PyObject* target;
+    PyCallable(PyObject*);
+    ~PyCallable();
+    PyObject* call(PyObject*, PyObject*);
+public:
+    bool isNone();
+};
+
+class PyTimerCallable: public PyCallable
+{
+private:
+    int interval;
+    int last_ran;
+    void call(int);
+public:
+    bool hasElapsed(int);
+    bool test(int);
+    PyTimerCallable(PyObject*, int, int);
+    PyTimerCallable();
+};
+
+class PyClickCallable: public PyCallable
+{
+public:
+    void call(sf::Vector2f, std::string, std::string);
+    PyObject* borrow();
+    PyClickCallable(PyObject*);
+    PyClickCallable();
+};
+
+class PyKeyCallable: public PyCallable
+{
+public:
+    void call(std::string, std::string);
+    //PyObject* borrow(); // not yet implemented
+    PyKeyCallable(PyObject*);
+    PyKeyCallable();
+};

src/PyScene.cpp

@@ -1,6 +1,7 @@
 #include "PyScene.h"
 #include "ActionCode.h"
 #include "Resources.h"
+#include "PyCallable.h"
 
 PyScene::PyScene(GameEngine* g) : Scene(g)
 {
@@ -27,6 +28,7 @@ void PyScene::do_mouse_input(std::string button, std::string type)
         target = d->click_at(sf::Vector2f(mousepos));
         if (target)
         {
+            /*
             PyObject* args = Py_BuildValue("(iiss)", (int)mousepos.x, (int)mousepos.y, button.c_str(), type.c_str());
             PyObject* retval = PyObject_Call(target->click_callable, args, NULL);
             if (!retval)
@@ -38,6 +40,8 @@ void PyScene::do_mouse_input(std::string button, std::string type)
             {   
                 std::cout << "click_callable returned a non-None value. It's not an error, it's just not being saved or used." << std::endl;
             }
+            */
+            target->click_callable->call(mousepos, button, type);
         }
     }
 }

src/Scene.cpp

@@ -4,7 +4,7 @@
 //Scene::Scene() { game = 0; std::cout << "WARN: default Scene constructor called. (game = " << game << ")" << std::endl;};
 Scene::Scene(GameEngine* g)
 {
-    key_callable = Py_None;
+    key_callable = std::make_unique<PyKeyCallable>();
     game = g; 
     ui_elements = std::make_shared<std::vector<std::shared_ptr<UIDrawable>>>();
 }
@@ -43,6 +43,7 @@ bool Scene::unregisterActionInjected(int code, std::string name)
 
 void Scene::key_register(PyObject* callable)
 {
+    /*
     if (key_callable)
     {
         // decrement reference before overwriting
@@ -50,11 +51,16 @@ void Scene::key_register(PyObject* callable)
     }
     key_callable = callable;
     Py_INCREF(key_callable);
+    */
+    key_callable = std::make_unique<PyKeyCallable>(callable);
 }
 
 void Scene::key_unregister()
 {
+    /*
     if (key_callable == NULL) return;
     Py_DECREF(key_callable);
     key_callable = NULL;
+    */
+    key_callable.reset();
 }

src/Scene.h

@@ -9,6 +9,7 @@
 #include "Common.h"
 #include <list>
 #include "UI.h"
+#include "PyCallable.h"
 //#include "GameEngine.h"
 
 class GameEngine; // forward declare
@@ -41,7 +42,8 @@ public:
     
     std::shared_ptr<std::vector<std::shared_ptr<UIDrawable>>> ui_elements;
 
-    PyObject* key_callable;
+    //PyObject* key_callable;
+    std::unique_ptr<PyKeyCallable> key_callable;
     void key_register(PyObject*);
     void key_unregister();
 };

src/UI.cpp

@@ -56,6 +56,7 @@ UIDrawable* UIGrid::click_at(sf::Vector2f point)
 
 void UIDrawable::click_register(PyObject* callable)
 {
+    /*
     if (click_callable)
     {
         // decrement reference before overwriting
@@ -63,13 +64,18 @@ void UIDrawable::click_register(PyObject* callable)
     }
     click_callable = callable;
     Py_INCREF(click_callable);
+    */
+    click_callable = std::make_unique<PyClickCallable>(callable);
 }
 
 void UIDrawable::click_unregister()
 {
+    /*
     if (click_callable == NULL) return;
     Py_DECREF(click_callable);
     click_callable = NULL;
+    */
+    click_callable.reset();
 }
 
 void UIDrawable::render()
@@ -378,7 +384,7 @@ void UIGrid::render(sf::Vector2f)
     for (auto e : *entities) {
         // TODO skip out-of-bounds entities (grid square not visible at all, check for partially on visible grid squares / floating point grid position)
         //auto drawent = e->cGrid->indexsprite.drawable();
-        auto drawent = e->sprite;
+        auto& drawent = e->sprite;
         //drawent.setScale(zoom, zoom);
         drawent.setScale(zoom);
         auto pixel_pos = sf::Vector2f(

src/UI.h

@@ -5,6 +5,7 @@
 #include "IndexTexture.h"
 #include "Resources.h"
 #include <list>
+#include "PyCallable.h"
 
 enum PyObjectsEnum : int
 {
@@ -27,7 +28,8 @@ public:
     virtual PyObjectsEnum derived_type() = 0;
 
     // Mouse input handling - callable object, methods to find event's destination
-    PyObject* click_callable;
+    //PyObject* click_callable;
+    std::unique_ptr<PyClickCallable> click_callable;
     virtual UIDrawable* click_at(sf::Vector2f point) = 0;
     void click_register(PyObject*);
     void click_unregister();
@@ -322,16 +324,16 @@ static PyObject* PyUIDrawable_get_click(PyUIGridObject* self, void* closure) {
     switch (objtype)
     {
         case PyObjectsEnum::UIFRAME:
-            ptr = ((PyUIFrameObject*)self)->data->click_callable;
+            ptr = ((PyUIFrameObject*)self)->data->click_callable->borrow();
             break;
         case PyObjectsEnum::UICAPTION:
-            ptr = ((PyUICaptionObject*)self)->data->click_callable;
+            ptr = ((PyUICaptionObject*)self)->data->click_callable->borrow();
             break;
         case PyObjectsEnum::UISPRITE:
-            ptr = ((PyUISpriteObject*)self)->data->click_callable;
+            ptr = ((PyUISpriteObject*)self)->data->click_callable->borrow();
             break;
         case PyObjectsEnum::UIGRID:
-            ptr = ((PyUIGridObject*)self)->data->click_callable;
+            ptr = ((PyUIGridObject*)self)->data->click_callable->borrow();
             break;
         default:
             PyErr_SetString(PyExc_TypeError, "no idea how you did that; invalid UIDrawable derived instance for _get_click");

src/UITestScene.cpp

@@ -123,8 +123,9 @@ UITestScene::UITestScene(GameEngine* g) : Scene(g)
     // TODO - reimplement UISprite style rendering within UIEntity class. Entities don't have a screen pixel position, they have a grid position, and grid sets zoom when rendering them.
     auto e5a = std::make_shared<UIEntity>(*e5); // this basic constructor sucks: sprite position + zoom are irrelevant for UIEntity.
     e5a->grid = e5;
-    auto e5as = UISprite(indextex, 85, sf::Vector2f(0, 0), 1.0);
-    e5a->sprite = e5as; // will copy constructor even exist for UISprite...?
+    //auto e5as = UISprite(indextex, 85, sf::Vector2f(0, 0), 1.0);
+    //e5a->sprite = e5as; // will copy constructor even exist for UISprite...?
+    e5a->sprite = UISprite(indextex, 85, sf::Vector2f(0, 0), 1.0);
     e5a->position = sf::Vector2f(1, 0);
 
     e5->entities->push_back(e5a);

src/scripts/game.py

@@ -9,7 +9,7 @@ texture_hot = mcrfpy.Texture("assets/kenney_lava.png", 16, 12, 11)
 
 # Test stuff
 mcrfpy.createScene("boom")
-#mcrfpy.setScene("boom")
+mcrfpy.setScene("boom")
 ui = mcrfpy.sceneUI("boom")
 box = mcrfpy.Frame(40, 60, 200, 300, fill_color=(255,128,0), outline=4.0, outline_color=(64,64,255,96))
 ui.append(box)
@@ -36,10 +36,95 @@ box.children.append(sprite)
 box.children.append(spritecap)
 box.click = click_sprite
 
+f_a = mcrfpy.Frame(250, 60, 80, 80, fill_color=(255, 92, 92))
+f_a_txt = mcrfpy.Caption(5, 5, "0", font)
+
+f_b = mcrfpy.Frame(340, 60, 80, 80, fill_color=(92, 255, 92))
+f_b_txt = mcrfpy.Caption(5, 5, "0", font)
+
+f_c = mcrfpy.Frame(430, 60, 80, 80, fill_color=(92, 92, 255))
+f_c_txt = mcrfpy.Caption(5, 5, "0", font)
+
+
+ui.append(f_a)
+f_a.children.append(f_a_txt)
+ui.append(f_b)
+f_b.children.append(f_b_txt)
+ui.append(f_c)
+f_c.children.append(f_c_txt)
+
+import sys
+def ding(*args):
+    f_a_txt.text = str(sys.getrefcount(ding)) + " refs"
+    f_b_txt.text = sys.getrefcount(dong)
+    f_c_txt.text = sys.getrefcount(stress_test)
+
+def dong(*args):
+    f_a_txt.text = str(sys.getrefcount(ding)) + " refs"
+    f_b_txt.text = sys.getrefcount(dong)
+    f_c_txt.text = sys.getrefcount(stress_test)
+
+running = False
+timers = []
+
+def add_ding():
+    global timers
+    n = len(timers)
+    mcrfpy.setTimer(f"timer{n}", ding, 100)
+    print("+1 ding:", timers)
+
+def add_dong():
+    global timers
+    n = len(timers)
+    mcrfpy.setTimer(f"timer{n}", dong, 100)
+    print("+1 dong:", timers)
+
+def remove_random():
+    global timers
+    target = random.choice(timers)
+    print("-1 timer:", target)
+    print("remove from list")
+    timers.remove(target)
+    print("delTimer")
+    mcrfpy.delTimer(target)
+    print("done")
+
+import random
+import time
+def stress_test(*args):
+    global running
+    global timers
+    if not running:
+        print("stress test initial")
+        running = True
+        timers.append("recurse")
+        add_ding()
+        add_dong()
+        mcrfpy.setTimer("recurse", stress_test, 1000)
+        mcrfpy.setTimer("terminate", lambda *args: mcrfpy.delTimer("recurse"), 30000)
+        ding(); dong()
+    else:
+        #print("stress test random activity")
+        #random.choice([
+        #    add_ding,
+        #    add_dong,
+        #    remove_random
+        #    ])()
+        #print(timers)
+        print("Segfaultin' time")
+        mcrfpy.delTimer("recurse")
+        print("Does this still work?")
+        time.sleep(0.5)
+        print("How about now?")
+
+
+stress_test()
+
+
 # Loading Screen
 mcrfpy.createScene("loading")
 ui = mcrfpy.sceneUI("loading")
-mcrfpy.setScene("loading")
+#mcrfpy.setScene("loading")
 logo_texture = mcrfpy.Texture("assets/temp_logo.png", 1024, 1, 1)
 logo_sprite = mcrfpy.Sprite(50, 50, logo_texture, 0, 0.5)
 ui.append(logo_sprite)