Repository navigation
Conversation
Confetti3
left a comment
There was a problem hiding this comment.
Thanks for this. Putting the support guides in the offline package and checking the package's links before the manifest is hashed is the right place for it, rewriting unshipped targets to a pinned source revision is a good choice, and the added test runs the real inventory. A few things to change, mainly so the rewriter keeps the document the packaging step chose and the link matcher stops guessing.
1. The rewriter picks its own source and overwrites the copied file. scripts/package-team.ps1:38-42 already chooses a source for each inventory document (staged install, build output, then the repository, with a topic-folder search for the flat docs entries). scripts/package-docs.py:36-54 then decides again from the package path (START_HERE.md and SECURITY.md by name, otherwise the repository file or a basename search under docs/) and writes source.read_text() over the copy at line 71. The two resolvers have different precedence and different ambiguity rules (the PowerShell one takes the first match, the script raises), so a document supplied by the stage or build directory is copied correctly and then silently replaced by the repository version, and the manifest hashes the replacement. The current stage does not ship any .md file, so nothing changes today, but the first time one does the content will differ without any error. Please resolve each document once, in package-team.ps1, and pass the script explicit records (package path, source file, whether it is the START_HERE alias). The script then rewrites the selected content, using that source's location for relative links, and keeps everything else as it is. A test where the staged and repository contents of one document differ would cover it.
2. The link matcher works on raw text, so validation shares its blind spots. LINK and REFERENCE (scripts/package-docs.py:12-13) run over the whole file. Some inputs behave wrongly:
[Help](guide(v2).md)gives the destinationguide(v2, because the pattern stops at the first).[](missing.md)yields onlyicon.png, socheck()passes whilemissing.mdis broken.- A link-shaped example inside a code span or a fenced block is treated as a real link, so a literal example can fail packaging or be rewritten.
None of the shipped guides contain these forms, so this is about the check staying reliable rather than a current failure. Please find destination spans with a Markdown-aware tokenizer (one that handles code spans and fences, balanced parentheses, nested image links and reference definitions with titles), apply both the relocation and the check to those spans, and add a case for each of the above to scripts/test-package-docs.py.
3. Python 3.10 is needed, but CMake accepts any Python 3. find_package(Python3 REQUIRED COMPONENTS Interpreter) (CMakeLists.txt:20) has no minimum version, and scripts/package-docs.py:71 calls Path.write_text(..., newline=...), which was added in 3.10 (is_relative_to on lines 63 and 82 needs 3.9). Please require Python3 3.10 there, and either state the same minimum for the python that scripts/package-team.ps1:79 calls or drop the newline argument.
4. Suggestion: widen the workflow's path filter. .github/workflows/package-documentation.yml runs on docs/** and the packaging files, but the test also reads .github/SECURITY.md, ATTRIBUTION.md and the assets/*/README.md files in the inventory. Adding those paths means a broken link in one of them is caught on its own PR.
Still needs a real package run through package-team.ps1 to confirm the shipped guides open and link correctly offline.
Extract this focused change from the locally tested development build. Keep other features out of the upstream review; the pull request records implementation details, prior integration evidence, and outstanding isolated validation.
87d70da to
aad01da
Compare
Ship the additional troubleshooting/support guides offline, and relocate Markdown destinations before the manifest is hashed. Rebased onto current release; all four review points are addressed.
Source selection and preservation
package-team.ps1uses one shared copy function for inventory files, the selected start page, and selection art. It records the actual selected source and package destination, with explicit repository aliases and a preferred START_HERE alias. The rewriter edits the copied package content in place and resolves links from the selected source's directory. It does not reread repository prose or rediscover documents by basename. Ambiguous repository matches, duplicate destinations, and unmapped packaged Markdown fail explicitly.Markdown parsing and validation
Both relocation and validation use destination spans from the pinned Marko 2.2.4 CommonMark parser. The source-preserving edits cover balanced/escaped parentheses, nested linked images, reference definitions and titles, blockquotes and lists. Inline code, fenced code and indented examples remain untouched. Packaged targets become relative links; existing unshipped repository targets become URLs pinned to the exact source commit. Broken local paths and escapes are rejected.
The parser is a build-only dependency in
scripts/requirements-docs.txt; it is not included in the game package. CMake and the packaging preflight require Python 3.10+. Packaging accepts an explicit Python executable and rejects an invalid override. The documentation workflow installs the pinned parser and also watches security guidance, attribution, assets, the copy helper and its requirements.Validation
PackageDocsCTest passed on Python 3.12.5 (33.63 seconds).git diff --checkpassed.The complete Discord-enabled
package-team.ps1release flow was not run: the required SDK archive is absent on this machine. The shared content-copy/relocation path was executed with real repository content; that is not a claim of a signed release or native gameplay acceptance. Validation checks local file targets, not heading anchors, raw HTML links or remote availability.Compatibility update (17 October 2026)
Aligned the documented and CMake Python minimum at 3.12 and moved the PackageDocs test registration away from other pending CMake additions. #70 now reuses this PR's source-aware document/package copier instead of introducing a competing assembler.
Checked all 16 open contribution PRs against
releaseatcd4daa5c2fb19ee5a3ecd14d0e2d9cbee753aa4f. All 120 PR pairs merge cleanly; three complete merge orders produce the same source tree. The four dependent PRs document the additional branch refresh required after a prerequisite squash merge. These checks cover the exact branch heads linked in this update, not future upstream edits.Validation of the combined tree:
relay_unreachable; its full rerun passed on unchanged code. Both logs are retained.The current pass did not run the full Discord SDK, Inno Setup installer integration, a physical-controller session inside SSFIV, or a real match between two PCs. No new release assets or private test package were published. Earlier feature-specific results above remain identified separately where applicable.