Skip to content

Fix active issues and add Unity 6000.5 CI#276

Open
Eli Pinkerton (wallstop) wants to merge 34 commits into
mainfrom
codex/active-issues-ci-6000-5
Open

Fix active issues and add Unity 6000.5 CI#276
Eli Pinkerton (wallstop) wants to merge 34 commits into
mainfrom
codex/active-issues-ci-6000-5

Conversation

@wallstop

@wallstop Eli Pinkerton (wallstop) commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes the currently open issue set gathered for this branch, including Unity 6000.5 object-id compatibility, self-hosted runner provisioning, devcontainer/NPM release hardening, editor/runtime regressions, current-health design docs, and feel-good randomness helpers.
  • Adds Unity 6000.5.2f1 to the shared Unity version source and validates runner bootstrap/maintenance uses that source.
  • Fixes Unity 6000.5 compile errors: 3.4.0 references Sirenix/Odin types without dependency #275 by making Odin/Sirenix integration package-local and optional: production code no longer trusts the global ODIN_INSPECTOR symbol, asmdefs define WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR from the odininspector package, and fast contracts cover the failure class.

Validation

  • npm run validate:prepush
  • pwsh -NoProfile -File scripts/tests/test-sync-script-contracts.ps1 -VerboseOutput
  • pwsh -NoProfile -File scripts/tests/test-unity-workflow-matrix-contract.ps1 -VerboseOutput
  • pwsh -NoProfile -File scripts/check-eol.ps1 -VerboseOutput
  • git diff --check
  • Sub-agent adversarial review loop reached consensus/no revisions after fixing dangling Odin sample #endif lines.

Unity editor tests were not run locally because the available Unity MCP editor is attached to D:/Code/Packages rather than this clean worktree. CI now covers the configured Unity matrix including 6000.5.2f1.

Fixes #275
Fixes #274
Fixes #270
Fixes #254
Fixes #221
Fixes #217
Fixes #200
Fixes #185
Fixes #160
Fixes #138
Fixes #135
Fixes #134


Note

High Risk
Touches core CI provisioning, Unity license/lock lifecycle, asmdef/Odin compilation paths, and broad workflow behavior—failures can block releases or break consumers without Odin installed.

Overview
Adds Unity 6000.5.2f1 to the shared version matrix, bumps Reflex for 6000.5 editor compatibility, and wires self-hosted Windows CI to run maintain-windows-runner.ps1 before Unity legs (with longer job timeouts, max-parallel: 2, and Unity org lock release only when acquire succeeded). Runner bootstrap now pins dispatch via DAD-MACHINE / ELI-MACHINE labels and reads all versions from unity-versions.json; GHCR/devcontainer image paths move to ambiguous-interactive.

Odin is package-local: production and tests gate on WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR from the odininspector package (not global ODIN_INSPECTOR), with Unity base-class fallbacks in runtime singletons/AttributeEffect and relaxed runtime asmdef precompiled refs.

Runtime/editor product changes include feel-good randomness (ExactAveragePrd, BadLuckProtection, WeightedShuffleBag), AssetChangeDetectionUtility.ResetLoopProtection(), LayeredImage self-update via UI Toolkit scheduling, and doc link validation moved to lint-doc-links.ps1.

Reviewed by Cursor Bugbot for commit ba1eaf5. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI review requested due to automatic review settings July 6, 2026 01:17
@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a wide set of open issues across Unity 6000.5 compatibility, optional Odin/Sirenix integration, CI/self-hosted runner maintenance, devcontainer publishing hardening, and several runtime/editor regressions, with accompanying tests and documentation updates.

Changes:

  • Adds Unity 6000.5.2f1 to the canonical CI version source and wires runner bootstrap/maintenance to use it.
  • Makes Odin integration package-local via an asmdef-defined symbol and updates runtime/editor code, tests, and docs accordingly.
  • Introduces/expands product and test coverage for feel-good randomness helpers, asset-change loop reset, and LayeredImage rendering behavior.

Reviewed changes

