Repository navigation
feat(tools): preview controller timelines without queuing - #11816
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed head b536d34 against current main. The preview passes only the document and timing options to compile_timeline with a local placeholder sequence; it has no command-file path or queue_timeline call. Its text and JSON projections omit the queue header and sequence, and duration is taken from the final event so the trailing inter-repeat gap is excluded. I extracted the exact PR blobs and ran the existing timeline suite plus the new preview suite under Python 3.11: 22 tests passed, 0 failures. The three-way merge tree with current main is clean, and GitHub path searches found no open PR editing these preview/compiler files. Hosted relinker/Windows checks are still pending; Linux product jobs are skipped, which is consistent with this host-tool-only change. The author-reported Windows/WSL coverage is documented in the PR, not independently rerun here.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
left a comment
There was a problem hiding this comment.
I rechecked the base after my earlier content review. The preview and test files are unchanged, and I reran the existing timeline suite plus the exact preview suite against current main's pad_input.py/pad_timeline.py: all 22 tests passed on Python 3.11. Current conventions, intake, eligibility, and labels pass; Windows product/relinker and Ubuntu/Windows relinker jobs passed, while macOS relinker jobs remain pending.
GitHub still reports base 5d52b06ff3de0b2ce640dd296db465295c80e7fa, whereas current upstream main is 801d5b9bc11049e6274daca09d70690bd5b8d0a9. CONTRIBUTING asks branches to stay current with main. Please update the branch and rerun conventions; I am changing my earlier approval to request that policy fix before merge. I found no new issue in the preview implementation.
b536d34 to
1f563f2
Compare
|
@MarcZX10 Rebased this branch onto upstream main The feature implementation and regression-test file are unchanged from the reviewed head. I reran the two relevant registered CTest suites on this exact rebased head on Windows and WSL: 22 unittest cases per platform, zero failures. Windows has one existing symlink-privilege skip; Linux has none. Both native relinker-only builds/help, current-main conventions, whitespace, registration and XML skip audits pass. All related groups remain below 30 seconds. The PR description now records the new base/head and verification results; earlier combined/projection runs are not presented as tests of this head. New hosted CI has been triggered. Please re-review the updated branch and update the policy-based changes-requested review if this resolves the outstanding request. AI-assisted: yes. |
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed 1f563f246edd666b636b658b36ba204277fe150d — approve. The preview command delegates validation and compilation to the existing timeline compiler and emits scheduled events without opening a controller command file.
Windows/Python 3.11: head 4 passed (4 total). This is a new command; there is no prior implementation for a behavioral regression control. The cases cover valid previews, repeated timelines, invalid input and CLI output. No controller or title was driven.
The inspected hosted Windows relinker job merged this exact head and completed successfully. This is not a full product or game build.
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed — approved
The preview reuses the production timeline compiler and exposes only event data; the CLI tests confirm that it does not create or modify controller command files.
What I checked
- Reviewed exact head
1f563f246edd666b636b658b36ba204277fe150dagainst livemain10bec5c322eb581b3e959e6c7e18bcf09e068e96. preview()delegates validation and event timing topad_timeline.compile_timeline, then removes the private placeholder sequence/header from its output. Text output does not emit the queue header; JSON has no sequence field.- Applied the PR diff to a detached worktree at live
mainand ran both the new preview suite and the existing timeline suite. This also exercised the main-branch duplicate-button validation added since the PR base. - Ran the repository overlap checker. It could not compare six older PRs because their
CMakeLists.txtbranches conflict with main; I inspected those additions (#674, #1835, #3786, #5021, #9508, #9539). They add separate tests or options; none duplicates or edits thepreview_pad_timelineregistration. - Current-main merge preview is clean; conventions and diff checks pass.
Results
| Check | Result |
|---|---|
| Current-main merge preview | Clean (eed741b6bf68ab4d1203d015a516aaaa76ee164b) |
| PR-head preview tests | 4/4 passed |
| Live-main + PR preview tests | 4/4 passed |
| Live-main timeline tests | 21/21 passed |
| Conventions | Pass |
| Diff check | Pass; no output |
| Hosted checks | Windows, all relinker platforms, ASan, conventions, labels, eligibility, and report pass; product Linux and specialized jobs skipped |
Raw test output
# Exact PR head
Ran 4 tests in 0.236s
OK
# PR diff applied to live main
Ran 4 tests in 0.174s
OK
Ran 21 tests in 0.363s
OK
All conventions checks passed.
Negative control
No separate mutation control was reported or run. The suite does directly check that preview leaves existing command-file bytes, modification time, and directory contents unchanged.
Findings
Blocking
None found in the preview logic, side-effect boundary, or current-main integration.
Non-blocking
I did not run the native relinker build or the full CTest matrix locally. The hosted relinker jobs pass. The overlap tool's six uncomparable CMake warnings were manually checked as described above.
Sources
- Preview implementation at reviewed head
- Preview tests at reviewed head
- Timeline compiler at live main
- Timeline regression tests at live main
- Live main
Not verified
I did not run Windows, macOS, controller hardware, a game, or the full CTest matrix locally. The 25 Python tests above ran on the live-main-plus-PR source tree in this Linux environment.
To get this to a good state
No further change is needed for the preview behavior reviewed here.
|
@StoneDC reviewed at 1f563f2: PASS. The new @adriadam10 exact-head CI is green (run 40565149664); we'll merge this in the next train unless you'd rather merge it yourself. |
|
Checked the preview against |
What
Add tools/preview_pad_timeline.py to inspect compiled controller events without queuing commands. Reuse compile_timeline with a private placeholder sequence, then expose only event timing/buttons, count and duration as text or JSON. Text deliberately has no APS5_PAD_TIMELINE queue header; JSON identifies itself as a preview and omits sequence fields.
Rebased onto upstream main
3fea2a937ba9fbab2f06c51691fe0c5763659aa3in1f563f246edd666b636b658b36ba204277fe150dto address the review's branch-currency request. Main is an ancestor of this head. The feature implementation and its regression-test file are byte-identical to the previously reviewed versions; no functionality was added during this update.Tested
Exact tested head:
1f563f246edd666b636b658b36ba204277fe150d. Base: upstream main3fea2a937ba9fbab2f06c51691fe0c5763659aa3.Windows: Python 3.14.3, MinGW GCC 15.2.0, CMake/CTest 4.3.1, Ninja. WSL Ubuntu: Python 3.14.4, GCC 15.2.0, CMake/CTest 4.2.3, Ninja.
Configured with
-G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_RELINKER_ONLY=ON; ranctest --test-dir <build> -R '^(pad_timeline|preview_pad_timeline)$' --parallel 4 -V --output-junit <report>.The four preview tests and 18 existing timeline tests exercise actual text/JSON CLI output, repeat/gap timing, invalid inputs, and unchanged command-file bytes/mtime and directory contents. Preview timing is checked against the real compiler with a different placeholder sequence. Windows skips one existing symlink test because creation privilege is unavailable; Linux executes it. That skip is not counted as an executed pass. No command is queued and no controller/game is exercised.
Native relinker-only builds and
relinker --helppass on both platforms. Current-main conventions, whitespace, test-script registration audit (203 scripts), XML skip-policy checks and Python 3.9 grammar parsing pass. Grammar parsing is not execution on an older Python runtime. Every related CTest group is below 30 seconds.The original feature and test blobs were compared with the previous PR head and are unchanged. The shared production helpers (
performance_compare.pyfor CSV,pad_input.py/pad_timeline.pyfor preview) are also unchanged between the original base and this main revision. Current-main CMake registration and the relevant suites were exercised again on this exact head. No prerequisite or ordering dependency is introduced.Earlier ten-PR combinations and related selected-file projections remain historical evidence at their recorded commits; they were not rerun or relabeled as final-head integration here. The original publication overlap scan was for the previous head; this update preserves the existing feature and registration patch while refreshing main.
New hosted CI is triggered for this head. Not run locally: native macOS, an actual older Python runtime, full system-library/product builds, browser/WASM, game/PS5 or GPU execution. Skipped or pending hosted jobs are not claimed as passes.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)