Fix tests-run state being wiped by PlayerPrefs.DeleteAll by Abhinav-0311 · Pull Request #870 · IvanMurzak/Unity-MCP

Abhinav-0311 · 2026-07-11T13:54:11Z

Fixes #863.\n\n## Summary\n- Moves the ests-run pending request id and response option state from PlayerPrefs to editor SessionState.\n- Keeps the state alive across domain reloads while preventing project/test code that calls PlayerPrefs.DeleteAll() from wiping the routing id mid-run.\n- Logs an explicit error if a completed/deferred test run cannot be routed because the request id is missing.\n\n## Testing\n- git diff --check\n- Not run: Unity editor tests / local compile because Unity and dotnet are not available in this environment.

IvanMurzak · 2026-07-12T00:51:52Z

Hi @Abhinav-0311, thanks for the contribution.

Could you please explain, why is it needed for?

PlayerPrefs survives domain reload already.

Abhinav-0311 · 2026-07-12T06:19:33Z

You’re right that PlayerPrefs survives domain reload. The reason for this change is different: tests can legitimately call PlayerPrefs.DeleteAll() during SetUp/TearDown to clean game state. In that case the test run starts normally and the request id is stored, but while the run is in flight the user test deletes all PlayerPrefs. When RunFinished later reads TestCallRequestID, it is empty, so NotifyToolRequestCompleted is never called and the MCP client waits indefinitely. SessionState still survives the editor domain reload case needed by tests-run, but it is editor-only session state and is not affected by game/test code calling PlayerPrefs.DeleteAll(). So this keeps the existing domain-reload behavior while protecting the MCP routing state from project test fixtures. That is the repro covered by #863.

…

On Sun, Jul 12, 2026 at 6:22 AM Ivan Murzak ***@***.***> wrote: *IvanMurzak* left a comment (IvanMurzak/Unity-MCP#870) <#870 (comment)> Hi @Abhinav-0311 <https://github.com/Abhinav-0311>, thanks for the contribution. Could you please explain, why is it needed for? PlayerPrefs survives domain reload already. — Reply to this email directly, view it on GitHub <#870?email_source=notifications&email_token=BKJBDPHTCFQRNZ4K2PQKVWL5ELOL3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJUHEZTQMZXG442M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4949383779>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKJBDPDPYM23IBP337BBGXT5ELOL3AVCNFSNUABFKJSXA33TNF2G64TZHM4TKOJRGIZTIOJYHNEXG43VMU5TIOBWGI2TINZZHA42C5QC> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/BKJBDPA62K2DG7CQJBTVOUL5ELOL3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJUHEZTQMZXG442M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/BKJBDPFMKMLHNOMCMO64IHL5ELOL3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJUHEZTQMZXG442M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>

IvanMurzak · 2026-07-13T23:56:13Z

@Abhinav-0311 Thanks for clarifying. Now I see the problem you want to fix. Hm... this PR doesn't fix it, it only moves the problem from PlayerPrefs.DeleteAll to EditorPrefs.DeleteAll. The data still could be deleted by tests run.

Abhinav-0311 · 2026-07-14T08:21:11Z

Thanks, that makes sense as a concern. One clarification: this PR is not moving the data to EditorPrefs; it moves the tests-run routing/options from PlayerPrefs to UnityEditor.SessionState.

So PlayerPrefs.DeleteAll() and EditorPrefs.DeleteAll() should not clear these values. They are stored through SessionState.GetString/SetString, GetBool/SetBool, and GetInt/SetInt.

The specific bug I am trying to fix is the common test-fixture cleanup case:

[TearDown]
public void TearDown()
{
    PlayerPrefs.DeleteAll();
}

That can currently wipe TestCallRequestID while the test run is in flight, so RunFinished cannot notify the MCP request. SessionState keeps the existing domain-reload behavior for the deferred compile path, but separates this MCP/editor routing state from project runtime PlayerPrefs cleanup.

It is still true that editor test code could intentionally erase the package's exact SessionState keys if it references them, but that is a different class of issue from broad game-state cleanup. If you would prefer to avoid SessionState for the in-flight run as well, I can rework the PR to keep the active request/options in the registered collector instance and use SessionState only for the pre-run compile/domain-reload handoff.

IvanMurzak · 2026-07-14T10:08:35Z

Hey @Abhinav-0311, thanks for clarifying your vision.
It sounds right to me. Session data is a different level, and we may ignore that fact that it could be erased too. Because usually nobody uses the Session data in tests.

Please resolve merge conflicts, and I will run tests on this PR.

# Conflicts: # Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Editor/Scripts/API/Tool/Tests.Run.cs # Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Editor/Scripts/API/Tool/Tests.cs

Abhinav-0311 · 2026-07-14T13:16:14Z

Resolved the merge conflicts and pushed the rebased branch.

While resolving against current main, I kept the newer active-test-run guard and moved its timestamp (ActiveTestRunStartedUtcTicksKey) to SessionState as well. That way a test calling PlayerPrefs.DeleteAll() cannot make the active run look stale and clear the in-flight request id.

Local verification:

git diff --check origin/main...HEAD

I did not run Unity editor tests locally from this checkout.

IvanMurzak · 2026-07-15T00:18:56Z

                ClearActiveTestRun(requestId);
+                if (string.IsNullOrEmpty(requestId))
+                {
+                    Debug.LogError("[TestRunner] Compilation failed after a deferred test run, but the pending MCP request id was missing. The error response cannot be delivered to the client.");


Need to use a proper logging wrapper from Unity-MCP project, that would respect the logging settings in AI Game Developer window.

IvanMurzak · 2026-07-15T00:19:18Z

            }
+            else
+            {
+                Debug.LogError("[TestRunner] Test run finished, but the pending MCP request id was missing. The test results cannot be delivered to the client.");


Need to use a proper logging wrapper from Unity-MCP project, that would respect the logging settings in AI Game Developer window.

IvanMurzak · 2026-07-15T00:19:55Z

            };
        }