Copilot reviewed 65 out of 68 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Tests/Runtime/Visuals/LayeredImageTests.cs Adds panel-less/background update tests
Tests/Runtime/Utils/RuntimeSingletonTests.cs Updates singleton registry id assertion
Tests/Runtime/Random/FeelGoodRandomnessTests.cs Adds tests for new randomness helpers
Tests/Runtime/Random/FeelGoodRandomnessTests.cs.meta Unity meta for new test file
Tests/Editor/TestTypes/Odin/WGroup/OdinWGroupScriptableObjectTarget.cs Adds Odin WGroup SO test type
Tests/Editor/TestTypes/Odin/WGroup/OdinWGroupScriptableObjectTarget.cs.meta Unity meta for new test type
Tests/Editor/TestTypes/Odin/WGroup/OdinWGroupMonoBehaviourTarget.cs Adds Odin WGroup MB test type
Tests/Editor/TestTypes/Odin/WGroup/OdinWGroupMonoBehaviourTarget.cs.meta Unity meta for new test type
Tests/Editor/TestTypes/Odin/WGroup.meta Unity meta for new folder
Tests/Editor/TestAssets/SharedTextureTestFixtures.cs Uses EnsureAssetFolder helper
Tests/Editor/TestAssets/SharedPrefabTestFixtures.cs Uses EnsureAssetFolder helper
Tests/Editor/TestAssets/SharedEditorTestFixtures.cs Uses EnsureAssetFolder helper
Tests/Editor/CustomEditors/OdinWGroupInspectorTests.cs Adds Odin WGroup inspector integration tests
Tests/Editor/CustomEditors/OdinWGroupInspectorTests.cs.meta Unity meta for new test file
Tests/Editor/AssetProcessors/DetectAssetChangeProcessorTests.cs Tests loop-protection reset behavior
Tests/Core/Editor/AssetDatabaseBatchScopeTests.cs Tests adopting on-disk folders safely
scripts/validate-npm-package.ps1 Rejects PR description artifacts in package
scripts/validate-devcontainer-config.ps1 Enforces GHCR namespace/labels/contracts
scripts/unity/maintain-windows-runner.ps1 Adds runner maintenance entrypoint
scripts/unity/maintain-windows-runner.ps1.meta Unity meta for script
scripts/unity/bootstrap-windows-runner.ps1 Adds Windows prerequisite bootstrapper
scripts/unity/bootstrap-windows-runner.ps1.meta Unity meta for script
scripts/tests/test-unity-workflow-matrix-contract.ps1 Contracts for Unity matrix + runner scripts
scripts/tests/test-sync-script-contracts.ps1 Adds optional Odin integration contract tests
Runtime/WallstopStudios.UnityHelpers.asmdef Adds odininspector versionDefine (and Sirenix refs)
Runtime/Visuals/UIToolkit/LayeredImage.cs Renders initial frame and force-update behaviors
Runtime/Utils/ScriptableObjectSingleton.cs Switches to package-owned Odin symbol
Runtime/Utils/RuntimeSingletonRegistry.cs Uses GetUnityObjectId instead of GetInstanceID
Runtime/Utils/RuntimeSingleton.cs Switches to package-owned Odin symbol
Runtime/Tags/AttributeEffect.cs Switches to package-owned Odin symbol
Runtime/Core/Random/WeightedShuffleBag.cs Adds weighted no-replacement sampler
Runtime/Core/Random/WeightedShuffleBag.cs.meta Unity meta for new runtime file
Runtime/Core/Random/ExactAveragePrd.cs Adds exact-average PRD helper
Runtime/Core/Random/ExactAveragePrd.cs.meta Unity meta for new runtime file
Runtime/Core/Random/BadLuckProtection.cs Adds pity-timer helper
Runtime/Core/Random/BadLuckProtection.cs.meta Unity meta for new runtime file
Editor/WallstopStudios.UnityHelpers.Editor.asmdef Adds odininspector versionDefine
Editor/Utils/WGroup/WGroupGUI.cs Adds WGroup draw diagnostics counters
Editor/Utils/WButton/WButtonEditorHelper.cs Updates Odin example snippet
Editor/Internal/EditorCacheManager.cs Updates Odin guard symbol
Editor/CustomEditors/WButtonOdinScriptableObjectInspector.cs Updates Odin guard symbol and comment
Editor/CustomEditors/WButtonOdinMonoBehaviourInspector.cs Updates Odin guard symbol and comment
Editor/CustomEditors/WButtonOdinInspectorHelper.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/WValueDropDownOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/WShowIfOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/WReadOnlyOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/WNotNullOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/WInLineEditorOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/WEnumToggleButtonsOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/ValidateAssignmentOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/StringInListOdinDrawer.cs Updates Odin guard symbol
Editor/CustomDrawers/Odin/IntDropDownOdinDrawer.cs Updates Odin guard symbol
Editor/AssetProcessors/DetectAssetChangeProcessor.cs Adds internal loop-protection reset
Editor/AssetProcessors/AssetChangeDetectionUtility.cs Adds public reset utility wrapper
Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta Unity meta for new editor file
docs/overview/glossary.md Updates Odin compatibility wording
docs/features/utilities/singletons.md Documents package-owned Odin symbol behavior
docs/features/utilities/random-generators.md Documents feel-good randomness helpers
docs/features/inspector/inspector-button.md Updates Odin integration guidance
docs/features/effects/effects-system.md Adds current-resource ownership guidance
docs/features/editor-tools/asset-change-detection.md Documents loop-protection reset API
CHANGELOG.md Records new features and Odin fix
.npmignore Ignores PR description artifacts
.github/workflows/validate-devcontainer.yml Validates when publish workflow changes
.github/workflows/runner-bootstrap.yml Uses unity-versions + maintain script invocation
.github/workflows/build-publish-devcontainer.yml Updates GHCR namespace + validation + labels
.github/unity-versions.json Adds Unity 6000.5.2f1
.devcontainer/devcontainer.json Updates GHCR cache image namespace
Files not reviewed (3)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file
  • Runtime/Core/Random/BadLuckProtection.cs.meta: Generated file
  • Runtime/Core/Random/ExactAveragePrd.cs.meta: Generated file

