Skip to content

Conversation

@sjennings
Copy link
Contributor

@sjennings sjennings commented Jan 7, 2026

Summary

  • Moved SaveDirtyScenesIfNeeded() call outside the PlayMode conditional so EditMode tests don't get blocked by Unity's "Save Scene" modal dialog

Problem

When running EditMode tests through MCP with unsaved scene changes, Unity displays a "Save Scene" modal dialog that blocks the editor, causing MCP to timeout without receiving a response.

Solution

The SaveDirtyScenesIfNeeded() method was only being called for PlayMode tests. This one-line change ensures dirty scenes are saved before ALL test runs, not just PlayMode.

Testing

  • Verified EditMode tests no longer block on save dialogs
  • 1165 tests run successfully with this fix

Fixes #525

Summary by Sourcery

Bug Fixes:

  • Prevent EditMode test runs from being blocked by Unity's save scene modal dialog by saving dirty scenes for all test modes.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where EditMode tests were blocked by save dialogs. Dirty scenes are now automatically saved before running any test mode, ensuring smooth test execution across all test types.

✏️ Tip: You can customize this high-level summary in your review settings.

Move SaveDirtyScenesIfNeeded() call outside the PlayMode conditional
so EditMode tests don't get blocked by Unity's "Save Scene" modal dialog.

This prevents MCP from timing out when running EditMode tests with unsaved
scene changes.
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 7, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensures dirty scenes are saved before running tests in any mode by unconditionally invoking SaveDirtyScenesIfNeeded() prior to executing tests, preventing Unity save dialogs from blocking EditMode test runs.

Sequence diagram for unconditional saving of dirty scenes before test runs

sequenceDiagram
    actor Developer
    participant MCP
    participant TestRunnerService
    participant UnityEditor
    participant TestRunnerApi

    Developer->>MCP: requestTestRun(mode, filterOptions)
    MCP->>TestRunnerService: RunTestsAsync(mode, filterOptions)
    activate TestRunnerService
    TestRunnerService->>UnityEditor: SaveDirtyScenesIfNeeded()
    UnityEditor-->>TestRunnerService: dirty scenes saved
    TestRunnerService->>TestRunnerApi: Execute(settings)
    TestRunnerApi-->>TestRunnerService: testRunResult
    deactivate TestRunnerService
    TestRunnerService-->>MCP: testRunResult
    MCP-->>Developer: display test results
Loading

Class diagram for updated TestRunnerService test execution flow

classDiagram
    class TestRunnerService {
        - TestRunnerApi _testRunnerApi
        + Task~TestRunResult~ RunTestsAsync(TestMode mode, TestFilterOptions filterOptions)
        - void SaveDirtyScenesIfNeeded()
    }

    class TestRunnerApi {
        + void Execute(ExecutionSettings settings)
    }

    class ExecutionSettings {
        + ExecutionSettings(TestFilterOptions filterOptions)
    }

    class TestRunResult
    class TestFilterOptions

    class TestMode {
        <<enumeration>>
        EditMode
        PlayMode
    }

    TestRunnerService --> TestRunnerApi : uses
    TestRunnerService --> ExecutionSettings : creates
    ExecutionSettings --> TestFilterOptions : uses
    TestRunnerService --> TestRunResult : returns
    TestRunnerService --> TestMode : parameter
    TestRunnerService --> TestFilterOptions : parameter
Loading

File-Level Changes

Change Details Files
Invoke scene-saving logic for all test modes instead of only PlayMode before executing tests. MCPForUnity/Editor/Services/TestRunnerService.cs

Assessment against linked issues

Issue Objective Addressed Explanation
#525 Ensure dirty scenes are automatically saved before running all test modes (including EditMode) in TestRunnerService.RunTestsAsync(), so Unity's 'Save Scene' modal dialog does not block MCP when running EditMode tests.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

