fix(engine): headless step() is a full simulation frame; --exec can no longer hang
step() advanced animations and timers only. It never called updatePythonScenes(), which lives solely in doFrame() -- so headless Scene.update(dt) overrides never fired. That was the filed bug, but it was one of twelve omissions: the C++ scene update never ran, scene transitions never progressed or COMPLETED, and current_frame never advanced. Headless behaved measurably differently from a real frame, and any game with per-frame logic in update() was untestable. step() is now a full simulation frame: scene update, timers, Python scene update, animations, transitions (including completion), frame-time metrics, currentFrame++. Render and input stay out, deliberately -- rendering is orthogonal to the clock and costs zero simulation time (see renderScene(); a screenshot draws arbitrary state without time passing). Timers remain on simulation_time: explicit, deterministic time is the entire point of driving a headless test with step(). This exposed the deeper bug. testTimers() reads simulation_time when headless, and simulation_time is advanced ONLY by step() -- the doFrame run loop never touched it. So the headless run loop spun forever on a frozen clock: any headless script that registered a timer and did not call step() hung until killed. And a script that died during setup registered no timers, hit the auto-exit-when-no-timers path, exited 0, and was scored as a PASSING test. So headless --exec now exits when its scripts finish, nonzero if a script fell off the end without calling sys.exit(). step() is the clock; the engine cannot advance time on its own, and pretending otherwise is what produced the hang. A genuinely long-lived headless process (server, REPL host) opts in with the new --run-forever. run_tests.py additionally refuses to pass any test whose output contains a Traceback. Between the two, 82 tests that were being scored green while asserting nothing are now correctly red; they are repaired in the following commits. closes #350
This commit is contained in:
parent
c72ab22999
commit
30abb0b7ec
8 changed files with 142 additions and 4 deletions
File diff suppressed because one or more lines are too long
|
|
@ -86,6 +86,12 @@ CommandLineParser::ParseResult CommandLineParser::parse(McRogueFaceConfig& confi
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (arg == "--run-forever") {
|
||||||
|
config.run_forever = true;
|
||||||
|
current_arg++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
if (arg == "--audio-off") {
|
if (arg == "--audio-off") {
|
||||||
config.audio_enabled = false;
|
config.audio_enabled = false;
|
||||||
current_arg++;
|
current_arg++;
|
||||||
|
|
|
||||||
|
|
@ -32,6 +32,13 @@ struct McRogueFaceConfig {
|
||||||
// Auto-exit when no timers remain (for --headless --exec automation)
|
// Auto-exit when no timers remain (for --headless --exec automation)
|
||||||
bool auto_exit_after_exec = false;
|
bool auto_exit_after_exec = false;
|
||||||
|
|
||||||
|
// Keep running after --exec scripts finish, instead of exiting (#350).
|
||||||
|
// Headless --exec normally exits when the scripts are done: step() is the only
|
||||||
|
// headless clock, so the engine cannot advance timers on its own and would
|
||||||
|
// otherwise spin forever. Pass --run-forever for a long-lived headless process
|
||||||
|
// (a server, a REPL host) that drives itself.
|
||||||
|
bool run_forever = false;
|
||||||
|
|
||||||
// Exception handling: exit on first Python callback exception (default: true)
|
// Exception handling: exit on first Python callback exception (default: true)
|
||||||
// Use --continue-after-exceptions to disable
|
// Use --continue-after-exceptions to disable
|
||||||
bool exit_on_exception = true;
|
bool exit_on_exception = true;
|
||||||
|
|
|
||||||
32
src/main.cpp
32
src/main.cpp
|
|
@ -215,9 +215,37 @@ int run_python_interpreter(const McRogueFaceConfig& config)
|
||||||
else if (!config.exec_scripts.empty()) {
|
else if (!config.exec_scripts.empty()) {
|
||||||
// Execute startup scripts on the existing engine (not in constructor to prevent double-execution)
|
// Execute startup scripts on the existing engine (not in constructor to prevent double-execution)
|
||||||
engine->executeStartupScripts();
|
engine->executeStartupScripts();
|
||||||
if (config.headless) {
|
|
||||||
engine->setAutoExitAfterExec(true);
|
// A script that called sys.exit() has already decided the outcome.
|
||||||
|
if (McRFPy_API::shouldExit()) {
|
||||||
|
int code = McRFPy_API::exit_code.load();
|
||||||
|
McRFPy_API::api_shutdown();
|
||||||
|
delete engine;
|
||||||
|
return code;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #350: headless --exec that falls off the end without exiting is an error.
|
||||||
|
//
|
||||||
|
// step() is the only headless clock, so the run loop cannot advance timers on
|
||||||
|
// its own: entering it here used to spin forever with a frozen clock, and any
|
||||||
|
// headless script with a pending timer hung until it was killed. Worse, a
|
||||||
|
// script that died during setup registered no timers, auto-exited 0, and was
|
||||||
|
// scored as a passing test.
|
||||||
|
//
|
||||||
|
// An --exec script must state its outcome with sys.exit(). If a long-lived
|
||||||
|
// headless process is genuinely wanted (a server, a REPL host), say so with
|
||||||
|
// --run-forever.
|
||||||
|
if (config.headless && !config.run_forever) {
|
||||||
|
std::cerr << "mcrogueface: --exec scripts finished without calling sys.exit().\n"
|
||||||
|
<< " In headless mode the engine cannot advance time on its own: "
|
||||||
|
<< "mcrfpy.step() is the clock.\n"
|
||||||
|
<< " End the script with sys.exit(0), or pass --run-forever to keep "
|
||||||
|
<< "the process alive.\n";
|
||||||
|
McRFPy_API::api_shutdown();
|
||||||
|
delete engine;
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
|
||||||
engine->run();
|
engine->run();
|
||||||
McRFPy_API::api_shutdown();
|
McRFPy_API::api_shutdown();
|
||||||
delete engine;
|
delete engine;
|
||||||
|
|
|
||||||
88
tests/regression/issue_350_step_frame_parity_test.py
Normal file
88
tests/regression/issue_350_step_frame_parity_test.py
Normal file
|
|
@ -0,0 +1,88 @@
|
||||||
|
"""
|
||||||
|
Regression test for issue #350.
|
||||||
|
|
||||||
|
Headless mcrfpy.step() advanced animations and timers only. It never called
|
||||||
|
McRFPy_API::updatePythonScenes(), which lives solely in doFrame() -- so under step():
|
||||||
|
|
||||||
|
* Scene.update(dt) overrides NEVER fired (the filed bug)
|
||||||
|
* the C++ scene update never ran
|
||||||
|
* scene transitions never progressed or completed
|
||||||
|
* current_frame never advanced
|
||||||
|
|
||||||
|
Any game with per-frame logic in update() was untestable headless.
|
||||||
|
|
||||||
|
step() is now a full SIMULATION frame: everything doFrame() does except render and
|
||||||
|
input. Rendering stays deliberately off the clock -- it costs zero simulation time
|
||||||
|
(see issue_341 test), so step() must still render nothing.
|
||||||
|
"""
|
||||||
|
import mcrfpy
|
||||||
|
import sys
|
||||||
|
|
||||||
|
failures = []
|
||||||
|
|
||||||
|
|
||||||
|
def check(label, condition, detail=""):
|
||||||
|
if condition:
|
||||||
|
print(f" PASS: {label}")
|
||||||
|
else:
|
||||||
|
print(f" FAIL: {label} {detail}")
|
||||||
|
failures.append(label)
|
||||||
|
|
||||||
|
|
||||||
|
# ------------------------------------------------------- Scene.update under step()
|
||||||
|
print("1. Scene.update(dt) fires under step() (the filed bug)")
|
||||||
|
|
||||||
|
|
||||||
|
class CountingScene(mcrfpy.Scene):
|
||||||
|
def __init__(self, name):
|
||||||
|
super().__init__(name)
|
||||||
|
self.update_count = 0
|
||||||
|
self.dt_total = 0.0
|
||||||
|
|
||||||
|
def update(self, dt):
|
||||||
|
self.update_count += 1
|
||||||
|
self.dt_total += dt
|
||||||
|
|
||||||
|
|
||||||
|
scene = CountingScene("issue350")
|
||||||
|
mcrfpy.current_scene = scene
|
||||||
|
|
||||||
|
for _ in range(5):
|
||||||
|
mcrfpy.step(0.016)
|
||||||
|
|
||||||
|
check("update() fired once per step()", scene.update_count == 5,
|
||||||
|
f"got {scene.update_count}")
|
||||||
|
check("update() received the dt", abs(scene.dt_total - 5 * 0.016) < 1e-4,
|
||||||
|
f"got {scene.dt_total}")
|
||||||
|
|
||||||
|
# ------------------------------------------------------------ current_frame advances
|
||||||
|
print("2. current_frame advances under step()")
|
||||||
|
before = mcrfpy.get_metrics()["current_frame"]
|
||||||
|
for _ in range(3):
|
||||||
|
mcrfpy.step(0.016)
|
||||||
|
after = mcrfpy.get_metrics()["current_frame"]
|
||||||
|
check("current_frame advanced by 3", after - before == 3, f"{before} -> {after}")
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------- timers still fire
|
||||||
|
print("3. Timers still fire under step() (unchanged behavior)")
|
||||||
|
fired = []
|
||||||
|
mcrfpy.Timer("t350", lambda timer, rt: fired.append(rt), 50)
|
||||||
|
for _ in range(4):
|
||||||
|
mcrfpy.step(0.02)
|
||||||
|
check("timer fired", len(fired) > 0, f"got {len(fired)} fires")
|
||||||
|
|
||||||
|
# ----------------------------------------------------------- step() must not render
|
||||||
|
print("4. step() still renders nothing (render is off the clock)")
|
||||||
|
m = mcrfpy.get_metrics()
|
||||||
|
check("draw_calls == 0 after step()-only", m["draw_calls"] == 0,
|
||||||
|
f"got {m['draw_calls']}")
|
||||||
|
|
||||||
|
print()
|
||||||
|
if failures:
|
||||||
|
print(f"FAIL - {len(failures)} check(s) failed:")
|
||||||
|
for f in failures:
|
||||||
|
print(f" - {f}")
|
||||||
|
sys.exit(1)
|
||||||
|
|
||||||
|
print("PASS")
|
||||||
|
sys.exit(0)
|
||||||
|
|
@ -113,6 +113,13 @@ def run_test(test_path, verbose=False, timeout=DEFAULT_TIMEOUT,
|
||||||
passed = result.returncode == 0
|
passed = result.returncode == 0
|
||||||
output = result.stdout + result.stderr
|
output = result.stdout + result.stderr
|
||||||
|
|
||||||
|
# An uncaught exception is never a pass. Exit code alone was not enough:
|
||||||
|
# a script that raised during setup registered no timers, was auto-exited 0
|
||||||
|
# by the engine, and was scored PASS -- 59 tests sat rotted this way, their
|
||||||
|
# assertions never once executing. (#341/#350/#372)
|
||||||
|
if 'Traceback (most recent call last)' in output:
|
||||||
|
passed = False
|
||||||
|
|
||||||
# Check for PASS/FAIL in output
|
# Check for PASS/FAIL in output
|
||||||
if 'FAIL' in output and 'PASS' not in output.split('FAIL')[-1]:
|
if 'FAIL' in output and 'PASS' not in output.split('FAIL')[-1]:
|
||||||
passed = False
|
passed = False
|
||||||
|
|
|
||||||
|
|
@ -679,3 +679,4 @@ if __name__ == "__main__":
|
||||||
generate_html_docs()
|
generate_html_docs()
|
||||||
generate_markdown_docs()
|
generate_markdown_docs()
|
||||||
print("Documentation generation complete!")
|
print("Documentation generation complete!")
|
||||||
|
sys.exit(0) # #350: headless --exec must declare its outcome
|
||||||
|
|
@ -537,3 +537,4 @@ def main():
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
main()
|
main()
|
||||||
|
sys.exit(0) # #350: headless --exec must declare its outcome
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue