Conversation
…the lobby owner returns to the lobby
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdd a UI guard to suppress setter side-effects during programmatic updates and deterministically sort allowed difficulties; cache beatmap version lookups; inject DI into newly-added player cell custom data with error handling; fix level-hash lookup iteration; invert a level-unload guard; and add multiple PowerShell build/deploy/debug utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant ClientUI as Client UI
participant LocalLogic as Local Logic (MpPerPlayerUI)
participant Network as Network / SendToPlayer
participant Server as Server
ClientUI->>LocalLogic: user toggles option
LocalLogic->>Network: SendToPlayer(packet) activate
Network->>Server: packet delivered
Server->>Network: server state packet (owner)
Network->>LocalLogic: apply server packet
Note right of LocalLogic: with skipUpdateHandler set\nprogrammatic set_Value(...) doesn't call SendToPlayer
LocalLogic-->>ClientUI: update UI control values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/UI/MpPerPlayerUI.cpp (2)
172-175: Use an RAII guard instead of manualtrue/falsetoggling.The current pattern is fragile and easy to break in future edits. A scoped guard makes reset automatic and keeps these sections safe.
♻️ Proposed refactor (scoped guard)
namespace MultiplayerCore::UI { - // Setting toggle triggers setters, guard every set_Value with this to prevent unwanted state resets - bool skipUpdateHandler = false; + // Setting toggle triggers setters, guard every set_Value with this to prevent unwanted state resets + bool skipUpdateHandler = false; + + struct ScopedUpdateHandlerSkip { + bool& flag; + explicit ScopedUpdateHandlerSkip(bool& f) : flag(f) { flag = true; } + ~ScopedUpdateHandlerSkip() { flag = false; } + };- skipUpdateHandler = true; // set_Value triggers on update + ScopedUpdateHandlerSkip guard(skipUpdateHandler); // set_Value triggers on update ppdt->set_Value(false); ppmt->set_Value(false); - skipUpdateHandler = false;- skipUpdateHandler = true; + ScopedUpdateHandlerSkip guard(skipUpdateHandler); ppdt->set_Value(packet->PPDEnabled); ppmt->set_Value(packet->PPMEnabled); - skipUpdateHandler = false;Also applies to: 233-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UI/MpPerPlayerUI.cpp` around lines 172 - 175, Replace the manual toggling of skipUpdateHandler around the two set_Value calls with a scoped RAII guard that sets skipUpdateHandler = true on construction and resets it to false on destruction (e.g., implement a small guard class like SkipUpdateGuard). Update the code around ppdt->set_Value(false) and ppmt->set_Value(false) (and the identical block at the other location) to instantiate that guard at the start of the scope so the flag is automatically restored even on early returns/exceptions.
49-50: Scope the update guard to theMpPerPlayerUIinstance, not namespace state.Using a namespace-level mutable flag risks cross-instance interference if more than one
MpPerPlayerUIexists/re-initializes. This guard should be instance-owned (this->_skipUpdateHandler) to keep behavior isolated per UI controller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UI/MpPerPlayerUI.cpp` around lines 49 - 50, The namespace-level flag skipUpdateHandler should be converted to an instance member on MpPerPlayerUI to avoid cross-instance interference: add a private bool _skipUpdateHandler (initialized in MpPerPlayerUI constructor), remove the namespace variable, and update all guarded sites (where set_Value and other setters check skipUpdateHandler) to use this->_skipUpdateHandler instead; ensure any code that toggles the guard (set to true/false) is updated to operate on the instance member so each MpPerPlayerUI controls its own update guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/UI/MpPerPlayerUI.cpp`:
- Around line 172-175: Replace the manual toggling of skipUpdateHandler around
the two set_Value calls with a scoped RAII guard that sets skipUpdateHandler =
true on construction and resets it to false on destruction (e.g., implement a
small guard class like SkipUpdateGuard). Update the code around
ppdt->set_Value(false) and ppmt->set_Value(false) (and the identical block at
the other location) to instantiate that guard at the start of the scope so the
flag is automatically restored even on early returns/exceptions.
- Around line 49-50: The namespace-level flag skipUpdateHandler should be
converted to an instance member on MpPerPlayerUI to avoid cross-instance
interference: add a private bool _skipUpdateHandler (initialized in
MpPerPlayerUI constructor), remove the namespace variable, and update all
guarded sites (where set_Value and other setters check skipUpdateHandler) to use
this->_skipUpdateHandler instead; ensure any code that toggles the guard (set to
true/false) is updated to operate on the instance member so each MpPerPlayerUI
controls its own update guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9aeee1c5-7fe0-483b-9963-b28db58e7cd4
📒 Files selected for processing (1)
src/UI/MpPerPlayerUI.cpp
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Hooks/GameServerPlayerTableCellHooks.cpp (1)
37-44: Remove the manualAwake()call afterAddComponent().
AddComponent<T>()automatically invokesAwake()immediately. The manual call at line 39 is redundant and violates Unity'sMonoBehaviourcontract—Awake()should only be invoked by the engine. Keep the manual DI injection as a separate step (it's not tied to the lifecycle), but let Unity own theAwake()lifecycle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Hooks/GameServerPlayerTableCellHooks.cpp` around lines 37 - 44, Remove the manual call to Awake() on the newly added component: after calling AddComponent<T>() do NOT invoke customData->Awake(); instead let Unity call Awake() itself; keep the DI injection logic (use BSML::Helpers::GetDiContainer() and, if diContainer is valid, call diContainer->Inject(customData)) as a separate step but do not manually trigger lifecycle methods on the MonoBehaviour (remove the call to Awake() and any comment suggesting manual lifecycle invocation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Hooks/GameServerPlayerTableCellHooks.cpp`:
- Around line 39-45: When DI is unavailable, avoid leaving an uninjected
GameServerPlayerTableCellCustomData component attached and calling SetData;
instead detect GetDiContainer() == nullptr and either remove/destroy the
customData component or return the original cell behavior immediately.
Concretely, in the block around GetDiContainer()/Inject for
GameServerPlayerTableCellCustomData, if diContainer is null then do not call
customData->SetData(…) — instead remove the added component (or undo creation)
and return the fallback/default cell value; apply the same change to the second
identical block (the one referenced at lines 56-61) so un-injected components
are never left attached and SetData isn’t called when dependencies are missing.
- Around line 54-60: The code calls
AddComponent<MultiplayerCore::UI::GameServerPlayerTableCellCustomData*>() and
immediately calls diContainer->Inject(customData) without verifying customData
is non-null; add the same null-check used in the other (prefab) path before
calling Inject: after creating customData, if customData is nullptr log an error
and skip injection, otherwise call diContainer->Inject(customData). Ensure you
reference the customData variable and the
MultiplayerCore::UI::GameServerPlayerTableCellCustomData type when adding the
guard.
---
Nitpick comments:
In `@src/Hooks/GameServerPlayerTableCellHooks.cpp`:
- Around line 37-44: Remove the manual call to Awake() on the newly added
component: after calling AddComponent<T>() do NOT invoke customData->Awake();
instead let Unity call Awake() itself; keep the DI injection logic (use
BSML::Helpers::GetDiContainer() and, if diContainer is valid, call
diContainer->Inject(customData)) as a separate step but do not manually trigger
lifecycle methods on the MonoBehaviour (remove the call to Awake() and any
comment suggesting manual lifecycle invocation).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 531b3852-3cee-4521-a522-91633373e751
📒 Files selected for processing (3)
src/Beatmaps/BeatSaverBeatmapLevel.cppsrc/Hooks/GameServerPlayerTableCellHooks.cppsrc/Objects/MpPlayersDataModel.cpp
💤 Files with no reviewable changes (1)
- src/Objects/MpPlayersDataModel.cpp
✅ Files skipped from review due to trivial changes (1)
- src/Beatmaps/BeatSaverBeatmapLevel.cpp
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/UI/MpPerPlayerUI.cpp (2)
52-53: Consider makingskipUpdateHandlera private class member instead of namespace-level state.Namespace-scoped mutable state reduces encapsulation and could cause subtle bugs if multiple instances ever exist. Moving this to a private member variable of
MpPerPlayerUIwould be cleaner and safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UI/MpPerPlayerUI.cpp` around lines 52 - 53, The namespace-level mutable flag skipUpdateHandler should be moved into the MpPerPlayerUI class as a private member to avoid global state; add a private bool skipUpdateHandler; initialize it in MpPerPlayerUI's constructor, remove the namespace-scope declaration, and update all uses inside MpPerPlayerUI methods (the toggle handlers and any set_Value guards) to reference this->skipUpdateHandler instead of the global symbol; ensure any functions outside the class that relied on the namespace variable either become class methods or accept a reference to the instance.
175-178: Consider a RAII scope guard for exception safety.If
set_Valuewere to throw,skipUpdateHandlerwould remaintrue, breaking subsequent interactions. A simple scope guard ensures cleanup:💡 Example RAII guard pattern
// Helper struct (could be defined once and reused) struct ScopedGuard { bool& flag; ScopedGuard(bool& f) : flag(f) { flag = true; } ~ScopedGuard() { flag = false; } }; // Usage: { ScopedGuard guard(skipUpdateHandler); ppdt->set_Value(false); ppmt->set_Value(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UI/MpPerPlayerUI.cpp` around lines 175 - 178, The code sets skipUpdateHandler = true before calling ppdt->set_Value(false) and ppmt->set_Value(false) but resets it to false only after both calls, which is not exception safe; wrap the flag in an RAII scope guard so skipUpdateHandler is set true on construction and always reset to false in the destructor, then replace the manual set/reset and the two set_Value calls with a block that creates the guard (e.g., ScopedGuard) and calls ppdt->set_Value(false) and ppmt->set_Value(false) inside it; reference skipUpdateHandler, ppdt->set_Value, ppmt->set_Value and add the small ScopedGuard helper struct near the function/class to ensure cleanup on exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/UI/MpPerPlayerUI.cpp`:
- Around line 52-53: The namespace-level mutable flag skipUpdateHandler should
be moved into the MpPerPlayerUI class as a private member to avoid global state;
add a private bool skipUpdateHandler; initialize it in MpPerPlayerUI's
constructor, remove the namespace-scope declaration, and update all uses inside
MpPerPlayerUI methods (the toggle handlers and any set_Value guards) to
reference this->skipUpdateHandler instead of the global symbol; ensure any
functions outside the class that relied on the namespace variable either become
class methods or accept a reference to the instance.
- Around line 175-178: The code sets skipUpdateHandler = true before calling
ppdt->set_Value(false) and ppmt->set_Value(false) but resets it to false only
after both calls, which is not exception safe; wrap the flag in an RAII scope
guard so skipUpdateHandler is set true on construction and always reset to false
in the destructor, then replace the manual set/reset and the two set_Value calls
with a block that creates the guard (e.g., ScopedGuard) and calls
ppdt->set_Value(false) and ppmt->set_Value(false) inside it; reference
skipUpdateHandler, ppdt->set_Value, ppmt->set_Value and add the small
ScopedGuard helper struct near the function/class to ensure cleanup on
exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 706ba82e-53c2-4b59-9dbc-546b927a8299
📒 Files selected for processing (1)
src/UI/MpPerPlayerUI.cpp
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
scripts/pull-tombstone.ps1 (1)
22-24: Convert these global variables to script-local scope.
$currentDate,$recentDate, and$recentTombstoneare used only within this script and don't need global scope. Script-local variables prevent mutable state leakage if the script is dot-sourced and make the intent clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/pull-tombstone.ps1` around lines 22 - 24, Replace the unnecessary global-scoped variables with script-scoped ones: change the three variables currently declared as $global:currentDate, $global:recentDate, and $global:recentTombstone to script-local scope (use $script:currentDate, $script:recentDate, $script:recentTombstone or declare them without a scope inside the script) so their state is confined to this script (ensure any subsequent references use the same symbol names such as $script:currentDate, $script:recentDate, $script:recentTombstone or the unscoped names consistently).scripts/createqmod.ps1 (1)
37-71: Refactor duplicated dependency resolution into a helper function.The same existence/fallback/error logic is repeated three times. A small helper reduces drift and future bugs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/createqmod.ps1` around lines 37 - 71, Extract the repeated existence/fallback/error logic into a helper function like Resolve-DependencyPath that accepts a filename (e.g., $mod or $lib) and returns the resolved path or triggers an exit/error; replace the three identical blocks that iterate over $modJson.modFiles, $modJson.lateModFiles and $modJson.libraryFiles to call Resolve-DependencyPath for each item and append the returned path to $filelist, ensuring the helper encapsulates the "./build/" then "./extern/libs/" checks and the Write-Output/exit 1 error behavior used currently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build.ps1`:
- Line 10: Fix the typo in the build script help text: locate the Write-Output
call that prints the string "`"Build`" - Copiles your mod into a `".so`" or a
`".a`" library" and change "Copiles" to "Compiles" so the message reads
"`"Build`" - Compiles your mod into a `".so`" or a `".a`" library".
- Around line 20-31: The script uses relative paths and always runs the build
step even if CMake configure fails; update it to resolve the build directory
relative to the script location (use $PSScriptRoot or
$MyInvocation.MyCommand.Definition and Join-Path to form the "build" path) and
replace literal "build" and "./build" with that resolved path when calling
Test-Path, Remove-Item, New-Item and the cmake commands; after running the
configure command (cmake -B <resolvedBuild> ...), check its result via
$LASTEXITCODE or $? and only run the build step (cmake --build <resolvedBuild>)
when the configure succeeded, returning a non-zero exit code on failure.
In `@scripts/copy.ps1`:
- Around line 58-75: After each invocation of adb push inside the foreach loops
over $modFiles and $lateModFiles (the branches that use $useDebug to pick
build/debug vs build), check $LASTEXITCODE and if it is non-zero log an error
mentioning the failing file and exit with a non-zero code; do the same
immediately after calling & $PSScriptRoot/restart-game.ps1 (check $LASTEXITCODE,
log and exit non-zero on failure) so failures don’t get swallowed and the script
returns a failing status.
- Around line 53-70: The script uses working-directory-relative paths for
mod.json and build artifacts which break when copy.ps1 is run outside the repo
root; update the references that read/consume $modJson, $modFiles, $lateModFiles
and the adb push invocations that use build/... and build/debug/... to instead
build repo-rooted paths using the script root (use $PSScriptRoot to prefix
../mod.json and ../build or ../build/debug) so every Get-Content "./mod.json"
and all adb push sources point to $PSScriptRoot/../mod.json and
$PSScriptRoot/../build( /debug)/<fileName> respectively while keeping the
$useDebug conditional and existing foreach loops unchanged.
In `@scripts/createqmod.ps1`:
- Around line 18-40: The script uses cwd-relative paths for $mod, $cover and the
foreach loop over $modJson.modFiles which breaks when not run from the repo
root; change all path constructions to be anchored to $PSScriptRoot (or the repo
root resolved from it) instead of "./"—e.g., resolve $mod via
Join-Path/Resolve-Path with $PSScriptRoot before calling Get-Content, build
$cover and each $path for "./build/" and "./extern/libs/" from that same base,
and use the resulting absolute paths for Test-Path and to populate $filelist so
all operations (Get-Content, Test-Path, file additions) are repo-root portable.
- Around line 76-77: The script uses Compress-Archive with -Update which
preserves files removed from the source; change the logic around the $zip/$qmod
creation to remove any existing archive first and recreate it (e.g., if
(Test-Path $zip) { Remove-Item $zip -Force } then call Compress-Archive -Path
$filelist -DestinationPath $zip without -Update, and finally Move-Item $zip
$qmod -Force), referencing the existing $zip, $qmod and $filelist variables so
the created .qmod contains only current files.
In `@scripts/pull-tombstone.ps1`:
- Around line 48-52: The adb pull can fail but $analyze block still runs; change
the logic so ndk-stack.ps1 is only invoked when the pull succeeded: run the pull
(& adb pull /sdcard/... $fileName) and capture its success (check $LASTEXITCODE
or wrap the pull in a try/catch and set a flag like $pullSucceeded), then make
the conditional use both $analyze and that success flag (e.g., if ($analyze -and
$pullSucceeded) { & $PSScriptRoot/ndk-stack.ps1 -logName:$fileName }) so you
never analyze a stale/local file.
- Around line 26-33: The loop that reads tombstone_0$i currently calls exit 1
when $date is empty, aborting the whole scan; change this to skip the unreadable
slot instead: log a warning when the regex parse fails for $stats (including the
slot name tombstone_0$i), continue the for loop rather than calling exit, and
only fail the script afterward if none of the slots produced a valid $dateObj
(track a success flag or count). Update references in this block ($stats, $date,
$dateObj, tombstone_0$i) to implement the non-fatal skip and final aggregate
failure check.
In `@scripts/start-logging.ps1`:
- Around line 39-53: The script always resolves and appends a PID causing the
-all flag to be ignored; wrap the PID resolution loop and the "$command +=
\"--pid $bspid\"" line inside a conditional that only runs when $all -eq $false
so full-device logging works when $all is true; specifically gate the while loop
that sets $bspid and the subsequent check/exit and the $command += "--pid
$bspid" behind the if ($all -eq $false) block so $command is left unchanged when
$all is true.
- Around line 67-75: The current use of Invoke-Expression to run the
interpolated $command string (which may include $custom) creates a
command-injection risk; instead build the command and arguments as a safe array
and invoke directly (using the call operator & or Start-Process -FilePath with
-ArgumentList) rather than string interpolation and Invoke-Expression. Locate
where $command is assembled and replace the final string
concatenation/Invoke-Expression with a parameterized invocation that passes the
executable path and each argument separately, and handle piping operations
(Select-String/Out-File) by calling the appropriate cmdlets/functions directly
or by using Start-Process with -RedirectStandardOutput to the log file; ensure
$custom and other variables are passed as arguments, not embedded into a single
command string.
In `@scripts/validate-modjson.ps1`:
- Around line 21-33: Wrap the download and validation sequence in a
try/catch/finally: call Invoke-WebRequest with -ErrorAction Stop (and likewise
use -ErrorAction Stop on Get-Content and Test-Json) inside the try block,
perform validation using $modJsonRaw | Test-Json -Schema $modSchemaRaw in the
try, log and exit with non-zero in the catch when any step fails, and always
call Remove-Item $schema in the finally to ensure cleanup; reference the
existing symbols Invoke-WebRequest, $schema, $modJsonRaw, $modSchemaRaw,
Test-Json, Remove-Item to locate where to add try/catch/finally and error
handling.
- Around line 1-5: The script currently uses relative CWD paths for $mod and
mod.template.json and performs network/file operations without error handling;
change the path resolution for $mod and the template to use the script's
directory (e.g., compute the repo root from $PSScriptRoot via Split-Path -Parent
and build the full paths with Join-Path) so mod.json and mod.template.json are
found regardless of where the script is invoked, and add robust error handling
around Invoke-WebRequest and Remove-Item (use -ErrorAction Stop or wrap each
call in try/catch) to surface download/remove errors and clean up any partial
files if the download fails.
---
Nitpick comments:
In `@scripts/createqmod.ps1`:
- Around line 37-71: Extract the repeated existence/fallback/error logic into a
helper function like Resolve-DependencyPath that accepts a filename (e.g., $mod
or $lib) and returns the resolved path or triggers an exit/error; replace the
three identical blocks that iterate over $modJson.modFiles,
$modJson.lateModFiles and $modJson.libraryFiles to call Resolve-DependencyPath
for each item and append the returned path to $filelist, ensuring the helper
encapsulates the "./build/" then "./extern/libs/" checks and the
Write-Output/exit 1 error behavior used currently.
In `@scripts/pull-tombstone.ps1`:
- Around line 22-24: Replace the unnecessary global-scoped variables with
script-scoped ones: change the three variables currently declared as
$global:currentDate, $global:recentDate, and $global:recentTombstone to
script-local scope (use $script:currentDate, $script:recentDate,
$script:recentTombstone or declare them without a scope inside the script) so
their state is confined to this script (ensure any subsequent references use the
same symbol names such as $script:currentDate, $script:recentDate,
$script:recentTombstone or the unscoped names consistently).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03bf4f7f-6e85-4bb4-9b0d-d9f7486ab7c3
📒 Files selected for processing (8)
scripts/build.ps1scripts/copy.ps1scripts/createqmod.ps1scripts/pull-tombstone.ps1scripts/restart-game.ps1scripts/start-logging.ps1scripts/validate-modjson.ps1src/Objects/MpLevelLoader.cpp
✅ Files skipped from review due to trivial changes (1)
- scripts/restart-game.ps1
| ) | ||
|
|
||
| if ($help -eq $true) { | ||
| Write-Output "`"Build`" - Copiles your mod into a `".so`" or a `".a`" library" |
There was a problem hiding this comment.
Fix help text typo.
Line 10 says “Copiles”; should be “Compiles”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build.ps1` at line 10, Fix the typo in the build script help text:
locate the Write-Output call that prints the string "`"Build`" - Copiles your
mod into a `".so`" or a `".a`" library" and change "Copiles" to "Compiles" so
the message reads "`"Build`" - Compiles your mod into a `".so`" or a `".a`"
library".
| if (Test-Path -Path "build") { | ||
| remove-item build -R | ||
| } | ||
| } | ||
|
|
||
|
|
||
| if (($clean.IsPresent) -or (-not (Test-Path -Path "build"))) { | ||
| new-item -Path build -ItemType Directory | ||
| } | ||
|
|
||
| & cmake -G "Ninja" -DCMAKE_BUILD_TYPE="RelWithDebInfo" -B build | ||
| & cmake --build ./build |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat scripts/build.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 803
Anchor build paths to script location and check CMake configure exit code before building.
The script uses relative paths (build, ./build) that resolve to the current working directory, not the script location. If invoked from a different directory, it targets the wrong tree. Additionally, the CMake build command (line 31) executes even if the configure step (line 30) fails, causing confusing secondary errors.
Suggested fix
+ $repoRoot = Resolve-Path (Join-Path $PSScriptRoot "..")
+ $buildDir = Join-Path $repoRoot "build"
+
if ($clean.IsPresent) {
- if (Test-Path -Path "build") {
- remove-item build -R
+ if (Test-Path -Path $buildDir) {
+ Remove-Item $buildDir -Recurse -Force
}
}
- if (($clean.IsPresent) -or (-not (Test-Path -Path "build"))) {
- new-item -Path build -ItemType Directory
+ if (-not (Test-Path -Path $buildDir)) {
+ New-Item -Path $buildDir -ItemType Directory | Out-Null
}
- & cmake -G "Ninja" -DCMAKE_BUILD_TYPE="RelWithDebInfo" -B build
- & cmake --build ./build
+ & cmake -S $repoRoot -B $buildDir -G "Ninja" -DCMAKE_BUILD_TYPE="RelWithDebInfo"
+ if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
+ & cmake --build $buildDir
+ if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build.ps1` around lines 20 - 31, The script uses relative paths and
always runs the build step even if CMake configure fails; update it to resolve
the build directory relative to the script location (use $PSScriptRoot or
$MyInvocation.MyCommand.Definition and Join-Path to form the "build" path) and
replace literal "build" and "./build" with that resolved path when calling
Test-Path, Remove-Item, New-Item and the cmake commands; after running the
configure command (cmake -B <resolvedBuild> ...), check its result via
$LASTEXITCODE or $? and only run the build step (cmake --build <resolvedBuild>)
when the configure succeeded, returning a non-zero exit code on failure.
| $modJson = Get-Content "./mod.json" -Raw | ConvertFrom-Json | ||
|
|
||
| $modFiles = $modJson.modFiles | ||
| $lateModFiles = $modJson.lateModFiles | ||
|
|
||
| foreach ($fileName in $modFiles) { | ||
| if ($useDebug -eq $true) { | ||
| & adb push build/debug/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/early_mods/$fileName | ||
| } else { | ||
| & adb push build/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/early_mods/$fileName | ||
| } | ||
| } | ||
|
|
||
| foreach ($fileName in $lateModFiles) { | ||
| if ($useDebug -eq $true) { | ||
| & adb push build/debug/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/mods/$fileName | ||
| } else { | ||
| & adb push build/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/mods/$fileName |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the repository structure
git ls-files | head -20Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 509
🏁 Script executed:
# Read the full scripts/copy.ps1 file to understand context
wc -l scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 98
🏁 Script executed:
# Read the relevant section and surrounding context
sed -n '40,75p' scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1079
🏁 Script executed:
# Find mod.json location
find . -name "mod.json" -type fRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 61
🏁 Script executed:
# Check the build.ps1 script to understand build output paths
sed -n '1,50p' scripts/build.ps1 | head -40Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 803
🏁 Script executed:
# Check if mod.json is in scripts directory
ls -la scripts/ | grep -E "(mod\.json|\.json)"Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 61
🏁 Script executed:
# Read the beginning of copy.ps1 to see initialization and parameter handling
sed -n '1,40p' scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1095
🏁 Script executed:
# Check if mod.json exists in the repo (broader search)
find . -name "*.json" -type f | grep -i modRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 98
🏁 Script executed:
# Check full repository structure at root and scripts
ls -la scripts/ | head -30Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 717
🏁 Script executed:
# Read more of copy.ps1 to see if working directory is set
sed -n '40,60p' scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 619
🏁 Script executed:
# Check the README or any documentation on how to run scripts
grep -r "copy.ps1\|\.ps1" README.md 2>/dev/null | head -20Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 61
🏁 Script executed:
# Check if there's a script that calls copy.ps1 to see intended cwd
grep -r "copy\.ps1" . --include="*.ps1" --include="*.yml" --include="*.yaml" --include="*.md"Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 61
🏁 Script executed:
# Check if other PowerShell scripts follow similar patterns
rg 'Get-Content.*"\./[^"]*"' scripts/ --no-headingRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 155
🏁 Script executed:
# Check if any validation script gives hints about where mod.json should be
cat scripts/validate-modjson.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1149
🏁 Script executed:
# Check the entire copy.ps1 to understand full context
cat -n scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 2779
Use repo-rooted file paths for mod.json and build artifacts.
Lines 53, 60, 62, 68, and 70 use working-directory-relative paths. When copy.ps1 is invoked from directories other than the repository root, these relative path lookups will fail. Use $PSScriptRoot/../mod.json for the mod.json reference and $PSScriptRoot/../build for build artifacts to match the pattern already used for script invocations on lines 42, 49, and 74.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/copy.ps1` around lines 53 - 70, The script uses
working-directory-relative paths for mod.json and build artifacts which break
when copy.ps1 is run outside the repo root; update the references that
read/consume $modJson, $modFiles, $lateModFiles and the adb push invocations
that use build/... and build/debug/... to instead build repo-rooted paths using
the script root (use $PSScriptRoot to prefix ../mod.json and ../build or
../build/debug) so every Get-Content "./mod.json" and all adb push sources point
to $PSScriptRoot/../mod.json and $PSScriptRoot/../build( /debug)/<fileName>
respectively while keeping the $useDebug conditional and existing foreach loops
unchanged.
| foreach ($fileName in $modFiles) { | ||
| if ($useDebug -eq $true) { | ||
| & adb push build/debug/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/early_mods/$fileName | ||
| } else { | ||
| & adb push build/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/early_mods/$fileName | ||
| } | ||
| } | ||
|
|
||
| foreach ($fileName in $lateModFiles) { | ||
| if ($useDebug -eq $true) { | ||
| & adb push build/debug/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/mods/$fileName | ||
| } else { | ||
| & adb push build/$fileName /sdcard/ModData/com.beatgames.beatsaber/Modloader/mods/$fileName | ||
| } | ||
| } | ||
|
|
||
| & $PSScriptRoot/restart-game.ps1 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n scripts/copy.ps1 | head -100Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 2779
🏁 Script executed:
grep -n "ErrorActionPreference\|$ErrorActionPreference" scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 2454
🏁 Script executed:
cat scripts/restart-game.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 208
Check exit codes for adb push and restart-game.ps1 calls.
Lines 58–75 do not check $LASTEXITCODE after adb push or restart-game.ps1 operations. If a push fails (device disconnect, missing file, etc.), the loop continues silently and the script exits with code 0, creating a false sense of success. Add $LASTEXITCODE checks after each push and after the restart call, exiting non-zero on failure—consistent with how this script handles the build and validation steps.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/copy.ps1` around lines 58 - 75, After each invocation of adb push
inside the foreach loops over $modFiles and $lateModFiles (the branches that use
$useDebug to pick build/debug vs build), check $LASTEXITCODE and if it is
non-zero log an error mentioning the failing file and exit with a non-zero code;
do the same immediately after calling & $PSScriptRoot/restart-game.ps1 (check
$LASTEXITCODE, log and exit non-zero on failure) so failures don’t get swallowed
and the script returns a failing status.
| $mod = "./mod.json" | ||
|
|
||
| & $PSScriptRoot/validate-modjson.ps1 | ||
| if ($LASTEXITCODE -ne 0) { | ||
| exit $LASTEXITCODE | ||
| } | ||
| $modJson = Get-Content $mod -Raw | ConvertFrom-Json | ||
|
|
||
| if ($qmodName -eq "") { | ||
| $qmodName = $modJson.name | ||
| } | ||
|
|
||
| $filelist = @($mod) | ||
|
|
||
| $cover = "./" + $modJson.coverImage | ||
| if ((-not ($cover -eq "./")) -and (Test-Path $cover)) { | ||
| $filelist += ,$cover | ||
| } | ||
|
|
||
| foreach ($mod in $modJson.modFiles) { | ||
| $path = "./build/" + $mod | ||
| if (-not (Test-Path $path)) { | ||
| $path = "./extern/libs/" + $mod |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the createqmod.ps1 script
find . -name "createqmod.ps1" -type fRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 103
🏁 Script executed:
# Read the full script to understand the context
cat -n scripts/createqmod.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 2368
🏁 Script executed:
# Check if there are other scripts in the same directory that might give context
ls -la scripts/Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 717
🏁 Script executed:
# Look for README or documentation
find . -maxdepth 2 -type f \( -name "README*" -o -name "*.md" \) | head -20Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 117
🏁 Script executed:
# Check build.ps1 to see how paths are handled there
cat -n scripts/build.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1020
🏁 Script executed:
# Check if there are any CI/workflow files that show how this script is called
find . -path "*/.github/workflows/*.yml" -o -path "*/CI/*" -o -path "*/.gitlab-ci.yml" 2>/dev/null | head -10Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 214
🏁 Script executed:
# Check the workflow files to see how createqmod.ps1 is invoked
cat -n .github/workflows/Pre-Release.ymlRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 5502
🏁 Script executed:
cat -n .github/workflows/Release.ymlRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 5862
🏁 Script executed:
# Also check README to see if there's documentation about script usage
cat -n README.md | head -100Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 669
Resolve package input paths from repo root for portability.
The script mixes approaches: line 20 correctly uses $PSScriptRoot to invoke validate-modjson.ps1, but lines 18+ revert to cwd-relative paths (./mod.json, ./build/, ./extern/libs/), breaking if invoked from a directory other than the repository root. This inconsistency makes the script fragile for local development. Refactor paths to use $PSScriptRoot as the base, e.g., Join-Path (Resolve-Path (Join-Path $PSScriptRoot "..")) "mod.json" or similar, for consistent portability across all path operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/createqmod.ps1` around lines 18 - 40, The script uses cwd-relative
paths for $mod, $cover and the foreach loop over $modJson.modFiles which breaks
when not run from the repo root; change all path constructions to be anchored to
$PSScriptRoot (or the repo root resolved from it) instead of "./"—e.g., resolve
$mod via Join-Path/Resolve-Path with $PSScriptRoot before calling Get-Content,
build $cover and each $path for "./build/" and "./extern/libs/" from that same
base, and use the resulting absolute paths for Test-Path and to populate
$filelist so all operations (Get-Content, Test-Path, file additions) are
repo-root portable.
| & adb pull /sdcard/Android/data/com.beatgames.beatsaber/files/tombstone_0$recentTombstone $fileName | ||
|
|
||
| if ($analyze) { | ||
| & $PSScriptRoot/ndk-stack.ps1 -logName:$fileName | ||
| } |
There was a problem hiding this comment.
Gate analysis on successful adb pull.
Line 51 can run even when Line 48 failed, which may analyze an old/local stale file and produce misleading output.
Suggested fix
& adb pull /sdcard/Android/data/com.beatgames.beatsaber/files/tombstone_0$recentTombstone $fileName
+if ($LASTEXITCODE -ne 0) {
+ Write-Error "Failed to pull tombstone_0$recentTombstone"
+ exit 1
+}
if ($analyze) {
& $PSScriptRoot/ndk-stack.ps1 -logName:$fileName
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/pull-tombstone.ps1` around lines 48 - 52, The adb pull can fail but
$analyze block still runs; change the logic so ndk-stack.ps1 is only invoked
when the pull succeeded: run the pull (& adb pull /sdcard/... $fileName) and
capture its success (check $LASTEXITCODE or wrap the pull in a try/catch and set
a flag like $pullSucceeded), then make the conditional use both $analyze and
that success flag (e.g., if ($analyze -and $pullSucceeded) { &
$PSScriptRoot/ndk-stack.ps1 -logName:$fileName }) so you never analyze a
stale/local file.
| # if ($all -eq $false) { | ||
| $loops = 0 | ||
| while ([string]::IsNullOrEmpty($bspid) -and $loops -lt 6) { | ||
| Start-Sleep -Milliseconds 100 | ||
| $bspid = adb shell pidof com.beatgames.beatsaber | ||
| $loops += 1 | ||
| } | ||
|
|
||
| if ([string]::IsNullOrEmpty($bspid)) { | ||
| Write-Output "Could not connect to adb, exiting..." | ||
| exit 1 | ||
| } | ||
|
|
||
| $command += "--pid $bspid" | ||
| # } |
There was a problem hiding this comment.
-all is effectively ignored because --pid is always enforced.
Line 39–53 always resolve PID and append --pid, so Line 28 help text is inaccurate and full-device logging never happens. Gate PID logic behind if ($all -eq $false).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/start-logging.ps1` around lines 39 - 53, The script always resolves
and appends a PID causing the -all flag to be ignored; wrap the PID resolution
loop and the "$command += \"--pid $bspid\"" line inside a conditional that only
runs when $all -eq $false so full-device logging works when $all is true;
specifically gate the while loop that sets $bspid and the subsequent check/exit
and the $command += "--pid $bspid" behind the if ($all -eq $false) block so
$command is left unchanged when $all is true.
| $command += " | Select-String -pattern `"$pattern`"" | ||
| } | ||
|
|
||
| if ($file -eq $true) { | ||
| $command += " | Out-File -FilePath $PSScriptRoot\..\log.log" | ||
| } | ||
|
|
||
| Write-Output "Logging using Command `"$command`"" | ||
| Invoke-Expression $command |
There was a problem hiding this comment.
Replace Invoke-Expression command construction with direct invocation.
Line 67–75 executes interpolated strings containing $custom, which creates command-injection risk and is flagged by PSScriptAnalyzer.
Suggested fix
-Write-Output "Logging using Command `"$command`""
-Invoke-Expression $command
+Write-Output "Starting log capture..."
+if ($all) {
+ if ($file) { & adb logcat | Out-File -FilePath (Join-Path $PSScriptRoot "..\log.log") }
+ else { & adb logcat }
+} else {
+ if ($file) { & adb logcat --pid $bspid | Select-String -Pattern $pattern | Out-File -FilePath (Join-Path $PSScriptRoot "..\log.log") }
+ else { & adb logcat --pid $bspid | Select-String -Pattern $pattern }
+}🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 75-75: Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
(PSAvoidUsingInvokeExpression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/start-logging.ps1` around lines 67 - 75, The current use of
Invoke-Expression to run the interpolated $command string (which may include
$custom) creates a command-injection risk; instead build the command and
arguments as a safe array and invoke directly (using the call operator & or
Start-Process -FilePath with -ArgumentList) rather than string interpolation and
Invoke-Expression. Locate where $command is assembled and replace the final
string concatenation/Invoke-Expression with a parameterized invocation that
passes the executable path and each argument separately, and handle piping
operations (Select-String/Out-File) by calling the appropriate cmdlets/functions
directly or by using Start-Process with -RedirectStandardOutput to the log file;
ensure $custom and other variables are passed as arguments, not embedded into a
single command string.
| $mod = "./mod.json" | ||
|
|
||
| if (-not (Test-Path -Path $mod)) { | ||
| if (Test-Path -Path ".\mod.template.json") { | ||
| & qpm qmod build |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the PowerShell script
cat -n scripts/validate-modjson.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1415
🏁 Script executed:
# Check repo structure and locations of the files mentioned
fd "mod\.json|mod\.template\.json|mod\.schema\.json" -t fRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 96
🏁 Script executed:
# Find where this script is invoked from
rg "validate-modjson" -t json -t yaml -t powershell -t shellRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 113
🏁 Script executed:
# Search for invocations of the validate script more broadly
rg "validate-modjson" --max-count=20Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 192
🏁 Script executed:
# Check package.json and other script files for invocations
cat -n package.json | head -100Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 123
🏁 Script executed:
# Check for any CI/CD files
fd "\.yml|\.yaml|\.json" scripts/ -t fRepository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 61
🏁 Script executed:
# Check repo structure to confirm file locations
ls -la | head -20Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1306
🏁 Script executed:
# Check where mod.template.json actually is
find . -name "mod.template.json" -o -name "mod.json"Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 98
🏁 Script executed:
# Look at the other scripts that call validate-modjson.ps1 to understand context
head -20 scripts/copy.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 457
🏁 Script executed:
# Also check createqmod.ps1
head -20 scripts/createqmod.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 496
Resolve paths from script's parent directory; add error handling for network requests.
Lines 1–5 rely on the current working directory for path resolution, which breaks when this script is invoked from a different directory. Resolve paths using Split-Path -Parent $PSScriptRoot (or $PSScriptRoot/..) to reference the repo root where mod.json and mod.template.json reside.
Additionally, line 21 downloads the schema with Invoke-WebRequest without error handling—add -ErrorAction Stop or wrap in a try-catch to avoid leaving partial or invalid state if the download fails. The same applies to Remove-Item on line 27.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/validate-modjson.ps1` around lines 1 - 5, The script currently uses
relative CWD paths for $mod and mod.template.json and performs network/file
operations without error handling; change the path resolution for $mod and the
template to use the script's directory (e.g., compute the repo root from
$PSScriptRoot via Split-Path -Parent and build the full paths with Join-Path) so
mod.json and mod.template.json are found regardless of where the script is
invoked, and add robust error handling around Invoke-WebRequest and Remove-Item
(use -ErrorAction Stop or wrap each call in try/catch) to surface
download/remove errors and clean up any partial files if the download fails.
| Invoke-WebRequest $schemaUrl -OutFile ./mod.schema.json | ||
|
|
||
| $schema = "./mod.schema.json" | ||
| $modJsonRaw = Get-Content $mod -Raw | ||
| $modSchemaRaw = Get-Content $schema -Raw | ||
|
|
||
| Remove-Item $schema | ||
|
|
||
| Write-Output "Validating mod.json..." | ||
| if (-not ($modJsonRaw | Test-Json -Schema $modSchemaRaw)) { | ||
| Write-Output "Error: mod.json is not valid" | ||
| exit 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n scripts/validate-modjson.ps1Repository: EnderdracheLP/MultiplayerCore.Quest
Length of output: 1415
Add error handling around schema download and validation cleanup.
If Invoke-WebRequest fails at line 21, subsequent Get-Content/Remove-Item calls can fail noisily or silently. Wrap download/validation in try/finally with -ErrorAction Stop to ensure cleanup occurs and errors are clear.
Suggested fix
- Invoke-WebRequest $schemaUrl -OutFile ./mod.schema.json
-
- $schema = "./mod.schema.json"
- $modJsonRaw = Get-Content $mod -Raw
- $modSchemaRaw = Get-Content $schema -Raw
-
- Remove-Item $schema
-
- Write-Output "Validating mod.json..."
- if (-not ($modJsonRaw | Test-Json -Schema $modSchemaRaw)) {
- Write-Output "Error: mod.json is not valid"
- exit 1
- }
+ $schema = "./mod.schema.json"
+ try {
+ Invoke-WebRequest $schemaUrl -OutFile $schema -ErrorAction Stop
+ $modJsonRaw = Get-Content $mod -Raw -ErrorAction Stop
+ $modSchemaRaw = Get-Content $schema -Raw -ErrorAction Stop
+ Write-Output "Validating mod.json..."
+ if (-not ($modJsonRaw | Test-Json -Schema $modSchemaRaw)) {
+ Write-Output "Error: mod.json is not valid"
+ exit 1
+ }
+ } finally {
+ if (Test-Path $schema) { Remove-Item $schema -Force }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Invoke-WebRequest $schemaUrl -OutFile ./mod.schema.json | |
| $schema = "./mod.schema.json" | |
| $modJsonRaw = Get-Content $mod -Raw | |
| $modSchemaRaw = Get-Content $schema -Raw | |
| Remove-Item $schema | |
| Write-Output "Validating mod.json..." | |
| if (-not ($modJsonRaw | Test-Json -Schema $modSchemaRaw)) { | |
| Write-Output "Error: mod.json is not valid" | |
| exit 1 | |
| } | |
| $schema = "./mod.schema.json" | |
| try { | |
| Invoke-WebRequest $schemaUrl -OutFile $schema -ErrorAction Stop | |
| $modJsonRaw = Get-Content $mod -Raw -ErrorAction Stop | |
| $modSchemaRaw = Get-Content $schema -Raw -ErrorAction Stop | |
| Write-Output "Validating mod.json..." | |
| if (-not ($modJsonRaw | Test-Json -Schema $modSchemaRaw)) { | |
| Write-Output "Error: mod.json is not valid" | |
| exit 1 | |
| } | |
| } finally { | |
| if (Test-Path $schema) { Remove-Item $schema -Force } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/validate-modjson.ps1` around lines 21 - 33, Wrap the download and
validation sequence in a try/catch/finally: call Invoke-WebRequest with
-ErrorAction Stop (and likewise use -ErrorAction Stop on Get-Content and
Test-Json) inside the try block, perform validation using $modJsonRaw |
Test-Json -Schema $modSchemaRaw in the try, log and exit with non-zero in the
catch when any step fails, and always call Remove-Item $schema in the finally to
ensure cleanup; reference the existing symbols Invoke-WebRequest, $schema,
$modJsonRaw, $modSchemaRaw, Test-Json, Remove-Item to locate where to add
try/catch/finally and error handling.
1 participant
Summary by CodeRabbit
Bug Fixes
Refactor
Chores