The change removes the PlayMode-only condition on SaveDirtyScenesIfNeeded() in TestRunnerService, making it execute for all test modes. This addresses issue #525 where EditMode tests triggered a blocking save dialog when scenes were dirty.

Changes

Cohort / File(s) Change Summary
Scene auto-save control flow
MCPForUnity/Editor/Services/TestRunnerService.cs
Removed conditional guard that restricted SaveDirtyScenesIfNeeded() to PlayMode only; now executes unconditionally before all test runs to prevent modal save dialogs from blocking EditMode tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • Harden PlayMode test runs #396: Modifies TestRunnerService.cs to adjust the timing of SaveDirtyScenesIfNeeded() invocation within the test-run flow, directly overlapping in scope and intent.

Poem

🐰 No more save dialogs to delay my test run,
EditMode and PlayMode now save as one,
Dirty scenes wiped clean before tests begin,
Smooth sailing through the editor's test spin!

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: moving SaveDirtyScenesIfNeeded() call to execute for all test modes instead of just PlayMode, directly addressing issue #525.
Linked Issues check ✅ Passed The code change directly implements the proposed fix from issue #525 by moving SaveDirtyScenesIfNeeded() outside the PlayMode conditional, ensuring dirty scenes are saved for all test modes before execution.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #525; the modification involves only relocating SaveDirtyScenesIfNeeded() within TestRunnerService.cs with explanatory comments, with no extraneous or unrelated code additions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de4d0f and 98f791b.

📒 Files selected for processing (1)
  • MCPForUnity/Editor/Services/TestRunnerService.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T15:23:11.613Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 491
File: MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs:78-115
Timestamp: 2025-12-29T15:23:11.613Z
Learning: In MCPForUnity, prefer relying on the established testing process to catch UI initialization issues instead of adding defensive null checks for UI elements in editor windows. This means during reviews, verify that tests cover UI initialization paths and that code avoids repetitive null-check boilerplate in Editor Windows. If a UI element can be null, ensure there is a well-tested fallback or that its initialization is guaranteed by design, rather than sprinkling null checks throughout editor code.

Applied to files:

  • MCPForUnity/Editor/Services/TestRunnerService.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (1)
MCPForUnity/Editor/Services/TestRunnerService.cs (1)

109-111: LGTM! Clean fix for the blocking save dialog issue.

Moving SaveDirtyScenesIfNeeded() outside the PlayMode conditional ensures dirty scenes are saved before all test runs, preventing the modal dialog that was blocking EditMode tests. The added comments clearly document the rationale.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Services/TestRunnerService.cs (1)

333-333: Update warning message to reflect all test modes.

The warning message references "PlayMode tests" but SaveDirtyScenesIfNeeded() now runs before all test modes (EditMode and PlayMode). Update the message for accuracy.

📝 Proposed fix
-                        McpLog.Warn($"[TestRunnerService] Skipping unsaved scene '{scene.name}': save it manually before running PlayMode tests.");
+                        McpLog.Warn($"[TestRunnerService] Skipping unsaved scene '{scene.name}': save it manually before running tests.");
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de4d0f and 98f791b.

📒 Files selected for processing (1)
  • MCPForUnity/Editor/Services/TestRunnerService.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T15:23:11.613Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 491
File: MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs:78-115
Timestamp: 2025-12-29T15:23:11.613Z
Learning: In MCPForUnity, prefer relying on the established testing process to catch UI initialization issues instead of adding defensive null checks for UI elements in editor windows. This means during reviews, verify that tests cover UI initialization paths and that code avoids repetitive null-check boilerplate in Editor Windows. If a UI element can be null, ensure there is a well-tested fallback or that its initialization is guaranteed by design, rather than sprinkling null checks throughout editor code.

Applied to files:

  • MCPForUnity/Editor/Services/TestRunnerService.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (1)
MCPForUnity/Editor/Services/TestRunnerService.cs (1)

109-111: LGTM! Clean fix for the blocking save dialog issue.