Comment on lines 15 to 19
"System.Collections.Immutable.dll",
"System.Runtime.CompilerServices.Unsafe.dll"
"System.Runtime.CompilerServices.Unsafe.dll",
"Sirenix.Serialization.dll",
"Sirenix.OdinInspector.Attributes.dll"
],
Comment on lines 128 to 136
internal static void LogDrawProperty(string groupName, string propertyPath, int indentLevel)
{
if (MatchesGroupNameFilter(groupName))
{
++DrawPropertyLogCount;
}

if (!ShouldLog(groupName))
{
Comment on lines +90 to +93
bool testCompleted = false;
Exception caughtException = null;
WGroupIndentDiagnostics.GroupNameFilter = "stats";

Comment on lines +126 to +130
Assert.That(editor, Is.TypeOf<WButtonOdinMonoBehaviourInspector>());
bool testCompleted = false;
Exception caughtException = null;
WGroupIndentDiagnostics.GroupNameFilter = "stats";

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Review Complete ✅

I've reviewed the PR changes. Most linters have passed, and the Unity 6000.5.2f1 integration looks solid. However, I found one critical inconsistency:

Issue: Test assemblies still use old ODIN_INSPECTOR symbol

Problem: The production code correctly switched to WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR (via versionDefines in Runtime/WallstopStudios.UnityHelpers.asmdef and Editor/WallstopStudios.UnityHelpers.Editor.asmdef), but the test files still use the old global ODIN_INSPECTOR symbol.

Evidence:

  • Tests/Editor/CustomDrawers/WallstopStudios.UnityHelpers.Tests.Editor.CustomDrawers.asmdef has versionDefines: [] (empty)
  • Test files like Tests/Editor/CustomDrawers/Odin/WShowIfOdinDrawerTests.cs use #if UNITY_EDITOR && ODIN_INSPECTOR instead of WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR

Impact: If a project has a stray ODIN_INSPECTOR define but no Sirenix assemblies, tests would compile but production code wouldn't match test guards.

Fix needed: Add the versionDefines block to all test asmdefs that reference Odin types:

"versionDefines": [
  {
    "name": "odininspector",
    "expression": "0.0.1",
    "define": "WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR"
  }
]

Then update all test #if ODIN_INSPECTOR to #if WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR.

Other observations:

  • ✅ Unity 6000.5.2f1 added to .github/unity-versions.json
  • ✅ LayeredImage fps parameter type fix (int → float) looks correct
  • ✅ Contract tests passing (119/119)
  • ⚠️ Git push config validation failing (expected in CI, not a code issue)
  • ⏳ Unity Tests workflow queued/running

Copilot review

Review completed. See comment above for findings. Main issue: test assemblies need versionDefines for WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR to match production code's Odin gating strategy.

Comment thread Tests/Editor/CustomEditors/OdinWGroupInspectorTests.cs Outdated
@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Review Complete ✅

All previously identified issues have been successfully addressed in commit d75627d.

Issues Fixed

1. Test assembly Odin symbol guards

  • All test asmdefs now include versionDefines for WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR
  • All test files updated from #if ODIN_INSPECTOR to #if WALLSTOP_UNITY_HELPERS_ODIN_INSPECTOR
  • Consistent with production code guards

2. Runtime Sirenix dependencies removed

  • Runtime/WallstopStudios.UnityHelpers.asmdef no longer includes Sirenix.Serialization.dll or Sirenix.OdinInspector.Attributes.dll
  • versionDefines removed from Runtime asmdef
  • ScriptableObjectSingleton<T> now unconditionally derives from ScriptableObject (not SerializedScriptableObject)
  • Makes Odin truly optional at runtime

3. WGroupGUI diagnostics counter fixed

  • Counter now only increments when ShouldLog() returns true (when Enabled is true AND group filter matches)
  • Tests properly enable diagnostics before asserting counter values

CI Status

  • ✅ All linters passing (spelling, formatting, docs, etc.)
  • ✅ Contract tests: 119/119 passed
  • ⏳ Unity Tests queued/running
  • ⏳ Documentation validation in progress

Summary

The Odin integration is now truly optional and package-local. Projects without Odin will compile cleanly, and the symbol gating is consistent across production and test code.

Copilot AI review requested due to automatic review settings July 6, 2026 02:41
@wallstop

Copy link
Copy Markdown
Collaborator Author

Copilot review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 252 out of 253 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Copilot review

Completed a fresh review and pushed fc733dc to address an actionable CI issue I found: maintain-windows-runner.ps1 now invokes ensure-editor.ps1 via named hashtable splatting (instead of string-array args), which fixes UnityVersion misbinding ("-UnityVersion") in Runner Bootstrap runs. I also updated the workflow matrix contract test to lock this shape and re-ran scripts/tests/test-unity-workflow-matrix-contract.ps1 and scripts/tests/test-sync-script-contracts.ps1 successfully.

@wallstop Eli Pinkerton (wallstop) force-pushed the codex/active-issues-ci-6000-5 branch from fc733dc to 7ae4305 Compare July 6, 2026 03:13
Copilot AI review requested due to automatic review settings July 6, 2026 03:13
Copilot AI review requested due to automatic review settings July 6, 2026 21:47
@wallstop

Copy link
Copy Markdown
Collaborator Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit dd3f55c. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 271 out of 272 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file

@wallstop

Copy link
Copy Markdown
Collaborator Author

bugbot run

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 271 out of 272 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file

Comment on lines +411 to +414

if ($previousRepoRoot -ceq $currentRepoRoot) {
return
}

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit ad13f66. Configure here.

@wallstop

Copy link
Copy Markdown
Collaborator Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit db72c2a. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 272 out of 273 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file

@wallstop

Copy link
Copy Markdown
Collaborator Author

bugbot run

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 272 out of 273 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file

Comment thread scripts/unity/run-ci-tests.ps1 Outdated
Comment on lines +412 to +414
if ($previousRepoRoot -ceq $currentRepoRoot) {
return
}

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d649464. Configure here.

@wallstop

Copy link
Copy Markdown
Collaborator Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ba1eaf5. Configure here.

}
$pwshDirectory = Split-Path -Parent $pwshPath
$pwshDirectory | Out-File -FilePath $env:GITHUB_PATH -Append -Encoding utf8
Write-Host "Added PowerShell 7 to GITHUB_PATH: $pwshDirectory"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintenance runs before empty skip

Medium Severity

The new Maintain Unity editor on selected runner step runs immediately after checkout, while the Compute test assembly list step that sets steps.compute.outputs.is-empty runs later. Later steps correctly skip provisioning, the org lock, and tests when the assembly list is empty, but maintenance has already invoked Invoke-WindowsRunnerMaintenance (bootstrap plus ensure-editor.ps1) for that matrix leg. Matrix legs that were meant to no-op still perform heavy host and editor work, and the same leg then may run Provision Unity Editor again when not empty, duplicating provisioning in one job.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ba1eaf5. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 272 out of 273 changed files in this pull request and generated 5 comments.

Files not reviewed (1)
  • Editor/AssetProcessors/AssetChangeDetectionUtility.cs.meta: Generated file

Comment on lines 14 to 23
"overrideReferences": true,
"precompiledReferences": [
"nunit.framework.dll",
"System.Text.Json.dll",
"protobuf-net.dll",
"protobuf-net.Core.dll",
"System.Collections.Immutable.dll",
"Sirenix.Serialization.dll"
"Sirenix.Serialization.dll",
"Sirenix.OdinInspector.Attributes.dll"
],
Comment on lines 16 to 23
"overrideReferences": true,
"precompiledReferences": [
"nunit.framework.dll",
"System.Text.Json.dll",
"Sirenix.Serialization.dll",
"Sirenix.OdinInspector.Attributes.dll",
"Sirenix.OdinInspector.Editor.dll"
],
Comment on lines 16 to +21
"overrideReferences": true,
"precompiledReferences": ["nunit.framework.dll", "Sirenix.Serialization.dll"],
"precompiledReferences": [
"nunit.framework.dll",
"Sirenix.Serialization.dll",
"Sirenix.OdinInspector.Attributes.dll"
],
Comment on lines 16 to 22
"overrideReferences": true,
"precompiledReferences": [
"nunit.framework.dll",
"Sirenix.Serialization.dll",
"Sirenix.OdinInspector.Attributes.dll",
"Sirenix.OdinInspector.Editor.dll"
],
Comment on lines 16 to 22
"overrideReferences": true,
"precompiledReferences": [
"nunit.framework.dll",
"Sirenix.Serialization.dll",
"Sirenix.OdinInspector.Attributes.dll",
"Sirenix.OdinInspector.Editor.dll"
],
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment