john/McRogueFace
1
0
Fork 0
Code Issues 41 Pull requests Projects 1 Releases Wiki Activity

feat: Migrate to Python 3.14 (closes #135)

Browse source 
Replace deprecated Python C API calls with modern PyConfig-based initialization:
- PySys_SetArgvEx() -> PyConfig.argv (deprecated since 3.11)
- Py_InspectFlag -> PyConfig.inspect (deprecated since 3.12)

Fix critical memory safety bugs discovered during migration:
- PyColor::from_arg() and PyVector::from_arg() now return new references
  instead of borrowed references, preventing use-after-free when callers
  call Py_DECREF on the result
- GameEngine::testTimers() now holds a local shared_ptr copy during
  callback execution, preventing use-after-free when timer callbacks
  call delTimer() on themselves

Fix double script execution bug with --exec flag:
- Scripts were running twice because GameEngine constructor executed them,
  then main.cpp deleted and recreated the engine
- Now reuses existing engine and just sets auto_exit_after_exec flag

Update test syntax to use keyword arguments for Frame/Caption constructors.

Test results: 127/130 passing (97.7%)

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
John McCardle 2025-11-26 17:48:12 -05:00
commit 28396b65c9
14 changed files with 240 additions and 203 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/CommandLineParser.cpp
View file

@ -176,5 +176,5 @@ void CommandLineParser::print_help() {
}
void CommandLineParser::print_version() {
std::cout << "Python 3.12.0 (McRogueFace embedded)\n";
std::cout << "Python 3.14.0 (McRogueFace embedded)\n";
}

11
src/GameEngine.cpp
View file

@ -336,9 +336,14 @@ void GameEngine::testTimers()
auto it = timers.begin();
while (it != timers.end())
{
it->second->test(now);
// Remove timers that have been cancelled or are one-shot and fired
// Keep a local copy of the timer to prevent use-after-free.
// If the callback calls delTimer(), the map entry gets replaced,
// but we need the Timer object to survive until test() returns.
auto timer = it->second;
timer->test(now);
// Remove timers that have been cancelled or are one-shot and fired.
// Note: Check it->second (current map value) in case callback replaced it.
if (!it->second->getCallback() || it->second->getCallback() == Py_None)
{
it = timers.erase(it);

1
src/GameEngine.h
View file

@ -154,6 +154,7 @@ public:
void setWindowScale(float);
bool isHeadless() const { return headless; }
const McRogueFaceConfig& getConfig() const { return config; }
void setAutoExitAfterExec(bool enabled) { config.auto_exit_after_exec = enabled; }
void processEvent(const sf::Event& event);
// Window property accessors

100
src/McRFPy_API.cpp
View file

@ -397,10 +397,10 @@ PyStatus init_python(const char *program_name)
// search paths for python libs/modules/scripts
const wchar_t* str_arr[] = {
L"/scripts",
L"/lib/Python/lib.linux-x86_64-3.12",
L"/lib/Python/lib.linux-x86_64-3.14",
L"/lib/Python",
L"/lib/Python/Lib",
L"/venv/lib/python3.12/site-packages"
L"/venv/lib/python3.14/site-packages"
};
@ -419,61 +419,107 @@ PyStatus init_python(const char *program_name)
return status;
}
PyStatus McRFPy_API::init_python_with_config(const McRogueFaceConfig& config, int argc, char** argv)
PyStatus McRFPy_API::init_python_with_config(const McRogueFaceConfig& config)
{
// If Python is already initialized, just return success
if (Py_IsInitialized()) {
return PyStatus_Ok();
}
PyStatus status;
PyConfig pyconfig;
PyConfig_InitIsolatedConfig(&pyconfig);
// Configure UTF-8 for stdio
PyConfig_SetString(&pyconfig, &pyconfig.stdio_encoding, L"UTF-8");
PyConfig_SetString(&pyconfig, &pyconfig.stdio_errors, L"surrogateescape");
pyconfig.configure_c_stdio = 1;
// CRITICAL: Pass actual command line arguments to Python
status = PyConfig_SetBytesArgv(&pyconfig, argc, argv);
// Set interactive mode (replaces deprecated Py_InspectFlag)
if (config.interactive_mode) {
pyconfig.inspect = 1;
}
// Don't modify sys.path based on script location (replaces PySys_SetArgvEx updatepath=0)
pyconfig.safe_path = 1;
// Construct Python argv from config (replaces deprecated PySys_SetArgvEx)
// Python convention:
// - Script mode: argv[0] = script_path, argv[1:] = script_args
// - -c mode: argv[0] = "-c"
// - -m mode: argv[0] = module_name, argv[1:] = script_args
// - Interactive only: argv[0] = ""
std::vector<std::wstring> argv_storage;
if (!config.script_path.empty()) {
// Script execution: argv[0] = script path
argv_storage.push_back(config.script_path.wstring());
for (const auto& arg : config.script_args) {
std::wstring warg(arg.begin(), arg.end());
argv_storage.push_back(warg);
}
} else if (!config.python_command.empty()) {
// -c command: argv[0] = "-c"
argv_storage.push_back(L"-c");
} else if (!config.python_module.empty()) {
// -m module: argv[0] = module name
std::wstring wmodule(config.python_module.begin(), config.python_module.end());
argv_storage.push_back(wmodule);
for (const auto& arg : config.script_args) {
std::wstring warg(arg.begin(), arg.end());
argv_storage.push_back(warg);
}
} else {
// Interactive mode or no script: argv[0] = ""
argv_storage.push_back(L"");
}
// Build wchar_t* array for PyConfig
std::vector<wchar_t*> argv_ptrs;
for (auto& ws : argv_storage) {
argv_ptrs.push_back(const_cast<wchar_t*>(ws.c_str()));
}
status = PyConfig_SetWideStringList(&pyconfig, &pyconfig.argv,
argv_ptrs.size(), argv_ptrs.data());
if (PyStatus_Exception(status)) {
return status;
}
// Check if we're in a virtual environment
auto exe_path = std::filesystem::path(argv[0]);
auto exe_dir = exe_path.parent_path();
auto exe_wpath = executable_filename();
auto exe_path_fs = std::filesystem::path(exe_wpath);
auto exe_dir = exe_path_fs.parent_path();
auto venv_root = exe_dir.parent_path();
if (std::filesystem::exists(venv_root / "pyvenv.cfg")) {
// We're running from within a venv!
// Add venv's site-packages to module search paths
auto site_packages = venv_root / "lib" / "python3.12" / "site-packages";
auto site_packages = venv_root / "lib" / "python3.14" / "site-packages";
PyWideStringList_Append(&pyconfig.module_search_paths,
site_packages.wstring().c_str());
pyconfig.module_search_paths_set = 1;
}
// Set Python home to our bundled Python
auto python_home = executable_path() + L"/lib/Python";
PyConfig_SetString(&pyconfig, &pyconfig.home, python_home.c_str());
// Set up module search paths
#if __PLATFORM_SET_PYTHON_SEARCH_PATHS == 1
if (!pyconfig.module_search_paths_set) {
pyconfig.module_search_paths_set = 1;
}
// search paths for python libs/modules/scripts
const wchar_t* str_arr[] = {
L"/scripts",
L"/lib/Python/lib.linux-x86_64-3.12",
L"/lib/Python/lib.linux-x86_64-3.14",
L"/lib/Python",
L"/lib/Python/Lib",
L"/venv/lib/python3.12/site-packages"
L"/venv/lib/python3.14/site-packages"
};
for(auto s : str_arr) {
status = PyWideStringList_Append(&pyconfig.module_search_paths, (executable_path() + s).c_str());
if (PyStatus_Exception(status)) {
@ -481,15 +527,13 @@ PyStatus McRFPy_API::init_python_with_config(const McRogueFaceConfig& config, in
}
}
#endif
// Register mcrfpy module before initialization
if (!Py_IsInitialized()) {
PyImport_AppendInittab("mcrfpy", &PyInit_mcrfpy);
}
PyImport_AppendInittab("mcrfpy", &PyInit_mcrfpy);
status = Py_InitializeFromConfig(&pyconfig);
PyConfig_Clear(&pyconfig);
return status;
}
@ -535,9 +579,9 @@ void McRFPy_API::api_init() {
//setSpriteTexture(0);
}
void McRFPy_API::api_init(const McRogueFaceConfig& config, int argc, char** argv) {
// Initialize Python with proper argv - this is CRITICAL
PyStatus status = init_python_with_config(config, argc, argv);
void McRFPy_API::api_init(const McRogueFaceConfig& config) {
// Initialize Python with proper argv constructed from config
PyStatus status = init_python_with_config(config);
if (PyStatus_Exception(status)) {
Py_ExitStatusException(status);
}

4
src/McRFPy_API.h
View file

@ -29,8 +29,8 @@ public:
//static void setSpriteTexture(int);
inline static GameEngine* game;
static void api_init();
static void api_init(const McRogueFaceConfig& config, int argc, char** argv);
static PyStatus init_python_with_config(const McRogueFaceConfig& config, int argc, char** argv);
static void api_init(const McRogueFaceConfig& config);
static PyStatus init_python_with_config(const McRogueFaceConfig& config);
static void api_shutdown();
// Python API functionality - use mcrfpy.* in scripts
//static PyObject* _drawSprite(PyObject*, PyObject*);

1
src/PyColor.cpp
View file

@ -236,6 +236,7 @@ PyColorObject* PyColor::from_arg(PyObject* args)
// Check if args is already a Color instance
if (PyObject_IsInstance(args, (PyObject*)type.get())) {
Py_INCREF(args); // Return new reference so caller can safely DECREF
return (PyColorObject*)args;
}

40
src/PyVector.cpp
View file

@ -1,6 +1,7 @@
#include "PyVector.h"
#include "PyObjectUtils.h"
#include "McRFPy_Doc.h"
#include "PyRAII.h"
#include <cmath>
PyGetSetDef PyVector::getsetters[] = {
@ -261,35 +262,46 @@ int PyVector::set_member(PyObject* obj, PyObject* value, void* closure)
PyVectorObject* PyVector::from_arg(PyObject* args)
{
auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector");
if (PyObject_IsInstance(args, (PyObject*)type)) return (PyVectorObject*)args;
auto obj = (PyVectorObject*)type->tp_alloc(type, 0);
// Use RAII for type reference management
PyRAII::PyTypeRef type("Vector", McRFPy_API::mcrf_module);
if (!type) {
return NULL;
}
// Check if args is already a Vector instance
if (PyObject_IsInstance(args, (PyObject*)type.get())) {
Py_INCREF(args); // Return new reference so caller can safely DECREF
return (PyVectorObject*)args;
}
// Create new Vector object using RAII
PyRAII::PyObjectRef obj(type->tp_alloc(type.get(), 0), true);
if (!obj) {
return NULL;
}
// Handle different input types
if (PyTuple_Check(args)) {
// It's already a tuple, pass it directly to init
int err = init(obj, args, NULL);
int err = init((PyVectorObject*)obj.get(), args, NULL);
if (err) {
Py_DECREF(obj);
// obj will be automatically cleaned up when it goes out of scope
return NULL;
}
} else {
// Wrap single argument in a tuple for init
PyObject* tuple = PyTuple_Pack(1, args);
PyRAII::PyObjectRef tuple(PyTuple_Pack(1, args), true);
if (!tuple) {
Py_DECREF(obj);
return NULL;
}
int err = init(obj, tuple, NULL);
Py_DECREF(tuple);
int err = init((PyVectorObject*)obj.get(), tuple.get(), NULL);
if (err) {
Py_DECREF(obj);
return NULL;
}
}
return obj;
// Release ownership and return
return (PyVectorObject*)obj.release();
}
// Arithmetic operations

65
src/main.cpp
View file

@ -11,27 +11,27 @@
// Forward declarations
int run_game_engine(const McRogueFaceConfig& config);
int run_python_interpreter(const McRogueFaceConfig& config, int argc, char* argv[]);
int run_python_interpreter(const McRogueFaceConfig& config);
int main(int argc, char* argv[])
{
McRogueFaceConfig config;
CommandLineParser parser(argc, argv);
// Parse arguments
auto parse_result = parser.parse(config);
if (parse_result.should_exit) {
return parse_result.exit_code;
}
// Special handling for -m module: let Python handle modules properly
if (!config.python_module.empty()) {
config.python_mode = true;
}
// Initialize based on configuration
if (config.python_mode) {
return run_python_interpreter(config, argc, argv);
return run_python_interpreter(config);
} else {
return run_game_engine(config);
}
@ -52,13 +52,13 @@ int run_game_engine(const McRogueFaceConfig& config)
return 0;
}
int run_python_interpreter(const McRogueFaceConfig& config, int argc, char* argv[])
int run_python_interpreter(const McRogueFaceConfig& config)
{
// Create a game engine with the requested configuration
GameEngine* engine = new GameEngine(config);
// Initialize Python with configuration
McRFPy_API::init_python_with_config(config, argc, argv);
// Initialize Python with configuration (argv is constructed from config)
McRFPy_API::init_python_with_config(config);
// Import mcrfpy module and store reference
McRFPy_API::mcrf_module = PyImport_ImportModule("mcrfpy");
@ -116,49 +116,28 @@ int run_python_interpreter(const McRogueFaceConfig& config, int argc, char* argv
}
}
else if (!config.python_module.empty()) {
// Execute module using runpy
std::string run_module_code =
"import sys\n"
// Execute module using runpy (sys.argv already set at init time)
std::string run_module_code =
"import runpy\n"
"sys.argv = ['" + config.python_module + "'";
for (const auto& arg : config.script_args) {
run_module_code += ", '" + arg + "'";
}
run_module_code += "]\n";
run_module_code += "runpy.run_module('" + config.python_module + "', run_name='__main__', alter_sys=True)\n";
"runpy.run_module('" + config.python_module + "', run_name='__main__', alter_sys=True)\n";
int result = PyRun_SimpleString(run_module_code.c_str());
McRFPy_API::api_shutdown();
delete engine;
return result;
}
else if (!config.script_path.empty()) {
// Execute script file
// Execute script file (sys.argv already set at init time)
FILE* fp = fopen(config.script_path.string().c_str(), "r");
if (!fp) {
std::cerr << "mcrogueface: can't open file '" << config.script_path << "': ";
std::cerr << "[Errno " << errno << "] " << strerror(errno) << std::endl;
return 1;
}
// Set up sys.argv
wchar_t** python_argv = new wchar_t*[config.script_args.size() + 1];
python_argv[0] = Py_DecodeLocale(config.script_path.string().c_str(), nullptr);
for (size_t i = 0; i < config.script_args.size(); i++) {
python_argv[i + 1] = Py_DecodeLocale(config.script_args[i].c_str(), nullptr);
}
PySys_SetArgvEx(config.script_args.size() + 1, python_argv, 0);
int result = PyRun_SimpleFile(fp, config.script_path.string().c_str());
fclose(fp);
// Clean up
for (size_t i = 0; i <= config.script_args.size(); i++) {
PyMem_RawFree(python_argv[i]);
}
delete[] python_argv;
if (config.interactive_mode) {
// Even if script had SystemExit, continue to interactive mode
if (result != 0) {
@ -197,22 +176,18 @@ int run_python_interpreter(const McRogueFaceConfig& config, int argc, char* argv
}
else if (config.interactive_mode) {
// Interactive Python interpreter (only if explicitly requested with -i)
Py_InspectFlag = 1;
// Note: pyconfig.inspect is set at init time based on config.interactive_mode
PyRun_InteractiveLoop(stdin, "<stdin>");
McRFPy_API::api_shutdown();
delete engine;
return 0;
}
else if (!config.exec_scripts.empty()) {
// With --exec, run the game engine after scripts execute
// In headless mode, auto-exit when no timers remain
McRogueFaceConfig mutable_config = config;
if (mutable_config.headless) {
mutable_config.auto_exit_after_exec = true;
// With --exec, scripts were already executed by the first GameEngine constructor.
// Just configure auto-exit and run the existing engine to preserve timers/state.
if (config.headless) {
engine->setAutoExitAfterExec(true);
}
delete engine;
engine = new GameEngine(mutable_config);
McRFPy_API::game = engine;
engine->run();
McRFPy_API::api_shutdown();
delete engine;