Moving SaveDirtyScenesIfNeeded() outside the PlayMode conditional ensures dirty scenes are saved before all test runs, preventing the modal dialog that was blocking EditMode tests. The added comments clearly document the rationale.

dsarno added a commit to dsarno/unity-mcp that referenced this pull request Jan 7, 2026
Follow-up to PR CoplayDev#527: Since SaveDirtyScenesIfNeeded() now runs for all test modes, update the warning message to say 'tests' instead of 'PlayMode tests'.
@msanatan
Copy link
Member

msanatan commented Jan 7, 2026

Thanks @sjennings , this should be in the release today

msanatan pushed a commit to msanatan/unity-mcp that referenced this pull request Jan 7, 2026
#8)

* Add local test harness for fast developer iteration

Scripts for running the NL/T/GO test suites locally against a GUI Unity
Editor, complementing the CI workflows in .github/workflows/.

Benefits:
- 10-100x faster than CI (no Docker startup)
- Real-time Unity console debugging
- Single test execution for rapid iteration
- Auto-detects HTTP vs stdio transport

Usage:
  ./scripts/local-test/setup.sh           # One-time setup
  ./scripts/local-test/quick-test.sh NL-0 # Run single test
  ./scripts/local-test/run-nl-suite-local.sh  # Full suite

See scripts/local-test/README.md for details.

