Add ci builds / improve local builds#2
Merged
DaymareOn merged 29 commits intoDaymareOn:mainfrom Apr 15, 2026
Merged
Conversation
* Stub out most papyrus dependencies for release and Ci Builds * Add Github Actions CI * Add dev-scripts to simplify building locally * Maintain reference to real sources for local dev builds
Contributor
|
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:
📝 WalkthroughWalkthroughThis pull request establishes a complete CI/CD and local development build infrastructure for the FSMP MCM Skyrim mod. It introduces PowerShell build scripts, GitHub Actions workflow automation, Papyrus project configurations for dev/release/CI modes, a comprehensive dependency stub library for compile-time compilation, and supporting documentation and configuration files. Changes
Sequence DiagramsequenceDiagram
participant Developer
participant GitHub
participant GHActions as GitHub Actions
participant PowerShell
participant Caprica
participant FileSystem as File System
participant Artifacts
Developer->>GitHub: Push to main or create PR
GitHub->>GHActions: Trigger workflow
GHActions->>PowerShell: Execute setup_ci.ps1
PowerShell->>FileSystem: Create ci_tools directory
PowerShell->>Caprica: Download & extract Caprica.exe
PowerShell->>FileSystem: Store Caprica to ci_tools/
GHActions->>FileSystem: Checkout repo with submodules
GHActions->>PowerShell: Execute compile via .ppj config
PowerShell->>Caprica: Invoke Caprica.exe per .psc file
Caprica->>FileSystem: Generate .pex files to Scripts/
FileSystem->>GHActions: .pex files ready
GHActions->>FileSystem: Copy ESP, .pex, configs to Build/
FileSystem->>GHActions: Create directory structure
GHActions->>FileSystem: Compress Build/ to .zip
FileSystem->>Artifacts: Store FSMP-MCM.zip
alt Version Tag Detected
Artifacts->>GitHub: Publish GitHub Release
GitHub->>Artifacts: Attach .zip to release
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~65 minutes The diff introduces substantial, heterogeneous changes across multiple categories: PowerShell build automation with environment detection and dynamic configuration parsing, GitHub Actions workflow with conditional release logic, XML-based Papyrus project file refactoring with path resolution, and a large cohort of repetitive but individually simple stub declarations. While stub files are homogeneous and require less per-file effort, the build scripts and CI configuration demand careful scrutiny for correctness, error handling, and path resolution semantics. Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Contributor
Author
|
@coderabbitai please review |
Contributor
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 27
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/workflows/build.md:
- Line 17: Remove the stray token "// turbo" from the Markdown (it appears
accidentally in the build instructions); open the .agents/workflows/build.md
content and delete the exact string "// turbo" so the README no longer contains
the stray comment line and the build instructions render correctly.
In @.github/workflows/build.yml:
- Around line 24-35: The workflow currently calls Resolve-Path "Scripts"
(assigned to $output) before ensuring the Scripts directory exists and is clean,
which can fail or include stale .pex files; update the job to create the Scripts
directory if missing and remove existing contents (or explicitly delete *.pex)
prior to calling Resolve-Path "Scripts" and before packing the archive (also
apply same fix at the other occurrence around the later Resolve-Path call),
ensuring $output points to a fresh directory.
- Around line 1-12: The workflow "name: Build FSMP MCM" (jobs: build) lacks an
explicit permissions block, so add a top-level permissions mapping to minimize
GITHUB_TOKEN scope; insert a permissions block at the root of the workflow
(above jobs) that grants only what this job needs, e.g. permissions: contents:
read and actions: write (so the job can read repo files and upload artifacts)
and remove any broader defaults.
In `@dev-scripts/compile.ps1`:
- Around line 95-100: The output directory creation step (using $OutputDir)
doesn't remove stale .pex binaries, so before collecting $SourceFiles or
creating $OutputDir you should delete existing .pex files in the target Scripts
directory (e.g., remove "*.pex" under $OutputDir/Scripts or the concrete Scripts
folder) to ensure renamed/deleted source scripts don't leave stale binaries;
implement a cleanup that tests for and removes matching .pex files (and create
the Scripts dir afterwards if missing) so the subsequent $SourceFiles =
Get-ChildItem ... only reflects freshly built sources.
- Around line 16-38: The script currently auto-downloads Caprica from
$CapricaUrl using Invoke-WebRequest and extracts it to $DestDir without
provenance checks; either remove this auto-download block or replace it with a
verified flow: update $CapricaUrl to an authoritative release (e.g., the
Orvid/Caprica v0.3.0 release), add SHA-256 (or signature) verification of the
downloaded $zipFile before Expand-Archive, and document manual-install steps and
verification in comments (or fail with a clear message instructing the user to
install Caprica.exe into $DestDir). Ensure references to $Name, $zipFile,
$outDir, Invoke-WebRequest, and Expand-Archive are updated to reflect the new
verified download or removed if you choose manual-only installation.
In `@dev-scripts/setup_ci.ps1`:
- Around line 33-34: The script trusts the downloaded Caprica artifact and
assumes Get-ChildItem returns Caprica.exe; add integrity verification and an
explicit missing-file failure: after Download-And-Extract (captured in
$capricaPath) compute the file hash for the downloaded Caprica.zip or the
extracted Caprica.exe using Get-FileHash and compare it to a known expected
checksum variable (e.g., $expectedCapricaChecksum); if the checksum mismatches,
remove the bad artifact, write an error to the console, and exit non-zero. Also
change the Get-ChildItem pipeline that finds "Caprica.exe" to capture the result
in a variable (e.g., $capricaExe = Get-ChildItem ... | Select-Object -First 1)
and if $capricaExe is $null, write an explicit error and exit non-zero instead
of calling Copy-Item; only call Copy-Item when the checksum passes and
$capricaExe is present.
In `@README.md`:
- Around line 44-64: Add blank lines before and after the fenced PowerShell code
blocks in README.md to satisfy markdownlint MD031: ensure there is an empty line
immediately above the opening ```powershell and an empty line immediately below
the closing ``` for both the compile.ps1 example block and the Caprica.exe
example block so the fenced blocks are separated from surrounding paragraphs.
- Line 3: Update the build badge URL in README.md so it points to the canonical
repository; replace the repository segment "KaninHop/FSMP-MCM" with
"DaymareOn/FSMP-MCM" in the badge image/link URL (the string containing
"https://github.com/KaninHop/FSMP-MCM/actions/...") so the badge reflects the
correct CI pipeline for this repo.
In `@skyrimse.dev.ppj`:
- Line 4: Replace the hardcoded Flags path value with a placeholder and guiding
comment so other developers know to set their local compiler flags path; update
the Flags attribute (the Flags="..." entry) to something like
Flags="PATH_TO_LOCAL_FLAGS\TESV_Papyrus_Flags.flg" and add a short comment
"ADJUST THIS PATH" or similar next to it so it's clear it must be customized on
each machine.
In `@Source/Scripts/SkyUI_SDK/SKI_ConfigBase.psc`:
- Around line 411-433: The comment and code in SetOptionFlags indicate the
option flags buffer (_optionFlagsBuf) should be updated but the computed
oldFlags is never written back; fix by assigning the newly composed value into
_optionFlagsBuf[index] after calculating oldFlags (preserving the lower
byte/type via oldFlags %= 0x100 and adding a_flags * 0x100), so the buffer
matches the UI update performed by UI.InvokeIntA and remains consistent with
future reads.
In `@Source/Scripts/SkyUI_SDK/SKI_ConfigManager.psc`:
- Around line 143-226: Add a guard at the start of every routed UI event handler
from OnPageSelect through OnColorAccept to bail out if _activeConfig is none;
for handlers that normally call UI.InvokeBool(JOURNAL_MENU, MENU_ROOT +
".unlock", true) ensure you still call that unlock before returning so the UI
interaction stack isn’t left locked (reference the event handlers OnPageSelect,
OnOptionHighlight, OnOptionSelect, OnOptionDefault, OnKeymapChange,
OnSliderSelect, OnSliderAccept, OnMenuSelect, OnMenuAccept, OnColorSelect,
OnColorAccept and the _activeConfig.RemapKey/_activeConfig.* calls to locate the
spots).
- Around line 237-248: The capacity check (_configCount >= 128) runs before the
duplicate scan so an already-registered menu (a_menu) returns -1 at capacity;
change the flow in the registration routine to first iterate _modConfigs to
check for an existing entry (comparing _modConfigs[i] to a_menu and returning i
via GotoState("") if found) and only after that duplicate check enforce the
capacity limit using _configCount >= 128 to return -1, preserving the
redundant-registration path; reference the existing loop over _modConfigs, the
_configCount comparison, and the use of GotoState("") when reordering the
checks.
- Around line 128-139: OnModSelect currently only checks configIndex > -1 and
then uses _modConfigs[configIndex] without validating bounds or presence; update
the handler to ensure configIndex is within range (configIndex <
_modConfigs.length) and that _modConfigs[configIndex] is not none before calling
CloseConfig/OpenConfig on _activeConfig/_modConfigs[configIndex]; if the guard
fails, do not change _activeConfig (optionally log or ignore the event) so
OpenConfig() is never invoked on a none entry.
In `@Source/Scripts/SkyUI_SDK/SKI_ConfigMenu.psc`:
- Around line 1010-1014: The OnDefaultST handler resets the label/index but
fails to restore the widget's anchor state; update OnDefaultST to mirror its
select handler by setting the widget anchor property (e.g., assign
_effectWidgetVAnchorIdx and also write SKI_ActiveEffectsWidgetInstance.VAnchor
or the equivalent widget anchor field) and recompute the widget position after
changing anchors (use _alignmentBaseOffsets/_effectWidgetHAnchorIdx and
_effectWidgetXOffset for X and the vertical equivalents for Y), and make the
same change in the corresponding default handler at the 1036-1039 region so both
label text and actual widget anchors are restored when Default is pressed.
- Around line 796-800: The OnDefaultST event is clearing the armor flag instead
of hands; change the flag constant used when calling
SKI_FavoritesManagerInstance.SetGroupFlag in OnDefaultST from
GROUP_FLAG_UNEQUIP_ARMOR to GROUP_FLAG_UNEQUIP_HANDS so the handler toggles the
correct bit (reference the event OnDefaultST and
SKI_FavoritesManagerInstance.SetGroupFlag and the GROUP_FLAG_UNEQUIP_HANDS
symbol exposed in SKI_FavoritesManager).
- Around line 746-751: OnDefaultST resets _favCurGroupIdx and current group but
sets the displayed text with SetTextOptionValueST(_favCurGroupIdx+1), causing
the UI label to differ from the accept path which uses
SetMenuOptionValueST(_favGroupNames[_favCurGroupIdx]); change the default path
to call SetMenuOptionValueST(_favGroupNames[_favCurGroupIdx]) (use the same
_favGroupNames and _favCurGroupIdx as in the accept flow) so the visible menu
label matches the selected favorite group.
In `@Source/Scripts/SkyUI_SDK/SKI_FavoritesManager.psc`:
- Around line 373-412: In SetGroupHotkey move or add the declaration/assignment
of oldKeycode before the unmap special-case so UnregisterForKey(oldKeycode) uses
a defined value: retrieve oldKeycode = _groupHotkeys[a_groupIndex] (and ensure
oldIndex = _groupHotkeys.Find(a_keycode) is also available if needed) before the
if (a_keycode == -1) block, then use that oldKeycode in UnregisterForKey and
proceed with setting _groupHotkeys[a_groupIndex] = -1; this ensures
UnregisterForKey is called with the current key instead of an undeclared
variable.
- Around line 230-246: OnMenuOpen currently iterates _itemInvalidFlags1 and
_itemInvalidFlags2 which may be nil if OnVersionUpdate hasn't run; add a
nil-check (and lazy initialization) for both arrays at the start of OnMenuOpen
so they are created (with length 128 and default false values) if unset before
the loops that reset them, then continue calling InitControls() and
InitMenuGroupData(); reference _itemInvalidFlags1, _itemInvalidFlags2,
OnMenuOpen and OnVersionUpdate to ensure the initialization logic matches the
version-upgrade creation behavior.
- Around line 721-732: The loop stops early because it compares signed ints
against 0x80000000; change the loop termination to iterate until the bit mask
becomes zero rather than comparing to 0x80000000: start with h = 0x00000001 and
use a loop condition like "while (h != 0)" (or equivalent) so LeftShift(h,1)
will produce 0 after the highest bit and the loop covers all mask bits; update
the block around GetGroupFlag(GROUP_FLAG_UNEQUIP_ARMOR), PlayerREF.GetWornForm,
LogicalAND(h, _usedOutfitMask) and PlayerREF.UnEquipItemEX to use this new
condition so all slot masks including 0x80000000 are processed correctly.
In `@Source/Scripts/SkyUI_SDK/SKI_PlayerLoadGameAlias.psc`:
- Around line 5-7: The current OnPlayerLoadGame event directly casts
GetOwningQuest() to SKI_QuestBase and calls OnGameReload(), which can fail
silently; instead, store GetOwningQuest() in a local variable, check that it is
not None and that it is of type SKI_QuestBase (or use a safe cast check) before
calling OnGameReload() on that instance (refer to OnPlayerLoadGame,
GetOwningQuest, SKI_QuestBase, and OnGameReload to locate the code).
In `@Source/Scripts/SkyUI_SDK/SKI_QF_ConfigManagerInstance.psc`:
- Around line 13-18: The code currently casts self to Quest (__temp) and then to
SKI_ConfigManager (kmyQuest) and calls kmyQuest.ForceReset() unconditionally;
you must guard against a failed cast by checking that kmyQuest is not None
before calling ForceReset. Modify the block around the casts (__temp and
kmyQuest) to perform a null-check on kmyQuest (or __temp) and only call
SKI_ConfigManager.ForceReset() when the cast succeeded, otherwise skip the call
(and optionally log or return) to avoid runtime errors.
In `@Source/Scripts/SkyUI_SDK/SKI_WidgetBase.psc`:
- Around line 42-46: The Ready property currently returns _initialized but
should expose the widget's true readiness flag _ready; change the getter in the
Ready property to return _ready (so UI.Invoke* calls and property setters don't
run before OnWidgetLoad() sets _ready and _widgetRoot exists), leaving the
property read-only and preserving existing behavior in OnGameReload() and
OnWidgetLoad().
In `@Source/Scripts/SkyUI_SDK/SKI_WidgetManager.psc`:
- Around line 65-100: InitWidgetLoader currently bails if both UI.InvokeString
loadMovie attempts fail; change it to perform a bounded retry instead: in
InitWidgetLoader (reference symbols: InitWidgetLoader, UI.InvokeString,
UI.GetInt, HUD_MENU) implement a short retry loop (e.g. 3 attempts) with a small
delay (Utility.Wait or RegisterForSingleUpdate) that re-runs the loadMovie calls
and re-reads _global.WidgetLoader.SKYUI_RELEASE_IDX before returning; only
return after exhausting retries, and if it eventually succeeds send the existing
SKIWF_widgetManagerReady notification (or proceed with the normal success path);
additionally ensure the code re-registers for the HUD/menu open event if you
rely on menu callbacks so the loader will try again later if HUD was still
coming up.
- Around line 115-129: Both event handlers (OnWidgetLoad and OnWidgetError)
index _widgets using IDs from the SWF without validating them; add a
bounds-and-null check before any _widgets[widgetID] access. In OnWidgetLoad
(where widgetID is obtained via "int widgetID = a_strArg as int") and
OnWidgetError (where widgetID comes from a_numArg), validate that widgetID is >=
0 and < _widgets.Length and that _widgets[widgetID] != none; only then call
client.OnWidgetLoad() or use the widget for logging. If the ID is invalid or the
cast fails, avoid indexing and emit a safe fallback Debug.Trace that includes
the raw widgetID (or arg) and errorType so you can recover without causing an
out‑of‑bounds access.
In `@Source/Scripts/Stubs/JContainers/JMap.psc`:
- Line 5: The getStr stub currently declares a String default parameter which
breaks callers that pass ints/floats; update the stub for function getStr(Int
object, String key, String default="") to accept a flexible default type (e.g.
change the default parameter to a Variant/Any type or provide overloads allowing
Int and Float defaults) so the signature matches actual JContainers usage;
ensure the native/global modifiers remain and update any dependent declarations
to use the new parameter type to preserve compatibility.
In `@Source/Scripts/Stubs/SKSE/UI.psc`:
- Around line 16-18: The current stub for Function Invoke(string menuName,
string target) incorrectly calls InvokeBool(menuName, target, false) which adds
an extra boolean argument; change the stub to match the real SKSE signature by
declaring Function Invoke(...) as native (i.e., make Invoke a native function
instead of a wrapper) so it invokes Scaleform with no added arguments; update
the declaration for Invoke in this file (and any related declarations) to be
native and remove the wrapper call to InvokeBool to ensure behavior matches
UI.Invoke.
In `@Source/Scripts/Stubs/SKSE/Utility.psc`:
- Around line 6-7: Update the declaration for the SKSE Papyrus Utility.Wait
function by removing the incorrect float return type so it is declared as a void
(i.e. match the pattern used by WaitMenuMode); locate the `Function Wait(float
afSeconds) native global` declaration (symbol: Wait) in
Source/Scripts/Stubs/SKSE/Utility.psc and change it to the no-return form
consistent with the SKSE API.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 45e9d744-1009-4740-8524-395a2a6ee8f6
📒 Files selected for processing (44)
.agents/workflows/build.md.github/workflows/build.yml.gitignoreREADME.mdScripts/FSMPM.pexScripts/FSMPMPlayerScript.pexSource/Scripts/FSMPM.pscSource/Scripts/SkyUI_SDK/SKI_ActiveEffectsWidget.pscSource/Scripts/SkyUI_SDK/SKI_ConfigBase.pscSource/Scripts/SkyUI_SDK/SKI_ConfigManager.pscSource/Scripts/SkyUI_SDK/SKI_ConfigMenu.pscSource/Scripts/SkyUI_SDK/SKI_FavoritesManager.pscSource/Scripts/SkyUI_SDK/SKI_Main.pscSource/Scripts/SkyUI_SDK/SKI_PlayerLoadGameAlias.pscSource/Scripts/SkyUI_SDK/SKI_QF_ConfigManagerInstance.pscSource/Scripts/SkyUI_SDK/SKI_QuestBase.pscSource/Scripts/SkyUI_SDK/SKI_SettingsManager.pscSource/Scripts/SkyUI_SDK/SKI_WidgetBase.pscSource/Scripts/SkyUI_SDK/SKI_WidgetManager.pscSource/Scripts/Stubs/BaseGame/Actor.pscSource/Scripts/Stubs/BaseGame/Alias.pscSource/Scripts/Stubs/BaseGame/Debug.pscSource/Scripts/Stubs/BaseGame/Faction.pscSource/Scripts/Stubs/BaseGame/Form.pscSource/Scripts/Stubs/BaseGame/GlobalVariable.pscSource/Scripts/Stubs/BaseGame/Keyword.pscSource/Scripts/Stubs/BaseGame/ObjectReference.pscSource/Scripts/Stubs/BaseGame/Quest.pscSource/Scripts/Stubs/BaseGame/ReferenceAlias.pscSource/Scripts/Stubs/BaseGame/TESV_Papyrus_Flags.flgSource/Scripts/Stubs/ConsoleUtil/ConsoleUtil.pscSource/Scripts/Stubs/JContainers/JMap.pscSource/Scripts/Stubs/JContainers/JValue.pscSource/Scripts/Stubs/PapyrusUtil/JsonUtil.pscSource/Scripts/Stubs/PapyrusUtil/MiscUtil.pscSource/Scripts/Stubs/PapyrusUtil/PapyrusUtil.pscSource/Scripts/Stubs/SKSE/StringUtil.pscSource/Scripts/Stubs/SKSE/UI.pscSource/Scripts/Stubs/SKSE/Utility.pscdev-scripts/compile.ps1dev-scripts/setup_ci.ps1skyrimse.dev.ppjskyrimse.ppjskyrimse_ci.ppj
Contributor
Author
|
@coderabbitai -> update your review with the latest changes; Resolve all comments in Source/Scripts/SkyUI_SDK as that's a third party MIT licenses sdk that is only included to make CI reliable |
Contributor
🧩 Analysis chain
🏁 Script executed: Length of output: 4725 🏁 Script executed: Length of output: 1600 🏁 Script executed: Length of output: 306
The threads to resolve are all on these files:
I already have this recorded in long-term memory so I won't raise issues for those files in future reviews. I'll now trigger a fresh review of the latest changes (excluding Please exclude all files under 🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
skyrimse.dev.ppj (1)
3-4: 🧹 Nitpick | 🔵 TrivialAdd guidance comment for the Flags path.
The
Flagsattribute at line 4 uses a hardcoded path without the "ADJUST THIS PATH" comment that other paths (lines 25-31) have. This inconsistency could cause confusion for developers setting up their local environment.📝 Suggested improvement
<PapyrusProject xmlns="PapyrusProject.xsd" - Flags="C:\Games\Skyrim Special Edition\Data\Source\Scripts\TESV_Papyrus_Flags.flg" + Flags="C:\Games\Skyrim Special Edition\Data\Source\Scripts\TESV_Papyrus_Flags.flg" <!-- ADJUST THIS PATH --> Game="sse"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skyrimse.dev.ppj` around lines 3 - 4, The Flags attribute on the PapyrusProject element is using a hardcoded absolute path without the same "ADJUST THIS PATH" guidance present for other path attributes; update the Flags attribute (the Flags="...TESV_Papyrus_Flags.flg" entry) to include the same developer guidance comment (e.g., add an "ADJUST THIS PATH" inline comment or surrounding XML comment) so it's consistent with the other paths and makes local setup expectations clear.README.md (1)
3-3:⚠️ Potential issue | 🟡 MinorUse the canonical repository in the badge URL.
The badge still tracks the fork, so the main README can show the wrong pipeline state after merge.
🪪 Proposed fix
- +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 3, The build badge in README.md currently points to the forked repo URL (the image line containing "https://github.com/KaninHop/FSMP-MCM/actions/workflows/build.yml/badge.svg"); update that URL to the canonical repository's owner/repo in the badge markdown so the README shows the upstream pipeline status (replace "KaninHop/FSMP-MCM" with the canonical owner/repository name in the badge image and link).dev-scripts/setup_ci.ps1 (1)
33-34:⚠️ Potential issue | 🟠 MajorValidate the downloaded Caprica artifact before copying it.
This still trusts the ZIP contents and pipes a possibly empty
Get-ChildItemresult straight intoCopy-Item. Please pin the expected checksum and fail explicitly whenCaprica.exeis missing instead of surfacing a generic pipeline error.🔒 Proposed hardening
$capricaPath = Download-And-Extract "https://github.com/KrisV-777/Caprica/releases/download/0.3.0a/Caprica.zip" "Caprica" -Get-ChildItem -Path $capricaPath -Filter "Caprica.exe" -Recurse | Select-Object -First 1 | Copy-Item -Destination (Join-Path $toolDir "Caprica.exe") +$expectedCapricaSha256 = "<pin Caprica.zip sha256>" +$actualCapricaSha256 = (Get-FileHash -Path (Join-Path $env:TEMP "Caprica.zip") -Algorithm SHA256).Hash +if ($actualCapricaSha256 -ne $expectedCapricaSha256) { + throw "Caprica.zip checksum mismatch." +} + +$capricaExe = Get-ChildItem -Path $capricaPath -Filter "Caprica.exe" -Recurse | Select-Object -First 1 +if (-not $capricaExe) { + throw "Caprica.exe not found after extraction." +} + +Copy-Item -LiteralPath $capricaExe.FullName -Destination (Join-Path $toolDir "Caprica.exe") -Force🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/setup_ci.ps1` around lines 33 - 34, The script currently pipes the possibly-empty Get-ChildItem result into Copy-Item after Download-And-Extract; instead, verify the downloaded artifact and the extracted Caprica.exe explicitly: after calling Download-And-Extract (keep the call to Download-And-Extract), compute and compare the expected checksum (pin the expected SHA256) of the downloaded ZIP or the extracted Caprica.exe, then use Get-ChildItem to find Caprica.exe (from the same logic) and check that the file exists before calling Copy-Item; if the checksum mismatches or Caprica.exe is not found, throw/Write-Error with a clear message and exit non-zero rather than allowing a generic pipeline error. Ensure references to Download-And-Extract, Get-ChildItem, Copy-Item, Caprica.exe and $toolDir are used so you update the correct variables and checks.dev-scripts/compile.ps1 (2)
129-134:⚠️ Potential issue | 🟠 MajorClean stale
.pexoutputs before compiling.Line 129–134 still only ensures the output directory exists. If scripts are removed/renamed, stale binaries can leak into dev deploys.
Suggested fix
# Ensure output directory exists if (-not (Test-Path $OutputDir)) { - New-Item -ItemType Directory -Path $OutputDir -Force + New-Item -ItemType Directory -Path $OutputDir -Force | Out-Null +} else { + Get-ChildItem -Path $OutputDir -Filter "*.pex" -File -ErrorAction SilentlyContinue | Remove-Item -Force }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/compile.ps1` around lines 129 - 134, The script currently only ensures $OutputDir exists but doesn't remove stale .pex files; modify the compile flow to delete any existing .pex files in $OutputDir before compiling (e.g., remove files matching *.pex), then proceed to create the directory and collect source files via Get-ChildItem "Source/Scripts/*.psc" (excluding FSMPM_AutoBindings.psc); ensure the removal step runs safely (only target $OutputDir and optionally check it exists) so renamed/removed scripts cannot leave stale binaries.
16-31:⚠️ Potential issue | 🟠 MajorCaprica auto-download source remains questionable and brittle.
Line 16 still depends on the previously flagged
KrisV-777/Capricarelease URL. If unavailable or unofficial, bootstrap fails or weakens supply-chain trust.Use this read-only check to validate endpoint/repo availability and compare with upstream release metadata:
#!/bin/bash set -euo pipefail echo "== Check configured download URL ==" curl -sSI "https://github.com/KrisV-777/Caprica/releases/download/0.3.0a/Caprica.zip" | head -n 5 echo echo "== Check configured repository metadata ==" curl -sS "https://api.github.com/repos/KrisV-777/Caprica" | jq -r '.full_name // .message' echo echo "== Check upstream Caprica latest release ==" curl -sS "https://api.github.com/repos/Orvid/Caprica/releases/latest" | jq -r '.tag_name // .message'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/compile.ps1` around lines 16 - 31, The script currently hardcodes $CapricaUrl and $ExpectedHash which is brittle and points to an unofficial repo; change the logic in the bootstrap block (the variables $CapricaUrl, $ExpectedHash and the download via Invoke-WebRequest/$zipFile) to dynamically validate and/or fetch the upstream release from the official Orvid/Caprica GitHub release API: use Invoke-RestMethod to query "repos/Orvid/Caprica/releases/latest", derive the asset download URL and tag, compare the tag against your configured version, fetch the release asset to $zipFile, then compute and verify its checksum instead of trusting the hardcoded $ExpectedHash; also add a clear fallback or error path if the upstream API or asset URL is missing so the install fails safe rather than silently using an unofficial source.Source/Scripts/Stubs/JContainers/JMap.psc (1)
5-5:⚠️ Potential issue | 🟠 Major
getStrdefault parameter type still looks too restrictive.Line 5 keeps
String default=""; if current callsites still pass numeric defaults, this stub signature can break CI compilation. This appears to be the same unresolved concern raised earlier.Run this to confirm callsite argument types against the current stub definition:
#!/bin/bash set -euo pipefail echo "== JMap.getStr stub definition ==" cat -n Source/Scripts/Stubs/JContainers/JMap.psc | sed -n '1,20p' echo echo "== JMap.getStr callsites with context ==" rg -n --type-add 'psc:*.psc' --type psc -C2 '\bJMap\.getStr\s*\('If non-string third arguments are present, update the stub to accept a flexible type (for example
Var) to match real usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/Scripts/Stubs/JContainers/JMap.psc` at line 5, The stub for JMap.getStr currently declares the third parameter as String default="" which is too restrictive for callsites that pass numbers; update the signature in JMap.psc for the function getStr(Int object, String key, String default="") to accept a flexible type (e.g., replace the third parameter type with Var and adjust the default accordingly) so the stub matches real usage and CI compilation; locate the declaration of getStr in Source/Scripts/Stubs/JContainers/JMap.psc and change the parameter type to Var (and ensure any related native declaration remains consistent).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/workflows/build.md:
- Around line 11-12: Update the Caprica download reference in the "Caprica
Compiler: `Caprica/Caprica.exe`" line to point to the KrisV-777 fork used by CI
(KrisV-777/Caprica, v0.3.0a) instead of Orvid/Caprica so builders fetch the same
compiler version as README.md and dev-scripts/setup_ci.ps1; replace the current
Orvid/Caprica/releases URL with the KrisV-777/Caprica release URL and mention
version 0.3.0a for clarity.
In `@dev-scripts/setup_ci.ps1`:
- Around line 8-10: The script currently sets $rootDir using Get-Location which
depends on the caller's CWD; change root resolution to derive the repository
root from the script location instead (e.g. use $PSScriptRoot or Split-Path on
$MyInvocation.MyCommand.Definition) and then recompute $toolDir from that root
($toolDir = Join-Path $rootDir "ci_tools") so Caprica.exe lookups no longer
depend on where the script was invoked; update the assignment to $rootDir and
ensure $toolDir uses that new value.
In `@Source/Scripts/Stubs/BaseGame/Alias.psc`:
- Around line 24-49: Alias.psc is missing the Event OnPlayerLoadGame stub
declared elsewhere (e.g. SKI_PlayerLoadGameAlias.psc), so add a matching empty
event declaration Event OnPlayerLoadGame() EndEvent to the base alias stub
chain; locate the Event blocks in Alias.psc (and mirror the same addition in
ReferenceAlias.psc if present) so the base alias classes expose the
OnPlayerLoadGame event for CI and consistent handling.
- Around line 4-23: The Alias stub is missing SKSE mod-event methods which
prevents scripts from compiling; add the SKSE Alias members RegisterForModEvent
and SendModEvent to match the Form.psc SKSE surface. Specifically, declare a
RegisterForModEvent method (e.g., RegisterForModEvent(string asEventName, string
asSender) native) and the SendModEvent method(s) used by SKSE (e.g.,
SendModEvent(string asEventName, string asArg, string asSender) native, and an
overload with fewer args if Form.psc provides one) so alias-based scripts can
register and dispatch mod events.
In `@Source/Scripts/Stubs/BaseGame/Form.psc`:
- Around line 5-31: The Form.psc stub is missing SKSE menu APIs required by
SkyUI scripts; add native declarations for RegisterForMenu(string menuName,
string callbackName) and UnregisterForMenu(string menuName) as well as the menu
callbacks OnMenuOpen(string menuName) native and OnMenuClose(string menuName)
native to the Form class so calls from SKI_Main.psc, SKI_ConfigManager.psc,
SKI_WidgetManager.psc, SKI_SettingsManager.psc and SKI_FavoritesManager.psc
resolve; ensure the signatures exactly match those names so subclasses like
Quest.psc inherit them.
In `@Source/Scripts/Stubs/BaseGame/GlobalVariable.psc`:
- Around line 4-6: The stub for GlobalVariable.Mod is missing its float return
type; update the declaration for Mod (the GlobalVariable.Mod function) to return
float (i.e., change the signature from a void/unnamed return to a float return)
so it matches the Papyrus API and CI contract—ensure the signature reads as a
float-returning function and keep the parameter name afHowMuch intact and the
native specifier.
---
Duplicate comments:
In `@dev-scripts/compile.ps1`:
- Around line 129-134: The script currently only ensures $OutputDir exists but
doesn't remove stale .pex files; modify the compile flow to delete any existing
.pex files in $OutputDir before compiling (e.g., remove files matching *.pex),
then proceed to create the directory and collect source files via Get-ChildItem
"Source/Scripts/*.psc" (excluding FSMPM_AutoBindings.psc); ensure the removal
step runs safely (only target $OutputDir and optionally check it exists) so
renamed/removed scripts cannot leave stale binaries.
- Around line 16-31: The script currently hardcodes $CapricaUrl and
$ExpectedHash which is brittle and points to an unofficial repo; change the
logic in the bootstrap block (the variables $CapricaUrl, $ExpectedHash and the
download via Invoke-WebRequest/$zipFile) to dynamically validate and/or fetch
the upstream release from the official Orvid/Caprica GitHub release API: use
Invoke-RestMethod to query "repos/Orvid/Caprica/releases/latest", derive the
asset download URL and tag, compare the tag against your configured version,
fetch the release asset to $zipFile, then compute and verify its checksum
instead of trusting the hardcoded $ExpectedHash; also add a clear fallback or
error path if the upstream API or asset URL is missing so the install fails safe
rather than silently using an unofficial source.
In `@dev-scripts/setup_ci.ps1`:
- Around line 33-34: The script currently pipes the possibly-empty Get-ChildItem
result into Copy-Item after Download-And-Extract; instead, verify the downloaded
artifact and the extracted Caprica.exe explicitly: after calling
Download-And-Extract (keep the call to Download-And-Extract), compute and
compare the expected checksum (pin the expected SHA256) of the downloaded ZIP or
the extracted Caprica.exe, then use Get-ChildItem to find Caprica.exe (from the
same logic) and check that the file exists before calling Copy-Item; if the
checksum mismatches or Caprica.exe is not found, throw/Write-Error with a clear
message and exit non-zero rather than allowing a generic pipeline error. Ensure
references to Download-And-Extract, Get-ChildItem, Copy-Item, Caprica.exe and
$toolDir are used so you update the correct variables and checks.
In `@README.md`:
- Line 3: The build badge in README.md currently points to the forked repo URL
(the image line containing
"https://github.com/KaninHop/FSMP-MCM/actions/workflows/build.yml/badge.svg");
update that URL to the canonical repository's owner/repo in the badge markdown
so the README shows the upstream pipeline status (replace "KaninHop/FSMP-MCM"
with the canonical owner/repository name in the badge image and link).
In `@skyrimse.dev.ppj`:
- Around line 3-4: The Flags attribute on the PapyrusProject element is using a
hardcoded absolute path without the same "ADJUST THIS PATH" guidance present for
other path attributes; update the Flags attribute (the
Flags="...TESV_Papyrus_Flags.flg" entry) to include the same developer guidance
comment (e.g., add an "ADJUST THIS PATH" inline comment or surrounding XML
comment) so it's consistent with the other paths and makes local setup
expectations clear.
In `@Source/Scripts/Stubs/JContainers/JMap.psc`:
- Line 5: The stub for JMap.getStr currently declares the third parameter as
String default="" which is too restrictive for callsites that pass numbers;
update the signature in JMap.psc for the function getStr(Int object, String key,
String default="") to accept a flexible type (e.g., replace the third parameter
type with Var and adjust the default accordingly) so the stub matches real usage
and CI compilation; locate the declaration of getStr in
Source/Scripts/Stubs/JContainers/JMap.psc and change the parameter type to Var
(and ensure any related native declaration remains consistent).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 7bd3016b-a26d-454f-a312-fbd012aaf550
📒 Files selected for processing (45)
.agents/workflows/build.md.github/workflows/build.yml.gitignore.vscode/tasks.jsonREADME.mdScripts/FSMPM.pexScripts/FSMPMPlayerScript.pexSource/Scripts/FSMPM.pscSource/Scripts/SkyUI_SDK/SKI_ActiveEffectsWidget.pscSource/Scripts/SkyUI_SDK/SKI_ConfigBase.pscSource/Scripts/SkyUI_SDK/SKI_ConfigManager.pscSource/Scripts/SkyUI_SDK/SKI_ConfigMenu.pscSource/Scripts/SkyUI_SDK/SKI_FavoritesManager.pscSource/Scripts/SkyUI_SDK/SKI_Main.pscSource/Scripts/SkyUI_SDK/SKI_PlayerLoadGameAlias.pscSource/Scripts/SkyUI_SDK/SKI_QF_ConfigManagerInstance.pscSource/Scripts/SkyUI_SDK/SKI_QuestBase.pscSource/Scripts/SkyUI_SDK/SKI_SettingsManager.pscSource/Scripts/SkyUI_SDK/SKI_WidgetBase.pscSource/Scripts/SkyUI_SDK/SKI_WidgetManager.pscSource/Scripts/Stubs/BaseGame/Actor.pscSource/Scripts/Stubs/BaseGame/Alias.pscSource/Scripts/Stubs/BaseGame/Debug.pscSource/Scripts/Stubs/BaseGame/Faction.pscSource/Scripts/Stubs/BaseGame/Form.pscSource/Scripts/Stubs/BaseGame/GlobalVariable.pscSource/Scripts/Stubs/BaseGame/Keyword.pscSource/Scripts/Stubs/BaseGame/ObjectReference.pscSource/Scripts/Stubs/BaseGame/Quest.pscSource/Scripts/Stubs/BaseGame/ReferenceAlias.pscSource/Scripts/Stubs/BaseGame/TESV_Papyrus_Flags.flgSource/Scripts/Stubs/ConsoleUtil/ConsoleUtil.pscSource/Scripts/Stubs/JContainers/JMap.pscSource/Scripts/Stubs/JContainers/JValue.pscSource/Scripts/Stubs/PapyrusUtil/JsonUtil.pscSource/Scripts/Stubs/PapyrusUtil/MiscUtil.pscSource/Scripts/Stubs/PapyrusUtil/PapyrusUtil.pscSource/Scripts/Stubs/SKSE/StringUtil.pscSource/Scripts/Stubs/SKSE/UI.pscSource/Scripts/Stubs/SKSE/Utility.pscdev-scripts/compile.ps1dev-scripts/setup_ci.ps1skyrimse.dev.ppjskyrimse.ppjskyrimse_ci.ppj
* Switch to submodule for skyui, move stubs to dependencies folder
…op/FSMP-MCM into Improve-Builds-and-Add-CI
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
README.md (2)
43-45:⚠️ Potential issue | 🟡 MinorAdd blank line before fenced code block.
The fenced code block starting at line 44 needs a blank line before it to satisfy markdownlint MD031. This was flagged in a previous review but remains unresolved.
📝 Proposed fix for MD031 compliance
2. Run the desired mode: + ```powershell🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 43 - 45, The fenced code block marker "```powershell" in README.md lacks a blank line before it (MD031); insert a single empty line immediately above the "```powershell" fence (the fenced code block starting at the line containing "```powershell") so the block is separated from the preceding paragraph and satisfies markdownlint MD031.
60-62:⚠️ Potential issue | 🟡 MinorAdd blank line before fenced code block.
The fenced code block starting at line 61 needs a blank line before it to satisfy markdownlint MD031. This was flagged in a previous review but remains unresolved.
📝 Proposed fix for MD031 compliance
You can run Caprica directly against them: + ```powershell🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 60 - 62, Add a blank line immediately before the fenced code block that starts with ```powershell containing ./Caprica/Caprica.exe skyrimse.ppj in README.md to satisfy markdownlint MD031; locate the triple-backtick fence and insert one empty line above it so the block is separated from the preceding paragraph.Dependencies/Stubs/BaseGame/GlobalVariable.psc (1)
4-6:⚠️ Potential issue | 🟠 MajorReturn
floatfromGlobalVariable.Mod.The Papyrus
GlobalVariable.Modfunction returns the updated value as a float. Line 6 declares it without a return type, which breaks the API contract and will fail for any code that reads the result. This issue was previously flagged and remains unresolved.🔧 Proposed fix
Function SetValue(float afNewValue) native float Function GetValue() native -Function Mod(float afHowMuch) native +float Function Mod(float afHowMuch) native🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dependencies/Stubs/BaseGame/GlobalVariable.psc` around lines 4 - 6, The Mod declaration on GlobalVariable is missing its return type: update the declaration of the Mod function (GlobalVariable.Mod) so it returns a float (matching the Papyrus API) rather than being void; ensure the signature mirrors GetValue (i.e., Mod accepts the float parameter afHowMuch and returns a float) so callers can read the updated value.dev-scripts/setup_ci.ps1 (2)
33-34:⚠️ Potential issue | 🟠 MajorVerify the downloaded Caprica asset before trusting it.
Line 33 downloads an executable archive, and Line 34 copies the first
Caprica.exeit finds without hash verification or an explicit "not found" check. That is both a supply-chain gap and an opaque failure mode if the release asset layout changes. Please pin the checksum from the exact release asset you intend to trust.Hardening patch
-$capricaPath = Download-And-Extract "https://github.com/KrisV-777/Caprica/releases/download/0.3.0a/Caprica.zip" "Caprica" -Get-ChildItem -Path $capricaPath -Filter "Caprica.exe" -Recurse | Select-Object -First 1 | Copy-Item -Destination (Join-Path $toolDir "Caprica.exe") +$capricaUrl = "https://github.com/KrisV-777/Caprica/releases/download/0.3.0a/Caprica.zip" +$capricaZip = Join-Path $env:TEMP "Caprica.zip" +$capricaPath = Download-And-Extract $capricaUrl "Caprica" +$expectedCapricaSha256 = "<pin-release-sha256>" +$actualCapricaSha256 = (Get-FileHash -Path $capricaZip -Algorithm SHA256).Hash +if ($expectedCapricaSha256 -ne "<pin-release-sha256>" -and $actualCapricaSha256 -ne $expectedCapricaSha256) { + throw "Caprica.zip checksum mismatch. Expected $expectedCapricaSha256, got $actualCapricaSha256." +} + +$capricaExe = Get-ChildItem -Path $capricaPath -Filter "Caprica.exe" -Recurse | Select-Object -First 1 +if (-not $capricaExe) { + throw "Caprica.exe not found after extraction from $capricaUrl" +} + +Copy-Item -LiteralPath $capricaExe.FullName -Destination (Join-Path $toolDir "Caprica.exe") -Force🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/setup_ci.ps1` around lines 33 - 34, Download-And-Extract currently fetches and extracts Caprica.zip to $capricaPath and the script blindly copies the first Caprica.exe found; you should pin and verify the exact release asset checksum and fail early if verification or file discovery fails. Modify the flow around Download-And-Extract/$capricaPath to accept or compute a known checksum for the exact release asset, verify the extracted file(s) against that checksum before copying, and replace the unguarded Get-ChildItem | Select-Object -First 1 | Copy-Item sequence with logic that explicitly locates the expected Caprica.exe, errors if not found, and only copies after successful checksum validation to $toolDir (reference symbols: Download-And-Extract, $capricaPath, Get-ChildItem, Caprica.exe, Copy-Item, $toolDir).
8-9:⚠️ Potential issue | 🟡 MinorResolve the repo root from
$PSScriptRoot.Using
Get-Locationon Line 8 makesci_toolsdepend on the caller’s CWD. Invoking this script from anywhere but the repository root writes the compiler to the wrong place and breaks later./ci_tools/Caprica.exelookups. Please sanity-check this once from the repo root and once from a child directory.Minimal fix
-$rootDir = Get-Location +$rootDir = (Resolve-Path (Join-Path $PSScriptRoot "..")).Path $toolDir = Join-Path $rootDir "ci_tools"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/setup_ci.ps1` around lines 8 - 9, The script uses Get-Location to set $rootDir which makes $toolDir and the ci_tools/Caprica.exe path depend on the caller's CWD; replace using $PSScriptRoot as the repository root instead so $rootDir = $PSScriptRoot (or resolve $PSScriptRoot to its parent if this script lives in a subfolder), then compute $toolDir from that root so ci_tools and Caprica.exe are always written/looked up relative to the repo root; update any related references that build $toolDir to use the new $rootDir variable and sanity-check by running the script from the repo root and from a child directory.Dependencies/Stubs/JContainers/JMap.psc (1)
5-5:⚠️ Potential issue | 🟠 MajorVerify
JMap.getStr's third parameter against real usage.Line 5 fixes
defaulttoString, but Lines 95-107 inSource/Scripts/FSMPM.pscpass numeric defaults like3.5,40, and0.2. If this stub is narrower than the real JContainers declaration, CI will reject valid script code. Please confirm the stub against the actual JContainers declaration before merging.Alignment options
- Mirror the real JContainers declaration in the stub.
- Or update the callers to pass string defaults consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dependencies/Stubs/JContainers/JMap.psc` at line 5, The stub for JMap.getStr currently declares the third parameter as String but callers in Source/Scripts/FSMPM.psc (around the code passing 3.5, 40, 0.2) pass numeric defaults; verify the real JContainers declaration and update this stub to exactly match it (e.g., change the third parameter type or add an overload/variant signature) so the compiler accepts numeric defaults, or alternatively update the FSMPM.psc callers to pass string defaults consistently; locate and modify the JMap.getStr declaration in Dependencies/Stubs/JContainers/JMap.psc (and adjust FSMPM.psc callers if you choose that route).Dependencies/Stubs/BaseGame/Alias.psc (1)
4-49:⚠️ Potential issue | 🟠 MajorAlias stub still appears to miss SKSE/engine parity members flagged earlier.
Compared to the prior unresolved findings,
Aliasstill does not declare mod-event members andOnPlayerLoadGame, which can leave CI stubs incomplete for alias-based consumers.#!/bin/bash set -euo pipefail echo "Inspect Alias/ReferenceAlias/Form stubs:" fd -i 'Alias.psc|ReferenceAlias.psc|Form.psc' Dependencies/Stubs | sed 's#^# - #' echo echo "Alias members currently declared:" cat -n Dependencies/Stubs/BaseGame/Alias.psc | sed -n '1,120p' echo echo "Check for SKSE/engine parity members in Alias chain:" rg -nP 'RegisterForModEvent|UnregisterForModEvent|SendModEvent|OnPlayerLoadGame' Dependencies/Stubs/BaseGame/Alias.psc Dependencies/Stubs/BaseGame/ReferenceAlias.psc Dependencies/Stubs/BaseGame/Form.psc echo echo "Check non-third-party scripts for Alias-side usage expectations:" rg -nP --iglob '*.psc' 'RegisterForModEvent\s*\(|SendModEvent\s*\(|Event\s+OnPlayerLoadGame\s*\(' Source/Scripts -g '!Source/Scripts/SkyUI_SDK/**'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dependencies/Stubs/BaseGame/Alias.psc` around lines 4 - 49, Alias.psc is missing SKSE/engine parity members used elsewhere (RegisterForModEvent, UnregisterForModEvent, SendModEvent and the OnPlayerLoadGame event), causing alias-based consumers and CI stubs to be incomplete; update Dependencies/Stubs/BaseGame/Alias.psc to declare the missing native functions RegisterForModEvent(string asEventName, string asFilter) native, UnregisterForModEvent(string asEventName, string asFilter) native and SendModEvent(string asEventName, string asArg, int aiNum) native (match signatures used in project) and add the Event OnPlayerLoadGame() EndEvent declaration so callers that expect these symbols compile and tests/stubs have parity with ReferenceAlias/Form usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 59-70: The CI build script duplicates packaging of Source/Scripts
(Copy-Item to $buildDir/Source/Scripts) but the skyrimse_ci.ppj manifest and
dev-scripts/compile.ps1 don’t include those files, causing divergence; pick a
single package manifest and align all paths: either remove the Source/Scripts
Copy-Item entries from the workflow (so packaging follows skyrimse_ci.ppj) or
add equivalent Source/Scripts entries to skyrimse_ci.ppj (the list around lines
29–35) and ensure dev-scripts/compile.ps1 uses that same manifest, updating
references to the manifest path if needed so CI and local Release builds use one
source of truth.
In @.gitmodules:
- Around line 1-6: The two duplicate submodules
Source/Scripts/Stubs/SkyUI-Community and Dependencies/SkyUI-Community both point
to https://github.com/doodlum/SkyUI-Community.git; either document why both are
required (e.g., “stub” vs runtime) in .gitmodules and the repo README, or
consolidate to a single submodule (choose one of the two paths) and update all
references in build scripts and code to use that single path, adding a symlink
or a build-time copy step to provide the other path if necessary; ensure you
update .gitmodules and any submodule initialisation instructions and verify git
submodule update --init works after the change.
In `@Dependencies/Stubs/PapyrusUtil/JsonUtil.psc`:
- Around line 1-2: Add minimal stub declarations for the JMap API inside the
JsonUtil script to stop unresolved symbol errors: declare a JMap object()
function and stubbed set/get string functions (e.g., JMap setStr(JMap map,
string key, string value) and string getStr(JMap map, string key) or equivalent
signatures used by callers) with empty bodies so calls to JMap.object(),
JMap.setStr(), and JMap.getStr() compile; place these stubs in the existing
JsonUtil script (scriptname JsonUtil Hidden) using the same symbol names so
FSMPM.psc references resolve.
In `@Dependencies/Stubs/PapyrusUtil/MiscUtil.psc`:
- Line 2: The file uses a lowercase declaration "scriptname MiscUtil Hidden";
update the Papyrus header to use standard capitalization by changing the
declaration to "Scriptname MiscUtil Hidden" so the Scriptname token follows
community conventions (locate the header line containing
scriptname/MiscUtil/Hidden and replace the lowercase token).
---
Duplicate comments:
In `@Dependencies/Stubs/BaseGame/Alias.psc`:
- Around line 4-49: Alias.psc is missing SKSE/engine parity members used
elsewhere (RegisterForModEvent, UnregisterForModEvent, SendModEvent and the
OnPlayerLoadGame event), causing alias-based consumers and CI stubs to be
incomplete; update Dependencies/Stubs/BaseGame/Alias.psc to declare the missing
native functions RegisterForModEvent(string asEventName, string asFilter)
native, UnregisterForModEvent(string asEventName, string asFilter) native and
SendModEvent(string asEventName, string asArg, int aiNum) native (match
signatures used in project) and add the Event OnPlayerLoadGame() EndEvent
declaration so callers that expect these symbols compile and tests/stubs have
parity with ReferenceAlias/Form usage.
In `@Dependencies/Stubs/BaseGame/GlobalVariable.psc`:
- Around line 4-6: The Mod declaration on GlobalVariable is missing its return
type: update the declaration of the Mod function (GlobalVariable.Mod) so it
returns a float (matching the Papyrus API) rather than being void; ensure the
signature mirrors GetValue (i.e., Mod accepts the float parameter afHowMuch and
returns a float) so callers can read the updated value.
In `@Dependencies/Stubs/JContainers/JMap.psc`:
- Line 5: The stub for JMap.getStr currently declares the third parameter as
String but callers in Source/Scripts/FSMPM.psc (around the code passing 3.5, 40,
0.2) pass numeric defaults; verify the real JContainers declaration and update
this stub to exactly match it (e.g., change the third parameter type or add an
overload/variant signature) so the compiler accepts numeric defaults, or
alternatively update the FSMPM.psc callers to pass string defaults consistently;
locate and modify the JMap.getStr declaration in
Dependencies/Stubs/JContainers/JMap.psc (and adjust FSMPM.psc callers if you
choose that route).
In `@dev-scripts/setup_ci.ps1`:
- Around line 33-34: Download-And-Extract currently fetches and extracts
Caprica.zip to $capricaPath and the script blindly copies the first Caprica.exe
found; you should pin and verify the exact release asset checksum and fail early
if verification or file discovery fails. Modify the flow around
Download-And-Extract/$capricaPath to accept or compute a known checksum for the
exact release asset, verify the extracted file(s) against that checksum before
copying, and replace the unguarded Get-ChildItem | Select-Object -First 1 |
Copy-Item sequence with logic that explicitly locates the expected Caprica.exe,
errors if not found, and only copies after successful checksum validation to
$toolDir (reference symbols: Download-And-Extract, $capricaPath, Get-ChildItem,
Caprica.exe, Copy-Item, $toolDir).
- Around line 8-9: The script uses Get-Location to set $rootDir which makes
$toolDir and the ci_tools/Caprica.exe path depend on the caller's CWD; replace
using $PSScriptRoot as the repository root instead so $rootDir = $PSScriptRoot
(or resolve $PSScriptRoot to its parent if this script lives in a subfolder),
then compute $toolDir from that root so ci_tools and Caprica.exe are always
written/looked up relative to the repo root; update any related references that
build $toolDir to use the new $rootDir variable and sanity-check by running the
script from the repo root and from a child directory.
In `@README.md`:
- Around line 43-45: The fenced code block marker "```powershell" in README.md
lacks a blank line before it (MD031); insert a single empty line immediately
above the "```powershell" fence (the fenced code block starting at the line
containing "```powershell") so the block is separated from the preceding
paragraph and satisfies markdownlint MD031.
- Around line 60-62: Add a blank line immediately before the fenced code block
that starts with ```powershell containing ./Caprica/Caprica.exe skyrimse.ppj in
README.md to satisfy markdownlint MD031; locate the triple-backtick fence and
insert one empty line above it so the block is separated from the preceding
paragraph.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 03b72aea-3dde-4ab6-b475-f77af3c4cd83
📒 Files selected for processing (28)
.github/workflows/build.yml.gitmodulesDependencies/SkyUI-CommunityDependencies/Stubs/BaseGame/Actor.pscDependencies/Stubs/BaseGame/Alias.pscDependencies/Stubs/BaseGame/Debug.pscDependencies/Stubs/BaseGame/Faction.pscDependencies/Stubs/BaseGame/Form.pscDependencies/Stubs/BaseGame/GlobalVariable.pscDependencies/Stubs/BaseGame/Keyword.pscDependencies/Stubs/BaseGame/ObjectReference.pscDependencies/Stubs/BaseGame/Quest.pscDependencies/Stubs/BaseGame/ReferenceAlias.pscDependencies/Stubs/BaseGame/TESV_Papyrus_Flags.flgDependencies/Stubs/ConsoleUtil/ConsoleUtil.pscDependencies/Stubs/JContainers/JMap.pscDependencies/Stubs/JContainers/JValue.pscDependencies/Stubs/PapyrusUtil/JsonUtil.pscDependencies/Stubs/PapyrusUtil/MiscUtil.pscDependencies/Stubs/PapyrusUtil/PapyrusUtil.pscDependencies/Stubs/SKSE/StringUtil.pscDependencies/Stubs/SKSE/UI.pscDependencies/Stubs/SKSE/Utility.pscREADME.mdSource/Scripts/Stubs/SkyUI-Communitydev-scripts/setup_ci.ps1skyrimse.ppjskyrimse_ci.ppj
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 5-10: YAML contains extra spaces inside square-bracket sequences
(e.g., branches: [ main ]) which fails linting; update the bracketed lists under
the branches and pull_request keys so there are no spaces inside the brackets
(change entries like "[ main ]" to "[main]") and ensure consistent formatting
for tags and other list literals.
In `@Dependencies/SkyUI-Community`:
- Line 1: The submodule entry for Dependencies/SkyUI-Community references a
missing commit 2a111d2417706e80e88f9ad95faa4ebd1056014a; fix by updating the
submodule to point at a valid reachable commit or ensuring the submodule is
initialized from the correct upstream: run git submodule update --init --remote
for Dependencies/SkyUI-Community, verify the upstream repo contains the intended
commit, and if not, change the submodule pointer to the correct commit hash from
the SkyUI-Community upstream (or document the exact upstream commit to pin) and
commit the updated submodule reference so CI can fetch it.
In `@tmp/fsmpm_diff.txt`:
- Around line 112-118: The current flow calls storeConfigAndSmpReset() and
ForcePageReset() even when loadConfigFile(presetFolder + "/" + presetName)
returns false, allowing partial/default values from a failed parse to be
persisted; change the logic so you first call loadConfigFile(...), if it returns
true set sLastLoadedPresetName = presetName and then call
storeConfigAndSmpReset() and ForcePageReset(), otherwise set
sLastLoadedPresetName = "" and do NOT call storeConfigAndSmpReset() or
ForcePageReset(); use the existing symbols loadConfigFile,
sLastLoadedPresetName, storeConfigAndSmpReset, and ForcePageReset to locate and
update the conditional.
- Around line 303-309: The code only sets bLastTagFound=false when the start tag
is missing but never when the end tag is missing, causing false positives;
modify the tag-parsing flow so that after computing endTagIndex (using
findStringInString) you check if endTagIndex is -1 (or not found) and set
bLastTagFound = false and return tagDefaultValue(tag) before continuing; ensure
bLastTagFound is set to true only when both startTagIndex and endTagIndex are
found (so adjust/remove the unreachable bLastTagFound = true placed after the
earlier return).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e686012a-a6ad-42a2-bd63-46f48a71f793
📒 Files selected for processing (3)
.github/workflows/build.ymlDependencies/SkyUI-Communitytmp/fsmpm_diff.txt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
dev-scripts/setup_ci.ps1 (2)
33-35:⚠️ Potential issue | 🟠 MajorHarden Caprica install: validate artifact and fail explicitly if executable is missing.
The current flow trusts the downloaded ZIP and can silently continue if
Caprica.exeis not found.Suggested fix
$capricaPath = Invoke-DownloadAndExtract "https://github.com/KrisV-777/Caprica/releases/download/0.3.0a/Caprica.zip" "Caprica" -Get-ChildItem -Path $capricaPath -Filter "Caprica.exe" -Recurse | Select-Object -First 1 | Copy-Item -Destination (Join-Path $toolDir "Caprica.exe") +$capricaExe = Get-ChildItem -Path $capricaPath -Filter "Caprica.exe" -Recurse | Select-Object -First 1 +if (-not $capricaExe) { + throw "Caprica.exe not found after extraction." +} +# Pin this to the known-good SHA256 for the exact release asset. +$expectedCapricaSha256 = "<sha256>" +$actualCapricaSha256 = (Get-FileHash -LiteralPath $capricaExe.FullName -Algorithm SHA256).Hash +if ($expectedCapricaSha256 -ne "<sha256>" -and $actualCapricaSha256 -ne $expectedCapricaSha256) { + throw "Caprica.exe checksum mismatch." +} +Copy-Item -LiteralPath $capricaExe.FullName -Destination (Join-Path $toolDir "Caprica.exe") -Force Write-Host "Caprica installed."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/setup_ci.ps1` around lines 33 - 35, After calling Invoke-DownloadAndExtract and setting $capricaPath, verify that a Caprica.exe exists before Copy-Item: use Get-ChildItem (same pattern already used) to locate the first Caprica.exe under $capricaPath, if not found write an error via Write-Error/Write-Host and exit with non-zero code (e.g., exit 1); if found, continue to Copy-Item using that resolved file path instead of assuming presence and then log "Caprica installed." This ensures Invoke-DownloadAndExtract, $capricaPath and the Copy-Item step explicitly validate the artifact and fail fast when missing.
8-9:⚠️ Potential issue | 🟡 MinorResolve repository root from script location, not caller CWD.
Line 8 makes execution context-dependent; running this outside repo root can place tools in the wrong directory.
Suggested fix
-$rootDir = Get-Location +$rootDir = (Resolve-Path (Join-Path $PSScriptRoot "..")).Path $toolDir = Join-Path $rootDir "ci_tools"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-scripts/setup_ci.ps1` around lines 8 - 9, The script currently sets $rootDir = Get-Location which uses the caller's CWD; change it to derive the repository root from the script's location instead (use $PSScriptRoot or Split-Path -Parent $MyInvocation.MyCommand.Definition) and then compute $toolDir from that root (the existing $toolDir = Join-Path $rootDir "ci_tools" can remain). Update the assignment to $rootDir in the top of setup_ci.ps1 (replace the Get-Location usage) so tool paths are resolved relative to the script file, not the caller's working directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/workflows/build.md:
- Line 11: Remove the trailing whitespace at the end of the line containing
"**Caprica Compiler**: `Caprica/Caprica.exe`" and ensure the file ends with
exactly one newline character; also check and remove any trailing spaces on the
other occurrence noted (line with the same text around the 41st line) so both
instances have no trailing spaces and the document has a single final newline.
In `@dev-scripts/setup_ci.ps1`:
- Around line 9-34: The Caprica installer places the exe at ci_tools/Caprica.exe
but the build expects Caprica/Caprica.exe; modify setup_ci.ps1 so
Invoke-DownloadAndExtract result ($capricaPath) is copied into a Caprica
subfolder under $toolDir (e.g., ensure the folder Join-Path $toolDir "Caprica"
exists and Copy-Item places the file at Join-Path $toolDir
"Caprica\Caprica.exe") instead of copying straight to ci_tools/Caprica.exe;
update the Copy-Item call that follows $capricaPath (or create the directory
beforehand) so the installed path matches the lookup used by
dev-scripts/compile.ps1.
---
Duplicate comments:
In `@dev-scripts/setup_ci.ps1`:
- Around line 33-35: After calling Invoke-DownloadAndExtract and setting
$capricaPath, verify that a Caprica.exe exists before Copy-Item: use
Get-ChildItem (same pattern already used) to locate the first Caprica.exe under
$capricaPath, if not found write an error via Write-Error/Write-Host and exit
with non-zero code (e.g., exit 1); if found, continue to Copy-Item using that
resolved file path instead of assuming presence and then log "Caprica
installed." This ensures Invoke-DownloadAndExtract, $capricaPath and the
Copy-Item step explicitly validate the artifact and fail fast when missing.
- Around line 8-9: The script currently sets $rootDir = Get-Location which uses
the caller's CWD; change it to derive the repository root from the script's
location instead (use $PSScriptRoot or Split-Path -Parent
$MyInvocation.MyCommand.Definition) and then compute $toolDir from that root
(the existing $toolDir = Join-Path $rootDir "ci_tools" can remain). Update the
assignment to $rootDir in the top of setup_ci.ps1 (replace the Get-Location
usage) so tool paths are resolved relative to the script file, not the caller's
working directory.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5405395c-30de-4612-ab9f-f2f854c5f164
📒 Files selected for processing (2)
.agents/workflows/build.mddev-scripts/setup_ci.ps1
|
|
||
| ## Prerequisites | ||
|
|
||
| - **Caprica Compiler**: `Caprica/Caprica.exe` must be present. |
Contributor
There was a problem hiding this comment.
Fix markdownlint issues (trailing space and final newline).
Line 11 has trailing whitespace, and the file should end with exactly one newline.
Suggested fix
-- **Caprica Compiler**: `Caprica/Caprica.exe` must be present.
+- **Caprica Compiler**: `Caprica/Caprica.exe` must be present.
@@
- Warning `W4007` about `loadConfigDone` being unused is expected and harmless.
+Also applies to: 41-41
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 11-11: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/workflows/build.md at line 11, Remove the trailing whitespace at the
end of the line containing "**Caprica Compiler**: `Caprica/Caprica.exe`" and
ensure the file ends with exactly one newline character; also check and remove
any trailing spaces on the other occurrence noted (line with the same text
around the 41st line) so both instances have no trailing spaces and the document
has a single final newline.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…Add-CI # Conflicts: # Scripts/FSMPM.pex # Scripts/FSMPMPlayerScript.pex
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
2 participants
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.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
Using the generated zip and installing via vortex: (conflict list)
MCM that's the same as master (overwrote a testing version with a big splash screen image)
