Remove deprecated registerPyAction/registerInputAction system (closes #2, closes #3)

This is our largest net-negative commit yet\! Removed the entire deprecated
action registration system that provided unnecessary two-step indirection:
keyboard → action string → Python callback

Removed components:
- McRFPy_API::_registerPyAction() and _registerInputAction() methods
- McRFPy_API::callbacks map for storing Python callables
- McRFPy_API::doAction() method for executing callbacks
- ACTIONPY macro from Scene.h for detecting "_py" suffixed actions
- Scene::registerActionInjected() and unregisterActionInjected() methods
- tests/api_registerPyAction_issue2_test.py (tested deprecated functionality)

The game now exclusively uses keypressScene() for keyboard input handling,
which is simpler and more direct. Also commented out the unused _camFollow
function that referenced non-existent do_camfollow variable.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
John McCardle 2025-07-03 21:43:22 -04:00
parent cc8a7d20e8
commit 281800cd23
6 changed files with 2 additions and 180 deletions

View File

@ -7,7 +7,6 @@
#include <filesystem>
#include <cstring>
std::map<std::string, PyObject*> McRFPy_API::callbacks;
std::vector<sf::SoundBuffer> McRFPy_API::soundbuffers;
sf::Music McRFPy_API::music;
sf::Sound McRFPy_API::sfx;
@ -18,11 +17,6 @@ PyObject* McRFPy_API::mcrf_module;
static PyMethodDef mcrfpyMethods[] = {
{"registerPyAction", McRFPy_API::_registerPyAction, METH_VARARGS,
"Register a callable Python object to correspond to an action string. (actionstr, callable)"},
{"registerInputAction", McRFPy_API::_registerInputAction, METH_VARARGS,
"Register a SFML input code to correspond to an action string. (input_code, actionstr)"},
{"createSoundBuffer", McRFPy_API::_createSoundBuffer, METH_VARARGS, "(filename)"},
{"loadMusic", McRFPy_API::_loadMusic, METH_VARARGS, "(filename)"},
@ -344,63 +338,7 @@ void McRFPy_API::REPL_device(FILE * fp, const char *filename)
}
// python connection
PyObject* McRFPy_API::_registerPyAction(PyObject *self, PyObject *args)
{
PyObject* callable;
const char * actionstr;
if (!PyArg_ParseTuple(args, "sO", &actionstr, &callable)) return NULL;
//TODO: if the string already exists in the callbacks map,
// decrease our reference count so it can potentially be garbage collected
callbacks[std::string(actionstr)] = callable;
Py_INCREF(callable);
// return None correctly
Py_INCREF(Py_None);
return Py_None;
}
PyObject* McRFPy_API::_registerInputAction(PyObject *self, PyObject *args)
{
int action_code;
const char * actionstr;
if (!PyArg_ParseTuple(args, "iz", &action_code, &actionstr)) return NULL;
bool success;
if (actionstr == NULL) { // Action provided is None, i.e. unregister
std::cout << "Unregistering\n";
success = game->currentScene()->unregisterActionInjected(action_code, std::string(actionstr) + "_py");
} else {
std::cout << "Registering " << actionstr << "_py to " << action_code << "\n";
success = game->currentScene()->registerActionInjected(action_code, std::string(actionstr) + "_py");
}
success ? Py_INCREF(Py_True) : Py_INCREF(Py_False);
return success ? Py_True : Py_False;
}
void McRFPy_API::doAction(std::string actionstr) {
// hard coded actions that require no registration
//std::cout << "Calling Python Action: " << actionstr;
if (!actionstr.compare("startrepl")) return McRFPy_API::REPL();
if (callbacks.find(actionstr) == callbacks.end())
{
//std::cout << " (not found)" << std::endl;
return;
}
//std::cout << " (" << PyUnicode_AsUTF8(PyObject_Repr(callbacks[actionstr])) << ")" << std::endl;
PyObject* retval = PyObject_Call(callbacks[actionstr], PyTuple_New(0), NULL);
if (!retval)
{
std::cout << "doAction 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 << "doAction returned a non-None value. It's not an error, it's just not being saved or used." << std::endl;
}
}
/*
PyObject* McRFPy_API::_refreshFov(PyObject* self, PyObject* args) {
@ -475,14 +413,8 @@ PyObject* McRFPy_API::_getSoundVolume(PyObject* self, PyObject* args) {
// Removed deprecated player_input, computerTurn, playerTurn functions
// These were part of the old turn-based system that is no longer used
//std::cout << "grid center: " << ag->center_x << ", " << ag->center_y << std::endl <<
// "player grid pos: " << e->cGrid->x << ", " << e->cGrid->y << std::endl <<
// "player sprite pos: " << e->cGrid->indexsprite.x << ", " << e->cGrid->indexsprite.y << std::endl;
ag->center_x = e->cGrid->indexsprite.x * ag->grid_size + ag->grid_size * 0.5;
ag->center_y = e->cGrid->indexsprite.y * ag->grid_size + ag->grid_size * 0.5;
}
}
/*
PyObject* McRFPy_API::_camFollow(PyObject* self, PyObject* args) {
PyObject* set_camfollow = NULL;
//std::cout << "camFollow Parse Args" << std::endl;

View File

@ -40,9 +40,6 @@ public:
static sf::Music music;
static sf::Sound sfx;
static std::map<std::string, PyObject*> callbacks;
static PyObject* _registerPyAction(PyObject*, PyObject*);
static PyObject* _registerInputAction(PyObject*, PyObject*);
static PyObject* _createSoundBuffer(PyObject*, PyObject*);
static PyObject* _loadMusic(PyObject*, PyObject*);
@ -70,7 +67,6 @@ public:
// accept keyboard input from scene
static sf::Vector2i cursor_position;
static void doAction(std::string);
static void executeScript(std::string);
static void executePyString(std::string);

View File

@ -54,10 +54,7 @@ void PyScene::do_mouse_input(std::string button, std::string type)
void PyScene::doAction(std::string name, std::string type)
{
if (ACTIONPY) {
McRFPy_API::doAction(name.substr(0, name.size() - 3));
}
else if (name.compare("left") == 0 || name.compare("rclick") == 0 || name.compare("wheel_up") == 0 || name.compare("wheel_down") == 0) {
if (name.compare("left") == 0 || name.compare("rclick") == 0 || name.compare("wheel_up") == 0 || name.compare("wheel_down") == 0) {
do_mouse_input(name, type);
}
else if ACTIONONCE("debug_menu") {

View File

@ -30,16 +30,6 @@ std::string Scene::action(int code)
return actions[code];
}
bool Scene::registerActionInjected(int code, std::string name)
{
std::cout << "Inject registered action - default implementation\n";
return false;
}
bool Scene::unregisterActionInjected(int code, std::string name)
{
return false;
}
void Scene::key_register(PyObject* callable)
{

View File

@ -4,7 +4,6 @@
#define ACTION(X, Y) (name.compare(X) == 0 && type.compare(Y) == 0)
#define ACTIONONCE(X) ((name.compare(X) == 0 && type.compare("start") == 0 && !actionState[name]))
#define ACTIONAFTER(X) ((name.compare(X) == 0 && type.compare("end") == 0))
#define ACTIONPY ((name.size() > 3 && name.compare(name.size() - 3, 3, "_py") == 0))
#include "Common.h"
#include <list>
@ -37,8 +36,6 @@ public:
bool hasAction(int);
std::string action(int);
virtual bool registerActionInjected(int, std::string);
virtual bool unregisterActionInjected(int, std::string);
std::shared_ptr<std::vector<std::shared_ptr<UIDrawable>>> ui_elements;

View File

@ -1,90 +0,0 @@
#!/usr/bin/env python3
"""Test for registerPyAction - Related to issue #2 (review necessity)"""
import mcrfpy
from mcrfpy import automation
from datetime import datetime
action_calls = {"test_action": 0, "another_action": 0}
def test_action_handler():
"""Handler for test_action"""
action_calls["test_action"] += 1
print(f"test_action called: {action_calls['test_action']} times")
def another_action_handler():
"""Handler for another_action"""
action_calls["another_action"] += 1
print(f"another_action called: {action_calls['another_action']} times")
def test_registerPyAction():
"""Test registerPyAction functionality (Alpha Blocker Issue #2)"""
print("Testing registerPyAction (Issue #2 - Review necessity)...")
print("This is marked as an Alpha Blocker - may need removal")
# Register actions
try:
mcrfpy.registerPyAction("test_action", test_action_handler)
print("✓ Registered test_action")
mcrfpy.registerPyAction("another_action", another_action_handler)
print("✓ Registered another_action")
except Exception as e:
print(f"✗ registerPyAction failed: {e}")
print("FAIL")
return
# Test registerInputAction to map keys to actions
try:
# These are SFML key codes - may need adjustment
mcrfpy.registerInputAction(0, "test_action") # Key A
mcrfpy.registerInputAction(1, "another_action") # Key B
print("✓ Registered input mappings")
except Exception as e:
print(f"✗ registerInputAction failed: {e}")
# Note: In headless mode, we can't easily trigger these actions
# They would normally be triggered by keyboard input
print("\nAnalysis for Issue #2:")
print("- registerPyAction allows mapping string names to Python callbacks")
print("- registerInputAction maps keyboard codes to action strings")
print("- This creates a two-step indirection: key -> action string -> callback")
print("- Modern approach might be direct key -> callback mapping")
print("- Or use keypressScene() for all keyboard handling")
# Try to trigger actions programmatically if possible
# This depends on internal implementation
def check_results(runtime):
print(f"\nAction call counts:")
print(f"- test_action: {action_calls['test_action']}")
print(f"- another_action: {action_calls['another_action']}")
if action_calls["test_action"] > 0 or action_calls["another_action"] > 0:
print("✓ Actions were triggered")
else:
print("✗ No actions triggered (expected in headless mode)")
# Take screenshot
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
filename = f"test_registerPyAction_issue2_{timestamp}.png"
automation.screenshot(filename)
print(f"Screenshot saved: {filename}")
print("PASS - Test completed, Issue #2 needs review for removal")
mcrfpy.delTimer("check_results")
# In headless mode, run synchronously
print("\nAction call counts:")
print(f"- test_action: {action_calls['test_action']}")
print(f"- another_action: {action_calls['another_action']}")
print("✗ No actions triggered (expected in headless mode)")
print("PASS - Test completed, Issue #2 needs review for removal")
# Run test directly in headless mode
test_registerPyAction()
# Exit cleanly
import sys
sys.exit(0)