Also updated .gitignore to:
- Allow scripts/local-test/ to be tracked
- Ignore generated artifacts (reports/*.xml, .claude/local/, .unity-mcp/)

* Fix issue CoplayDev#525: Save dirty scenes for all test modes

Move SaveDirtyScenesIfNeeded() call outside the PlayMode conditional
so EditMode tests don't get blocked by Unity's "Save Scene" modal dialog.

This prevents MCP from timing out when running EditMode tests with unsaved
scene changes.

* fix: add missing FAST_FAIL_TIMEOUT constant in PluginHub

The FAST_FAIL_TIMEOUT class attribute was referenced on line 149 but never
defined, causing AttributeError on every ping attempt. This error was silently
caught by the broad 'except Exception' handler, causing all fast-fail commands
(read_console, get_editor_state, ping) to fail after 6 seconds of retries with
'ping not answered' error.

Added FAST_FAIL_TIMEOUT = 10 to define a 10-second timeout for fast-fail
commands, matching the intent of the existing fast-fail infrastructure.

* feat(ScriptableObject): enhance dry-run validation for AnimationCurve and Quaternion

Dry-run validation now validates value formats, not just property existence:

- AnimationCurve: Validates structure ({keys:[...]} or direct array), checks
  each keyframe is an object, validates numeric fields (time, value, inSlope,
  outSlope, inWeight, outWeight) and integer fields (weightedMode)
- Quaternion: Validates array length (3 for Euler, 4 for raw) or object
  structure ({x,y,z,w} or {euler:[x,y,z]}), ensures all components are numeric

Refactored shared validation helpers into appropriate locations:
- ParamCoercion: IsNumericToken, ValidateNumericField, ValidateIntegerField
- VectorParsing: ValidateAnimationCurveFormat, ValidateQuaternionFormat

Added comprehensive XML documentation clarifying keyframe field defaults
(all default to 0 except as noted).

Added 5 new dry-run validation tests covering valid and invalid formats
for both AnimationCurve and Quaternion properties.

* test: fix integration tests after merge

- test_refresh_unity_retry_recovery: Mock now handles both refresh_unity and
  get_editor_state commands (refresh_unity internally calls get_editor_state
  when wait_for_ready=True)
- test_run_tests_async_forwards_params: Mock response now includes required
  'mode' field for RunTestsStartResponse Pydantic validation
- test_get_test_job_forwards_job_id: Updated to handle GetTestJobResponse as
  Pydantic model instead of dict (use model_dump() for assertions)

* Update warning message to apply to all test modes

Follow-up to PR CoplayDev#527: Since SaveDirtyScenesIfNeeded() now runs for all test modes, update the warning message to say 'tests' instead of 'PlayMode tests'.

* feat(run_tests): add wait_timeout to get_test_job to avoid client loop detection

When polling for test completion, MCP clients like Cursor can detect the
repeated get_test_job calls as 'looping' and terminate the agent.

Added wait_timeout parameter that makes the server wait internally for tests
to complete (polling Unity every 2s) before returning. This dramatically
reduces client-side tool calls from 10-20 down to 1-2, avoiding loop detection.

Usage: get_test_job(job_id='xxx', wait_timeout=30)
- Returns immediately if tests complete within timeout
- Returns current status if timeout expires (client can call again)
- Recommended: 30-60 seconds

* fix: use Pydantic attribute access in test_run_tests_async for merge compatibility

* revert: remove local test harness - will be submitted in separate PR

---------

Co-authored-by: Scott Jennings <scott.jennings+CIGINT@cloudimperiumgames.com>
msanatan added a commit that referenced this pull request Jan 7, 2026
* refactor: Split ParseColorOrDefault into two overloads and change default to Color.white

* Auto-format Python code

* Remove unused Python module

* Refactored VFX functionality into multiple files

Tested everything, works like a charm

* Rename ManageVfx folder to just Vfx

We know what it's managing

* Clean up whitespace on plugin tools and resources

* Make ManageGameObject less of a monolith by splitting it out into different files

* Remove obsolete FindObjectByInstruction method

We also update the namespace for ManageVFX

* refactor: Consolidate editor state resources into single canonical implementation

Merged EditorStateV2 into EditorState, making get_editor_state the canonical resource. Updated Unity C# to use EditorStateCache directly. Enhanced Python implementation with advice/staleness enrichment, external changes detection, and instance ID inference. Removed duplicate EditorStateV2 resource and legacy fallback mapping.

* Validate editor state with Pydantic models in both C# and Python

Added strongly-typed Pydantic models for EditorStateV2 schema in Python and corresponding C# classes with JsonProperty attributes. Updated C# to serialize using typed classes instead of anonymous objects. Python now validates the editor state payload before returning it, catching schema mismatches early.

* Consolidate run_tests and run_tests_async into single async implementation

Merged run_tests_async into run_tests, making async job-based execution the default behavior. Removed synchronous blocking test execution. Updated RunTests.cs to start test jobs immediately and return job_id for polling. Changed TestJobManager methods to internal visibility. Updated README to reflect single run_tests_async tool. Python implementation now uses async job pattern exclusively.

* Validate test job responses with Pydantic models in Python

* Change resources URI from unity:// to mcpforunity://

It should reduce conflicts with other Unity MCPs that users try, and to comply with Unity's requests regarding use of their company and product name

* Update README with all tools + better listing for resources

* Update other references to resources

* Updated translated doc - unfortunately I cannot verify

* Update the Chinese translation of the dev docks

* Change menu item from Setup Window to Local Setup Window

We now differentiate whether it's HTTP local or remote

* Fix URIs for menu items and tests

* Shouldn't have removed it

* Minor edits from CodeRabbit feedback

* Don't use reflection which takes longer

* Fix failing python tests

* Add serialization helpers for ParticleSystem curves and MinMaxCurve types

Added SerializeAnimationCurve and SerializeMinMaxCurve helper methods to properly serialize Unity's curve types. Updated GetInfo to use these helpers for startLifetime, startSpeed, startSize, gravityModifier, and rateOverTime instead of only reading constant values.

* Use ctx param

* Update Server/src/services/tools/run_tests.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Minor fixes

* Rename anything EditorStateV2 to just EditorState

It's the default, there's no old version

* Make infer_single_instance_id public by removing underscore prefix

* Fix Python tests, again

* Replace AI generated .meta files with actual Unity ones

* ## Pre-Launch Enhancements: Testing Infrastructure & Tool Improvements (#8)

* Add local test harness for fast developer iteration

Scripts for running the NL/T/GO test suites locally against a GUI Unity
Editor, complementing the CI workflows in .github/workflows/.

Benefits:
- 10-100x faster than CI (no Docker startup)
- Real-time Unity console debugging
- Single test execution for rapid iteration
- Auto-detects HTTP vs stdio transport

Usage:
  ./scripts/local-test/setup.sh           # One-time setup
  ./scripts/local-test/quick-test.sh NL-0 # Run single test
  ./scripts/local-test/run-nl-suite-local.sh  # Full suite

See scripts/local-test/README.md for details.

Also updated .gitignore to:
- Allow scripts/local-test/ to be tracked
- Ignore generated artifacts (reports/*.xml, .claude/local/, .unity-mcp/)

* Fix issue #525: Save dirty scenes for all test modes

Move SaveDirtyScenesIfNeeded() call outside the PlayMode conditional
so EditMode tests don't get blocked by Unity's "Save Scene" modal dialog.

This prevents MCP from timing out when running EditMode tests with unsaved
scene changes.

* fix: add missing FAST_FAIL_TIMEOUT constant in PluginHub

The FAST_FAIL_TIMEOUT class attribute was referenced on line 149 but never
defined, causing AttributeError on every ping attempt. This error was silently
caught by the broad 'except Exception' handler, causing all fast-fail commands
(read_console, get_editor_state, ping) to fail after 6 seconds of retries with
'ping not answered' error.

Added FAST_FAIL_TIMEOUT = 10 to define a 10-second timeout for fast-fail
commands, matching the intent of the existing fast-fail infrastructure.

* feat(ScriptableObject): enhance dry-run validation for AnimationCurve and Quaternion

Dry-run validation now validates value formats, not just property existence:

- AnimationCurve: Validates structure ({keys:[...]} or direct array), checks
  each keyframe is an object, validates numeric fields (time, value, inSlope,
  outSlope, inWeight, outWeight) and integer fields (weightedMode)
- Quaternion: Validates array length (3 for Euler, 4 for raw) or object
  structure ({x,y,z,w} or {euler:[x,y,z]}), ensures all components are numeric

Refactored shared validation helpers into appropriate locations:
- ParamCoercion: IsNumericToken, ValidateNumericField, ValidateIntegerField
- VectorParsing: ValidateAnimationCurveFormat, ValidateQuaternionFormat

Added comprehensive XML documentation clarifying keyframe field defaults
(all default to 0 except as noted).

Added 5 new dry-run validation tests covering valid and invalid formats
for both AnimationCurve and Quaternion properties.

* test: fix integration tests after merge

- test_refresh_unity_retry_recovery: Mock now handles both refresh_unity and
  get_editor_state commands (refresh_unity internally calls get_editor_state
  when wait_for_ready=True)
- test_run_tests_async_forwards_params: Mock response now includes required
  'mode' field for RunTestsStartResponse Pydantic validation
- test_get_test_job_forwards_job_id: Updated to handle GetTestJobResponse as
  Pydantic model instead of dict (use model_dump() for assertions)

* Update warning message to apply to all test modes

Follow-up to PR #527: Since SaveDirtyScenesIfNeeded() now runs for all test modes, update the warning message to say 'tests' instead of 'PlayMode tests'.

* feat(run_tests): add wait_timeout to get_test_job to avoid client loop detection

When polling for test completion, MCP clients like Cursor can detect the
repeated get_test_job calls as 'looping' and terminate the agent.

Added wait_timeout parameter that makes the server wait internally for tests
to complete (polling Unity every 2s) before returning. This dramatically
reduces client-side tool calls from 10-20 down to 1-2, avoiding loop detection.

Usage: get_test_job(job_id='xxx', wait_timeout=30)
- Returns immediately if tests complete within timeout
- Returns current status if timeout expires (client can call again)
- Recommended: 30-60 seconds

* fix: use Pydantic attribute access in test_run_tests_async for merge compatibility

* revert: remove local test harness - will be submitted in separate PR

---------

Co-authored-by: Scott Jennings <scott.jennings+CIGINT@cloudimperiumgames.com>

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: dsarno <david@lighthaus.us>
Co-authored-by: Scott Jennings <scott.jennings+CIGINT@cloudimperiumgames.com>
@msanatan msanatan merged commit 5e21f12 into CoplayDev:main Jan 7, 2026
2 checks passed
@msanatan
Copy link
Member

msanatan commented Jan 7, 2026

@sjennings f you're on LinkedIn, Twitter/X - share your profile and we'll tag you for your contributions! Completely optional of course

msanatan added a commit that referenced this pull request Jan 8, 2026
* refactor: Split ParseColorOrDefault into two overloads and change default to Color.white

* Auto-format Python code

* Remove unused Python module

* Refactored VFX functionality into multiple files

Tested everything, works like a charm

* Rename ManageVfx folder to just Vfx

We know what it's managing

* Clean up whitespace on plugin tools and resources

* Make ManageGameObject less of a monolith by splitting it out into different files

* Remove obsolete FindObjectByInstruction method

We also update the namespace for ManageVFX

* Add local test harness for fast developer iteration

Scripts for running the NL/T/GO test suites locally against a GUI Unity
Editor, complementing the CI workflows in .github/workflows/.

Benefits:
- 10-100x faster than CI (no Docker startup)
- Real-time Unity console debugging
- Single test execution for rapid iteration
- Auto-detects HTTP vs stdio transport

Usage:
  ./scripts/local-test/setup.sh           # One-time setup
  ./scripts/local-test/quick-test.sh NL-0 # Run single test
  ./scripts/local-test/run-nl-suite-local.sh  # Full suite

See scripts/local-test/README.md for details.

Also updated .gitignore to:
- Allow scripts/local-test/ to be tracked
- Ignore generated artifacts (reports/*.xml, .claude/local/, .unity-mcp/)

* Fix issue #525: Save dirty scenes for all test modes

Move SaveDirtyScenesIfNeeded() call outside the PlayMode conditional
so EditMode tests don't get blocked by Unity's "Save Scene" modal dialog.

This prevents MCP from timing out when running EditMode tests with unsaved
scene changes.

* refactor: Consolidate editor state resources into single canonical implementation

Merged EditorStateV2 into EditorState, making get_editor_state the canonical resource. Updated Unity C# to use EditorStateCache directly. Enhanced Python implementation with advice/staleness enrichment, external changes detection, and instance ID inference. Removed duplicate EditorStateV2 resource and legacy fallback mapping.

* Validate editor state with Pydantic models in both C# and Python

Added strongly-typed Pydantic models for EditorStateV2 schema in Python and corresponding C# classes with JsonProperty attributes. Updated C# to serialize using typed classes instead of anonymous objects. Python now validates the editor state payload before returning it, catching schema mismatches early.

* Consolidate run_tests and run_tests_async into single async implementation

Merged run_tests_async into run_tests, making async job-based execution the default behavior. Removed synchronous blocking test execution. Updated RunTests.cs to start test jobs immediately and return job_id for polling. Changed TestJobManager methods to internal visibility. Updated README to reflect single run_tests_async tool. Python implementation now uses async job pattern exclusively.

* Validate test job responses with Pydantic models in Python

* Change resources URI from unity:// to mcpforunity://

It should reduce conflicts with other Unity MCPs that users try, and to comply with Unity's requests regarding use of their company and product name

* Update README with all tools + better listing for resources

* Update other references to resources

* Updated translated doc - unfortunately I cannot verify

* Update the Chinese translation of the dev docks

* Change menu item from Setup Window to Local Setup Window

We now differentiate whether it's HTTP local or remote

* Fix URIs for menu items and tests

* Shouldn't have removed it

* fix: add missing FAST_FAIL_TIMEOUT constant in PluginHub

The FAST_FAIL_TIMEOUT class attribute was referenced on line 149 but never
defined, causing AttributeError on every ping attempt. This error was silently
caught by the broad 'except Exception' handler, causing all fast-fail commands
(read_console, get_editor_state, ping) to fail after 6 seconds of retries with
'ping not answered' error.

Added FAST_FAIL_TIMEOUT = 10 to define a 10-second timeout for fast-fail
commands, matching the intent of the existing fast-fail infrastructure.

* feat(ScriptableObject): enhance dry-run validation for AnimationCurve and Quaternion

Dry-run validation now validates value formats, not just property existence:

- AnimationCurve: Validates structure ({keys:[...]} or direct array), checks
  each keyframe is an object, validates numeric fields (time, value, inSlope,
  outSlope, inWeight, outWeight) and integer fields (weightedMode)
- Quaternion: Validates array length (3 for Euler, 4 for raw) or object
  structure ({x,y,z,w} or {euler:[x,y,z]}), ensures all components are numeric

Refactored shared validation helpers into appropriate locations:
- ParamCoercion: IsNumericToken, ValidateNumericField, ValidateIntegerField
- VectorParsing: ValidateAnimationCurveFormat, ValidateQuaternionFormat

Added comprehensive XML documentation clarifying keyframe field defaults
(all default to 0 except as noted).

Added 5 new dry-run validation tests covering valid and invalid formats
for both AnimationCurve and Quaternion properties.

* test: fix integration tests after merge

- test_refresh_unity_retry_recovery: Mock now handles both refresh_unity and
  get_editor_state commands (refresh_unity internally calls get_editor_state
  when wait_for_ready=True)
- test_run_tests_async_forwards_params: Mock response now includes required
  'mode' field for RunTestsStartResponse Pydantic validation
- test_get_test_job_forwards_job_id: Updated to handle GetTestJobResponse as
  Pydantic model instead of dict (use model_dump() for assertions)

* Update warning message to apply to all test modes

Follow-up to PR #527: Since SaveDirtyScenesIfNeeded() now runs for all test modes, update the warning message to say 'tests' instead of 'PlayMode tests'.

* feat(run_tests): add wait_timeout to get_test_job to avoid client loop detection

When polling for test completion, MCP clients like Cursor can detect the
repeated get_test_job calls as 'looping' and terminate the agent.

Added wait_timeout parameter that makes the server wait internally for tests
to complete (polling Unity every 2s) before returning. This dramatically
reduces client-side tool calls from 10-20 down to 1-2, avoiding loop detection.

Usage: get_test_job(job_id='xxx', wait_timeout=30)
- Returns immediately if tests complete within timeout
- Returns current status if timeout expires (client can call again)
- Recommended: 30-60 seconds

* fix: use Pydantic attribute access in test_run_tests_async for merge compatibility

* revert: remove local test harness - will be submitted in separate PR

* fix: stdio transport survives test runs without UI flicker

Root cause: WriteToConfigTests.TearDown() was unconditionally deleting
UseHttpTransport EditorPref even when tests were skipped on Windows
(NUnit runs TearDown even after Assert.Ignore).

Changes:
- Fix WriteToConfigTests to save/restore prefs instead of deleting
- Add centralized ShouldForceUvxRefresh() for local dev path detection
- Clean stale Python build/ artifacts before client configuration
- Improve reload handler flag management to prevent stuck Resuming state
- Show Resuming status during stdio bridge restart
- Initialize client config display on window open
- Add DevModeForceServerRefresh to EditorPrefs window known types

---------

Co-authored-by: Marcus Sanatan <msanatan@gmail.com>
Co-authored-by: Scott Jennings <scott.jennings+CIGINT@cloudimperiumgames.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EditMode tests blocked by "Save Scene" dialog when scene is dirty

2 participants