Add a command line interface - #386
Open
playday3008 wants to merge 45 commits into
Open
Conversation
The installation is probed for "gta5.exe" and "gta5_enhanced.exe", but the files Rockstar ships are named GTA5.exe and GTA5_Enhanced.exe. On Windows that difference does not exist; anywhere else the probe fails and the folder is rejected as not being a GTA V installation, so the key loader, the folder picker and the ModManager settings all refuse a perfectly good install. The same call sites built their paths by concatenating a literal backslash, which is not a separator off Windows either. They use Path.Combine. Two more in CodeWalker.Core did the same. RpfFile.CreateNew decided whether a path was already absolute by looking for a colon, so an absolute output path was treated as relative and 'pack -o /a/b/out.rpf' wrote a file named 'b\out.rpf' inside /a while reporting success. ExtractScripts joined its output paths the same way, and the Gen9Converter folder walk appended a backslash to normalize a folder. Path is an instance property on RpfFile, so System.IO.Path is spelled out at those call sites. Behaviour is unchanged on Windows.
CodeWalker is a WinForms application, so everything it knows about RPF archives is only reachable through a GUI on Windows. CodeWalker.Core already builds on its own; this is a console front end over it. Three commands to start: list, extract and hash. Every one of them takes --rpf and --exe, since reading an archive needs the encryption keys out of the installation, and every one can emit JSON instead of text so the output is scriptable. Targets net48, net8.0 and net10.0. net48 is what the rest of the solution builds against, and the modern targets are what this is actually meant to run on. The three handlers each opened the archive, loaded the keys, walked the entries and reported errors their own way. RpfService and RpfCommandOptions hold that once, so a handler is left with the part that is specific to it.
tree prints the archive as a directory tree, gen9 converts an archive to the Enhanced resource layout, pack builds an archive from a folder, and diff reports what changed between two archives. diff takes --si so the sizes it prints can use SI units like the rest, pack takes --force to overwrite an existing archive, and extract takes --no-overwrite to leave existing output alone. hash's JSON reports the signed and unsigned forms of each hash rather than only the unsigned one, since both spellings turn up in game files.
Every handler wrote out its own error result record and its own "print the message, pick an exit code" block, all of them the same except for the result type. The result records share a BaseResult, and RpfService.ReportError does the printing, so a failure path is one call. The glob translation only understood * and ?, and mapped * to '.*', so '*.ytd' matched across directory separators and there was no way to say "any depth". * and ? stop at a separator now, ** crosses them, and '**/' matches zero or more directory segments. extract counted a file it had skipped under --no-overwrite as extracted.
export turns game files into formats other tools can read: xml for metadata, textures for .ytd dictionaries as DDS, audio for .awc streams, and text for .gxt2 localization. Each subcommand takes an alias, so 'export t' and 'export ytd' both reach textures. The four subcommands differ only in how they turn one entry into output files. Everything around that -- validating the arguments, opening the archive, collecting the entries a filter selects, running them across threads, aggregating the results and reporting them -- is ExportService, and a subcommand supplies a delegate. CommonOptions holds the options that are not specific to reading an archive, so the option records stop repeating each other.
--threads accepted zero and negative values and then passed them to MaxDegreeOfParallelism, where anything below -1 throws. It is validated at parse time now, so the error names the option instead of surfacing as an exception from inside the loop. export ignored --no-overwrite: the flag was defined but never reached the code that writes the output files, so it overwrote regardless. Each exporter takes it and skips the files that already exist. The text exporter dereferenced the parsed .gxt2 without checking that it parsed, so a file it could not read took the process down instead of being reported as unsupported.
stat summarises an archive: file counts, total size, compression ratio and a breakdown per extension. search finds entries by path. validate walks every entry and reports the ones that fail to parse. inspect prints what is known about a single entry, including the resource version and the system and graphics page sizes for resources. That is the set the GUI offers for looking at an archive without opening one of its files.
The project had the analyzer packages referenced but AnalysisLevel at 'latest', which leaves most of the rules off, and no .editorconfig of its own, so it inherited whatever the solution root happened to say. This project now carries its own .editorconfig: latest-recommended, EnforceCodeStyleInBuild so the style rules fail the build rather than decorating the editor, and the naming, ordering and expression-style rules raised from suggestion to warning. CA1308 is the one that goes the other way, since lowercasing entry names is exactly what the RPF format wants. Clearing what that flagged is the rest of the diff: records sealed, accessibility modifiers spelled out, collection and object initializers, null propagation, compound assignment, and documentation comments generated so CS1591 has something to check.
The JSON contract says success is false when errorMessages is non-empty, and the exit code is 1 in the same case. Neither held. - diff set Success to a literal true, so a failed comparison still reported success. - export, extract, gen9, pack and validate derived Success and the exit code from their own error counters, which did not include the scan errors collected while opening the archive. An archive that failed to scan reported success and exited 0. - inspect and search returned 0 unconditionally. The counts were as inconsistent. export reported GrandTotalFileCount as totalFiles, which counts nested archives as files; a dry run counted nothing as exported because the status is "dry_run" and only "exported" was matched; export marked a format it does not handle as "skipped", which is also what --no-overwrite reports, so the two could not be told apart, and it is "unsupported" now. The rest are smaller: pack never set RpfManager.IsGen9, so --gen9 did nothing there; diff's JSON was missing the formatted sizes its text output printed; the progress bar could compute a fill wider than the bar; inspect built its per-type details with untyped casts; the exporters created the output directory before knowing whether the file would produce anything, so an archive of unsupported files left a tree of empty directories; pack accepted --threads without using it.
Debugging the CLI from VS Code needed the launch configuration written by hand every time, since the solution's own configurations all point at the WinForms projects.
Nothing was tested. CodeWalker.Cli.Tests builds the same sources with TESTING defined and xunit v3 on top, so the internals are reachable without making them public. Coverage starts with the pieces that are pure functions and easy to get wrong: the glob translation, the size formatting in both unit systems, the progress bar's rendering and throttling, the option parsing, and the net48 string polyfills, which are the only place where the three target frameworks can disagree about behaviour. The polyfill fuzz tests compare each polyfill against the framework method it stands in for. The polyfills were compiled out on anything newer than netstandard2.1, which meant net8.0 and net10.0 could not exercise them at all; the guard excludes netcoreapp2.1 and up and admits them under TESTING. The Json records are excluded from coverage. They are property bags with no branches.
No behaviour change. this. on instance members, predefined type names, parentheses in binary expressions and discards for unused results were all at silent or suggestion, so nothing enforced them and the tree drifted. They are suggestions or warnings now, IDE0130 included, and the reformat is what that produced.
The polyfills were written against APIs the framework they stand in for does not have. string.IndexOf(char, StringComparison) arrived in .NET Core 2.1; on net48 that call binds to IndexOf(char, int) and does not compile. CompareInfo.IndexOf with spans and an out match length is newer still. Contains(char) converts to a string before searching. ReplaceCore takes strings and recovers the match length through a helper, since the overload that reports it is not available here. The char overloads of StartsWith and EndsWith that took a StringComparison are gone: a comparison mode says nothing about a single character matched at one position, and nothing called them.
Hand-written accumulate loops over rpf.AllEntries appeared in most handlers, each rebuilding the same filter and the same running totals. They are LINQ now, which is shorter and states what is being counted. Ctrl+C was not handled at all, so interrupting a long walk over a large archive killed the process wherever it happened to be, including part way through writing an output file. Program installs a CancellationTokenSource on CancelKeyPress and every command takes the token; the collection loops and the Parallel.For options observe it. inspect's per-type details were built as an untyped object, so the JSON shape depended on which branch ran and nothing checked the field names. They are records now. The audio exporter treats a stream whose hash is 0 as metadata with no playable data and skips it. That was unexplained; there is a comment saying why. Copying a file over an existing one deletes and re-copies, which is two operations and a window where neither exists; File.Copy overwrites in one.
diff, export, extract, gen9, hash, inspect, list, pack, search, stat, tree and validate each get a test file. The archives are built in the test itself, so nothing depends on a GTA V installation being present.
Building net48 outside Windows needs the reference assemblies as a package, since there is no .NET Framework install to reference. Without it the whole project could only be built on Windows, and the net48 target is exactly the one most likely to break. The tests still only run under Mono on Linux, but they do run.
Every handler test called Execute with CancellationToken.None, so a test that hung had nothing to stop it and the runner's own timeout was the only backstop. They pass TestContext.Current.CancellationToken. The polyfill fuzz tests gain the edge cases the framework methods special-case: empty needles, empty haystacks, and a needle longer than what it is searched in.
Twelve handlers, their Json records and their tests were all in the project root, so the file list was two dozen entries deep before reaching anything structural. Handlers/ and Tests/Handlers/ mirror each other.
Both projects in this directory declared the same WarningsAsErrors property. Directory.Build.props already holds everything else they share.
ProgressBar wrote the bar, the stats and the padding as separate calls, so a redraw could interleave with another thread's output and a long file name could wrap the line. The line is built as one string now, clamped to the terminal width, and written once. Throttling moved from DateTime.Now to a Stopwatch, which does not jump when the system clock does. Update is private, Increment is the only way in and it stops at the total, and Dispose is idempotent and takes the same lock as the render. CollectFiles and the non-RPF count expressed the same predicate two different ways; they share one now. The export aggregation counted a file as both errored and skipped, and took its exit code from the error count rather than from the messages it was about to print, so a scan error could be reported without changing the exit code. The ThrowIfCancellationRequested at the top of each Parallel.For body is redundant: ParallelOptions.CancellationToken is already checked before every iteration. Tests cover ProgressBar, SizeFormat, Filter and both services.
Both handlers computed and printed in one pass, so nothing could be tested without capturing console output and parsing it back. Each is a Collect step returning the JSON result record plus one Print step per output format, and the tests call those directly. hash's encoding switch became ParseEncoding, which throws on an unknown name instead of returning an error result from the middle of Execute. The 'utf8' spelling that --encoding never accepted is gone; the default is the 'utf-8' the option documents. stat's JSON gained the *Formatted counterparts the other commands already emitted alongside their raw sizes: compressedSize, uncompressedSize, and the per-extension avg, min and max.
CollectFiles dropped every entry whose name ends in .rpf regardless of --recursive. That is right while recursing, since the archive's contents are walked in its place, but without it the nested archives simply vanished from the listing and there was no way to see that they exist.
hash parsed --encoding before entering the try block, so an unrecognised name escaped Execute as an unhandled ArgumentException instead of the error result and exit code 1 that every other failure produces. stat declared its scanErrors list inside the try block, so the catch could not reach it: an archive that logged scan errors and then threw reported the exception alone and dropped everything found on the way.
An entry stores 0 in FileSize when it is not compressed, and its real size comes from FileUncompressedSize or from the system and graphics page sizes. stat's total already went through GetFileSize, which handles that, but compressedSize read FileSize directly, so every uncompressed entry contributed nothing to it and the compression ratio came out too low. The entry type test is a switch now, and an entry that is neither a resource nor a binary throws instead of being dropped from both counts. The extension table gains its outer border, so the columns close against the dividers that were already between them. Trailing commas in the initializers of these two files are gone; the rest of the project does not use them.
CollectStats, PrintStats and PrintJsonStats had no tests of their own. These cover the extension grouping, the compression ratio, the size formatting in both unit systems, and the error paths.
Same split as hash and stat: a Collect step that returns the JSON result record, and a Print step per output format. Every handler declared its scanErrors list inside the try block, so the catch could not reach it and an archive that logged scan errors before throwing reported only the exception. The list is declared before the try in extract, inspect, list, stat and validate, and the catch passes it on. list no longer takes --threads. It never spawned any. tree's JSON nodes carry the resource version, which the human-readable output was already printing.
The matcher tried to guess whether a pattern was a glob and silently switched behaviour on the answer, so '*' in a file name and '*' as a wildcard could not be told apart. Matching is a plain case-insensitive substring test now, and the JSON says so. --dir searches every archive below a directory, which is what looking for a file across an installation actually needs. search marks --rpf optional and validates that exactly one of the two is given. The result carries rpfFiles, the archives that were searched, and each match names the archive it came from. nameHash and shortNameHash are gone from the match records. They were the Jenkins hashes of the entry name, which the hash command already prints, and nothing here consumed them.
diff compared two archives through one CommonOptions, so both sides shared a single --exe and a single --gen9. Comparing a legacy installation against an Enhanced one, which is the reason to run it, was not expressible. The options record is flat and names each side: --left-exe, --right-exe, --left-gen9, --right-gen9. --progress is accepted here too, and the handler is split into CollectDiff and the two Print steps like the others.
RpfCommandOptions, CommonCommandOptions and ExportCommandOptions each wrapped a group of options in a type that handlers had to compose, so the options records nested a level deep and every read went through another hop: options.Rpf.Recursive rather than options.Recursive. CliOptions replaces all three with one factory method per option. A handler lists the options it accepts, and its options record is flat. RpfService and ExportService move under Helpers/ as RpfHelper and ExportPipeline, and the BaseResult and ReportError pair they had accumulated moves out to its own Output.cs.
--gen9 only selected the encryption keys. Resource readers consult the process-wide RpfManager.IsGen9 to pick the memory layout, and nothing set it, so every resource file parsed as legacy against an Enhanced install: export textures threw on every .ytd, validate reported nothing valid, and inspect printed garbage dimensions. Setting it in LoadKeys covers every command, since they all reach it through ValidateAndLoadKeys or ValidateExeAndLoadKeys. The pack and gen9 handlers keep their own save/restore around the flag: tests run several handlers in one process and must not leak it between them.
Keys are process-wide, so loading the left installation's keys and then the right's before opening either archive left both sides being read with the right side's keys. --left-exe and --right-exe could not both be honoured, and neither could --left-gen9 and --right-gen9. Each side is now scanned while its own keys are loaded. Metadata is collected first, which identifies the entries that share a path, size and type and therefore still need a content comparison; only those are extracted and hashed, so the amount of data read is unchanged. Entries are also keyed relative to the archive root instead of by their full path. The archive's own file name is the first path component, so comparing two archives with different file names previously reported every entry as both added and removed.
Both commands reported every file in the archive as totalFiles and folded the filter's exclusions into the skipped count, so extracting a single file out of common.rpf read as '1 extracted, 664 skipped' out of 665. list and stat already counted what the filter selected. totalFiles is now the selected set and skipped counts only real skips: an existing output under --no-overwrite, or a format the exporter does not handle. RpfHelper.CountNonRpfFiles existed only to produce the old archive-wide number and is gone.
COMMANDS.md had drifted well away from the code: it described diff with a single --exe, listed --threads on commands that never had it, omitted search's --dir, and its sample output and hashes were invented. Rather than resynchronise a file that will drift again, the parts --help could not already express now live in the help output itself. Every command's help gains its --json field list and the exit codes, and the root gains the stdout/stderr split. System.CommandLine 2.0.2 keeps HelpBuilder and HelpContext internal, so CustomizeLayout is unreachable and HelpLayout replaces the help option's action instead. Two things the old document described correctly and the code did not: - hash reported its encoding as 'UTF8', which --encoding rejects. It now reports 'utf-8' and 'ascii', the spellings that parse. - search's patternType was pinned to 'substring' after glob detection was removed, so the JSON field and the '(substring)' suffix said nothing. --progress was defined four separate times with three different descriptions; it comes from CliOptions now, as --dry-run and --output already did. --output's default is expressed relatively so help shows '.' instead of whichever directory help happened to be run from.
No behaviour change. HashHandler and StatHandler carried full param/returns/exception blocks that restated their signatures, while the other fourteen files used a one-line summary or nothing. They now match. The same goes for the param tags on Filter and SizeFormat; ProgressBar and ExportPipeline keep theirs, where the text describes contract details the names do not. Comments that repeated the statement below them are gone, along with the decorative rules separating test sections. The box-drawing characters in TreeHandlerTests stay: they quote the glyphs the command actually emits. The cancellation rethrow is written the same way in every handler now, without the comment that was copied alongside it five times. csharp_style_var_* was already set to prefer explicit types but only at suggestion level, so nothing enforced it. Raised to warning; no code changes needed, the tree already complies.
Directory.Build.props redirects build output to bin/$(MSBuildProjectName) so the two projects sharing this directory do not collide, but the VS Code launch configurations still pointed at bin/Debug, so none of them could start the CLI. IndexRange 1.1.0 brings Microsoft.Bcl.Memory 9.0.0 in on net48, which carries GHSA-73j8-2gch-69rq; pinned to the patched 10.0.4. The build is warning-free again.
Sizes, the compression ratio and vector components were formatted with the ambient culture, so a machine with a comma decimal separator emitted '18,28 MiB' where another emitted '18.28 MiB'. Those strings are not only printed: they are the *Formatted fields in --json, which is documented for scripting, so the same archive described itself differently depending on where the tool ran.
RPF entry paths are separated with backslashes on every platform, but the export pipeline handed the raw path to Path.GetDirectoryName before translating them. On a platform whose separator is not a backslash the whole path reads as a bare file name, so the directory came back empty and every exported file landed in the output root. Entries sharing a name then overwrote each other silently: exporting the localization files out of x64b.rpf reported 6804 written and left 567 on disk, one per name rather than one per language. Extract already translated the separators before splitting the path; export now does the same, so both mirror the archive.
The project sets end_of_line = crlf and EnforceCodeStyleInBuild makes dotnet format check the file as it sits on disk, but the repository root normalizes with '* text=auto', so a non-Windows checkout writes LF and every file reports ENDOFLINE. The two could never agree off Windows. A .gitattributes scoped to this project forces crlf on checkout everywhere. Blobs are still stored normalized, and the root file is left alone so nothing conflicts with upstream.
Nothing ran the build or the 534 tests automatically; the repository had a .github directory with only FUNDING.yml in it. Windows covers all three target frameworks, including net48 natively. Linux is not redundant coverage: the archive-name path join in pack, and the export pipeline collapsing its output into one directory, both only reproduced off Windows, so a Linux leg is what would have caught them. Scoped by path to CodeWalker.Cli and CodeWalker.Core, since the rest of the solution is Windows-only WinForms and shader projects that do not build in a plain runner. The build passes -warnaserror. The project is warning-free today, so this is a regression gate; note it also promotes NuGet advisory warnings, which can appear on a branch that has not changed.
ExtractFileResource skips the 16 byte header, and when DecompressBytes
returns null it hands back the still-compressed bytes and subtracts
those 16 from entry.FileSize to match. The entry is shared, so that is
permanent: each extraction of the same file reads 16 bytes less than the
one before.
pass 0: entry.FileSize=73437 extracted=73421
pass 1: entry.FileSize=73405 extracted=73405
pass 2: entry.FileSize=73389 extracted=73389
It is also a data race: the CLI extracts across sixteen threads, and
nothing guards the field.
The data is short either way, and the caller has to deal with that.
ResourceDataReader built its two MemoryStreams straight from the page
flags, so a buffer smaller than they describe surfaced as an
ArgumentException about offset and length with nothing to say which file
it was. It checks first, and names the file and both sizes.
Found on one entry in x64a.rpf of a GTA V Enhanced install whose deflate
stream decodes at no offset. The other 24002 resources checked across
five archives decompress to exactly their flagged size, so this is one
bad file rather than a format CodeWalker cannot read.
Not covered by a test: reaching this path needs a real archive holding a
deliberately corrupt resource, and there is no test project over
CodeWalker.Core to build one in.
The xunit runner writes its failures to a file under TestResults and prints only the counts, so a failed run said "Failed: 1, Passed: 533" and named neither the test nor the assertion. The log is dumped on failure. The loop also ran under -e, so the first framework to fail skipped the rest. It runs all of them and fails afterwards, which matters here because net48 is first and the two modern targets never got a chance to report.
OutputDirectory_ComputedFromBackslashPath asserted that the directory an
export lands in contains no backslash. That only holds where the
platform separator is not a backslash, so it failed on Windows once the
translation it was named for actually worked:
Assert.DoesNotContain() Failure: Sub-string found
String: "/out\x64\levels\gta5\vehicles.rpf"
Found: "\"
It never tested what it claimed either. It was written while the
separators were not translated at all, and back then the directory came
back as the bare output root, which contains no backslash and starts
with /out, so both of its assertions passed against the bug.
BackslashEntryPath_MirrorsArchiveStructureInOutputDir covers the same
path and compares against Path.Combine of the expected segments, which
holds on either separator.
The existing jobs build CodeWalker.Cli and nothing else, so a change to CodeWalker.Core that breaks the WinForms projects consuming it would pass. This branch already carries such a change, and none of it had been compiled anywhere. msbuild rather than dotnet build, because the solution includes a C++ project for the shaders. Debug rather than Release: CodeWalker.csproj sets PlatformTarget to x64 only under Release|AnyCPU, and the SDK then infers a win-x64 RuntimeIdentifier that restore has not produced, which fails as NETSDK1047. One invocation with -restore keeps the restore and the build on the same evaluation. No -warnaserror here. This is a compile gate over code the CLI work does not own; the warning gate stays on the project this branch is for. The path filters gain the projects the job builds, so a change confined to the GUI still runs it.
Nothing was kept from a run, so trying a build meant compiling it. The CLI publishes three ways: self-contained single-file for win-x64 and linux-x64 from net10.0, which run with nothing installed, and a net48 build for Windows machines that already have the framework. No trimming, since the JSON output goes through reflection. The solution job uploads the Release output of all seven apps, one folder per project because each carries its own copy of the shared assemblies. It built Debug until now because Release failed with NETSDK1047: CodeWalker.csproj sets PlatformTarget to x64 under Release|AnyCPU and the SDK then infers a win-x64 RuntimeIdentifier that restore has not produced. Clearing RuntimeIdentifier keeps the AnyCPU assets, so the artifacts are Release builds and no project file changes.
The requirements list was the app's, with nothing saying so, and it had drifted: .NET Framework 4.5 where the projects target 4.8, behind a link to the 4.7.1 download, and no mention of Enhanced although the tree has Gen9 support. Each list now names the program it is for, and the command line program's drops the graphics and memory figures that do not apply. CodeWalker.Cli was not mentioned anywhere. It gets a section next to the menu and explorer modes, since all three are ways of using this without the world view.
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
1 participant
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.
Adds
CodeWalker.Cli, a console front end overCodeWalker.Core, so archivescan be worked with from a script or a machine without a desktop.
Twelve commands: extract, list, hash, tree, gen9, pack, diff, export, stat,
search, validate, inspect. Each takes
--jsonand writes one object tostdout;
--helpcarries the JSON fields and exit codes per command.Targets net48, net8.0 and net10.0. It has its own
.editorconfigandanalyzer settings and builds warning-free under
-warnaserror. Nothingoutside the new project changes style or tooling.
Shared code needed four fixes, all of which affect the GUI as well:
probed for as
gta5.exewhen the shipped file isGTA5.exe, so acase-sensitive filesystem rejects a valid install.
RpfFile.CreateNewlooked for a colon to decide a path was absolute.ExtractFileResourcepermanently subtracted 16 fromentry.FileSizewhenever a resource failed to decompress, so repeated extraction of the
same file returned less each time.
ResourceDataReaderthrew anArgumentExceptionnaming no file when theextracted buffer was shorter than the page flags require.
533 tests, none of which need a GTA V install. CI covers all three
frameworks on Windows, net8.0 and net10.0 on Linux, formatting, and a full
solution build so a Core change cannot quietly break the WinForms projects.
Also run against a real Enhanced install. The legacy, non-Gen9 paths are
untouched by this branch but untested: no legacy install available here.