Fixed C64Script-based effect cycling#110
Merged
Merged
Conversation
- Implemented the ENV() built-in function to retrieve environment variables in the C64 script VM. - Added error handling for incorrect argument counts and types. - Created a new script to test the ENV() function, validating behavior for absent and present environment variables. - Introduced a new assertion class for tint transitions in video quality tests. - Added a new script for cycling effects between green and amber monitors. - Enhanced debug logging in various parts of the script VM for better traceability. - Updated end-to-end tests to include new assertions and scenarios.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an ENV() built-in to the C64Script VM so scripts can read OS environment variables, and updates script/E2E assets to validate effect-cycling behavior and improve debug visibility.
Changes:
- Implement
ENV(name)/ENV(name, default)in the VM and document it in the spec/grammar. - Add a new script-level test for
ENV()plus an NTSC E2E scenario/assertion to validate green↔amber tint cycling. - Improve debug logging for WAIT/EFFECT dispatch and adjust preset application behavior during source create/update.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/script/vm/c64-script-vm-dispatch-builtins.c |
Adds runtime implementation of ENV() builtin. |
src/script/vm/c64-script-bytecode.c |
Adds compiler-time builtin resolution for ENV() arities and adjusts builtin arg-count handling. |
src/script/c64-script.h |
Adds C64SCRIPT_BUILTIN_ENV enum value. |
doc/c64script/c64script-spec.md |
Documents ENV() behavior and examples. |
doc/c64script/c64script-grammar.ebnf |
Updates grammar documentation block to include ENV() signatures. |
tests/script/scripts/test_builtin_env.c64script |
Adds a script exercising ENV() with present/absent variables and defaulting. |
tests/script/scripts/test_builtin_env.expected-trace.yaml |
Adds expected trace output for the new ENV script test. |
data/scripts/effect_cycle_green_amber.c64script |
Adds an effect-cycling script controlled by an env var. |
tests/e2e/scenarios/ntsc_script_effect_cycle/scenario.yaml |
New E2E scenario running the effect-cycle script with a new assertion. |
tests/e2e/assertions/tint_transition.py |
New assertion to detect green↔amber tint transitions over time. |
tests/e2e/assertions/runner.py |
Registers tint_transition assertion key. |
tests/e2e/assertions/__init__.py |
Exports TintTransitionAssertion. |
src/script/vm/c64-script-vm-dispatch-effects.c |
Adds debug logging around queuing/applying EFFECT updates. |
src/script/vm/c64-script-vm-dispatch-core.c |
Adds debug logging around WAIT execution timing. |
src/c64-source.c |
Changes preset auto-apply logic to run unconditionally when crt_preset is set. |
tests/script/fuzz/fuzz.sh |
Comment/message punctuation normalization. |
tests/script/test_interact_key.c |
Comment punctuation normalization. |
data/keymaps/symbolic_uk.c64keymap.ini |
Comment punctuation normalization. |
data/keymaps/symbolic_nl.c64keymap.ini |
Comment punctuation normalization. |
data/keymaps/positional_us.c64keymap.ini |
Comment punctuation normalization. |
doc/testing/e2e.md |
Documentation punctuation normalization. |
doc/developer.md |
Documentation punctuation normalization. |
.github/workflows/build-project.yaml |
YAML quoting/punctuation normalization. |
.github/prompts/pr-converge.prompt.md |
Markdown heading punctuation normalization. |
- Updated `c64_stream_effects_create` and `c64_stream_effects_update` functions to always apply the CRT preset unconditionally, allowing subsequent EFFECT commands to override previous settings. - Introduced `EffectTransitionAssertion` to detect multiple distinct visual effect states across video recordings, utilizing a feature vector for analysis. - Added a new scenario for NTSC demo effect cycle, incorporating the effect transition assertion into the testing framework. - Modified existing tests to streamline environment variable checks and improve clarity.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement the ENV() function to access environment variables in the C64 script VM, including error handling for argument validation. Introduce tests to validate its behavior with both present and absent variables. Enhance debug logging and update end-to-end tests to incorporate new assertions and scenarios.