+
+        public sealed class SessionStateString


Make a dedicated file for this new C# class

Abhinav-0311 · 2026-07-15T13:08:05Z

Addressed the requested changes in 52d09e3:

Replaced the two new direct Debug.LogError calls with UnityMcpPluginEditor.Instance.LogError(...), so they go through the Unity-MCP logging wrapper and respect the AI Game Developer logging settings.
Moved the SessionState wrappers out of TestResultCollector.cs.
Put each wrapper in its own file: SessionStateString.cs, SessionStateBool.cs, and SessionStateInt.cs.

Local verification:

git diff --cached --check
git diff --check

I did not run Unity editor tests locally from this checkout.

Abhinav-0311 requested a review from IvanMurzak as a code owner July 11, 2026 13:54

IvanMurzak added information-required and removed information-required labels Jul 13, 2026

IvanMurzak added the bug Something isn't working label Jul 13, 2026

IvanMurzak added the enhancement New feature or request label Jul 14, 2026

Keep test runner state out of PlayerPrefs

207bc1d

# Conflicts: # Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Editor/Scripts/API/Tool/Tests.Run.cs # Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Editor/Scripts/API/Tool/Tests.cs

Abhinav-0311 force-pushed the fix-tests-run-session-state branch from cb1984d to 207bc1d Compare July 14, 2026 13:15

Merge branch 'main' into fix-tests-run-session-state

22ea95d

IvanMurzak requested changes Jul 15, 2026

View reviewed changes

Address tests-run review feedback

52d09e3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix tests-run state being wiped by PlayerPrefs.DeleteAll#870

Fix tests-run state being wiped by PlayerPrefs.DeleteAll#870
Abhinav-0311 wants to merge 3 commits into
IvanMurzak:mainfrom
Abhinav-0311:fix-tests-run-session-state

Abhinav-0311 commented Jul 11, 2026

Uh oh!

IvanMurzak commented Jul 12, 2026

Uh oh!

Abhinav-0311 commented Jul 12, 2026 via email

Uh oh!

IvanMurzak commented Jul 13, 2026

Uh oh!

Abhinav-0311 commented Jul 14, 2026

Uh oh!

IvanMurzak commented Jul 14, 2026

Uh oh!

Abhinav-0311 commented Jul 14, 2026

Uh oh!

IvanMurzak Jul 15, 2026

Uh oh!

IvanMurzak Jul 15, 2026

Uh oh!

IvanMurzak Jul 15, 2026

Uh oh!

Abhinav-0311 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Abhinav-0311 commented Jul 11, 2026

Uh oh!

IvanMurzak commented Jul 12, 2026

Uh oh!

Abhinav-0311 commented Jul 12, 2026 via email

Uh oh!

IvanMurzak commented Jul 13, 2026

Uh oh!

Abhinav-0311 commented Jul 14, 2026

Uh oh!

IvanMurzak commented Jul 14, 2026

Uh oh!

Abhinav-0311 commented Jul 14, 2026

Uh oh!

IvanMurzak Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

IvanMurzak Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

IvanMurzak Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Abhinav-0311 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants