feat: Add PlayerPrefs Tool via MCP #339
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new Tool_PlayerPrefs to enable AI assistants to manage Unity's PlayerPrefs key-value storage system through the MCP (Model Context Protocol). The implementation follows the established codebase pattern of using partial classes split across multiple files, with each of the 7 operations (ExistsKey, GetKeyType, ReadKey, WriteKey, DeleteKey, DeleteAllKeys, Save) in its own file.
Key additions:
- Complete CRUD operations for PlayerPrefs with type inference
- Thread-safe execution using
MainThread.Instance.Run() - Comprehensive error handling with descriptive error messages
Regarding the author's question about Editor vs Runtime placement: The files are correctly placed in the Editor folder. All MCP Tools belong in Editor/Scripts/API/Tool/ because the MCP server runs only in Unity Editor, not in built games. Even though PlayerPrefs is a runtime API, the tool that manipulates it from the MCP server is an Editor-only feature, consistent with all other tools in the codebase (Console, GameObject, Assets, etc.).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
PlayerPrefs.cs |
Main partial class with error messages and type inference helper using heuristics since Unity doesn't expose PlayerPrefs types |
PlayerPrefs.ExistsKey.cs |
Checks key existence using PlayerPrefs.HasKey() |
PlayerPrefs.GetKeyType.cs |
Returns inferred type (int/float/string) using GetKeyValueAndType helper |
PlayerPrefs.ReadKey.cs |
Reads value and type, returns formatted success message |
PlayerPrefs.WriteKey.cs |
Writes typed values with enum parameter and parsing validation |
PlayerPrefs.DeleteKey.cs |
Deletes individual key with existence check |
PlayerPrefs.DeleteAllKeys.cs |
Deletes all keys with caution warning in description |
PlayerPrefs.Save.cs |
Forces disk write with platform-specific documentation |
*.meta files |
Unity metadata files for all new .cs files |
| public enum PlayerPrefsValueType | ||
| { | ||
| @int, | ||
| @float, | ||
| @string | ||
| } |
There was a problem hiding this comment.
The enum values use the @ prefix to allow C# keywords as identifiers (@int, @float, @string). While technically valid, this is generally discouraged as it can reduce code readability.
Recommendation: Use PascalCase enum naming without the @ prefix:
public enum PlayerPrefsValueType
{
Int,
Float,
String
}Then update the switch cases accordingly:
case PlayerPrefsValueType.Int:
// ...
case PlayerPrefsValueType.Float:
// ...
case PlayerPrefsValueType.String:
// ...This follows standard C# naming conventions and improves code clarity.
| private static (object value, string type) GetKeyValueAndType(string key) | ||
| { | ||
| // First, try to get as string | ||
| string stringValue = PlayerPrefs.GetString(key, string.Empty); | ||
|
|
||
| if (!string.IsNullOrEmpty(stringValue)) | ||
| { | ||
| // Try to parse as int | ||
| if (int.TryParse(stringValue, out int intResult)) | ||
| { | ||
| int intValue = PlayerPrefs.GetInt(key, int.MaxValue); | ||
| if (intValue != int.MaxValue && intValue == intResult) | ||
| return (intValue, "int"); | ||
| } | ||
|
|
||
| // Try to parse as float | ||
| if (float.TryParse(stringValue, out float floatResult)) | ||
| { | ||
| float floatValue = PlayerPrefs.GetFloat(key, float.NaN); | ||
| if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < 0.0001f) | ||
| return (floatValue, "float"); | ||
| } | ||
|
|
||
| return (stringValue, "string"); | ||
| } | ||
|
|
||
| // If string is empty, try int | ||
| int intVal = PlayerPrefs.GetInt(key, int.MinValue + 1); | ||
| if (intVal != int.MinValue + 1) | ||
| return (intVal, "int"); | ||
|
|
||
| // Try float | ||
| float floatVal = PlayerPrefs.GetFloat(key, float.NaN); | ||
| if (!float.IsNaN(floatVal)) | ||
| return (floatVal, "float"); | ||
|
|
||
| // Default to empty string | ||
| return (PlayerPrefs.GetString(key, string.Empty), "string"); | ||
| } |
There was a problem hiding this comment.
The type inference logic in GetKeyValueAndType has a potential bug. When a value stored as an int with the value int.MaxValue is retrieved, the sentinel value check on line 57 (intValue != int.MaxValue) will incorrectly exclude it, causing the method to fall through and potentially misidentify it as a float or string.
Similarly, if an int with value int.MinValue + 1 is stored, line 74's sentinel check will fail, causing incorrect type inference.
Recommendation: Use nullable return values or a different approach to distinguish "key doesn't exist" from "key has sentinel value". For example:
// For int check:
if (PlayerPrefs.HasKey(key)) {
int intValue = PlayerPrefs.GetInt(key);
if (intValue == intResult)
return (intValue, "int");
}Note: PlayerPrefs.HasKey() is already being checked in the calling methods, so this could be simplified with a different approach that doesn't rely on sentinel values.
| [McpPluginToolType] | ||
| public partial class Tool_PlayerPrefs | ||
| { | ||
| public static class Error | ||
| { | ||
| public static string KeyIsNullOrEmpty() | ||
| => "[Error] Key cannot be null or empty."; | ||
|
|
||
| public static string KeyDoesNotExist(string key) | ||
| => $"[Error] Key '{key}' does not exist."; | ||
|
|
||
| public static string ValueCannotBeNull() | ||
| => "[Error] Value cannot be null."; | ||
|
|
||
| public static string ValueTypeMustBeSpecified() | ||
| => "[Error] Value type must be specified ('int', 'float', or 'string')."; | ||
|
|
||
| public static string InvalidValueType(string valueType) | ||
| => $"[Error] Invalid value type '{valueType}'. Must be 'int', 'float', or 'string'."; | ||
|
|
||
| public static string FailedToWriteValue(string message) | ||
| => $"[Error] Failed to write value: {message}"; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Determines the value and type of a PlayerPrefs key. | ||
| /// Since PlayerPrefs doesn't expose the type, we try to infer it. | ||
| /// </summary> | ||
| private static (object value, string type) GetKeyValueAndType(string key) | ||
| { | ||
| // First, try to get as string | ||
| string stringValue = PlayerPrefs.GetString(key, string.Empty); | ||
|
|
||
| if (!string.IsNullOrEmpty(stringValue)) | ||
| { | ||
| // Try to parse as int | ||
| if (int.TryParse(stringValue, out int intResult)) | ||
| { | ||
| int intValue = PlayerPrefs.GetInt(key, int.MaxValue); | ||
| if (intValue != int.MaxValue && intValue == intResult) | ||
| return (intValue, "int"); | ||
| } | ||
|
|
||
| // Try to parse as float | ||
| if (float.TryParse(stringValue, out float floatResult)) | ||
| { | ||
| float floatValue = PlayerPrefs.GetFloat(key, float.NaN); | ||
| if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < 0.0001f) | ||
| return (floatValue, "float"); | ||
| } | ||
|
|
||
| return (stringValue, "string"); | ||
| } | ||
|
|
||
| // If string is empty, try int | ||
| int intVal = PlayerPrefs.GetInt(key, int.MinValue + 1); | ||
| if (intVal != int.MinValue + 1) | ||
| return (intVal, "int"); | ||
|
|
||
| // Try float | ||
| float floatVal = PlayerPrefs.GetFloat(key, float.NaN); | ||
| if (!float.IsNaN(floatVal)) | ||
| return (floatVal, "float"); | ||
|
|
||
| // Default to empty string | ||
| return (PlayerPrefs.GetString(key, string.Empty), "string"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The Tool_PlayerPrefs class lacks automated test coverage. Other tools in the codebase (e.g., Tool_Console, Tool_GameObject, Tool_Assets) have comprehensive test suites in Unity-MCP-Plugin/Assets/root/Tests/Editor/Tool/.
Consider adding a test file Unity-MCP-Plugin/Assets/root/Tests/Editor/Tool/PlayerPrefs/TestToolPlayerPrefs.cs with tests covering:
- ExistsKey operation (key exists, key doesn't exist, null/empty key)
- WriteKey with all value types (int, float, string)
- ReadKey and type inference logic
- GetKeyType accuracy
- DeleteKey and DeleteAllKeys operations
- Edge cases like int.MaxValue, int.MinValue, float.NaN, empty strings
While manual testing was performed (as documented in the PR description), automated tests ensure regression protection and maintain consistency with the codebase's testing practices.
| if (float.TryParse(stringValue, out float floatResult)) | ||
| { | ||
| float floatValue = PlayerPrefs.GetFloat(key, float.NaN); | ||
| if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < 0.0001f) |
There was a problem hiding this comment.
The float comparison tolerance of 0.0001f on line 65 may be too strict for certain float values, particularly very large or very small numbers. The current fixed epsilon doesn't scale with the magnitude of the value being compared.
For example, if storing a float value like 1000000.5f, the string representation and parse-back may introduce errors larger than 0.0001f due to floating-point precision limitations, causing the method to incorrectly identify the type as "string" instead of "float".
Recommendation: Use a relative epsilon or increase the tolerance. For example:
// Option 1: Relative comparison
float epsilon = Math.Max(Math.Abs(floatValue) * 1e-6f, 1e-6f);
if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < epsilon)
return (floatValue, "float");
// Option 2: Use a larger fixed epsilon
if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < 0.001f)
return (floatValue, "float");| if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < 0.0001f) | |
| float epsilon = Math.Max(Math.Abs(floatValue) * 1e-6f, 1e-6f); | |
| if (!float.IsNaN(floatValue) && Math.Abs(floatValue - floatResult) < epsilon) |
|
@IvanMurzak I added tests |
Awesome! Please take a look at the pull request review from Copilot and fix what looks important. Ping me when you done. Please do the same with your another pull request. |
| [Test] | ||
| public void Integration_LargeIntValue() | ||
| { | ||
| // Arrange | ||
| const int value = int.MaxValue; | ||
|
|
||
| // Act | ||
| var writeResult = _tool.WriteKey(TestKeyInt, value.ToString(), Tool_PlayerPrefs.PlayerPrefsValueType.@int); | ||
| ResultValidation(writeResult); | ||
|
|
||
| // Assert | ||
| Assert.AreEqual(value, PlayerPrefs.GetInt(TestKeyInt), "Large int value should be handled correctly."); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Integration_SmallIntValue() | ||
| { | ||
| // Arrange | ||
| const int value = int.MinValue; | ||
|
|
||
| // Act | ||
| var writeResult = _tool.WriteKey(TestKeyInt, value.ToString(), Tool_PlayerPrefs.PlayerPrefsValueType.@int); | ||
| ResultValidation(writeResult); | ||
|
|
||
| // Assert | ||
| Assert.AreEqual(value, PlayerPrefs.GetInt(TestKeyInt), "Small int value should be handled correctly."); | ||
| } |
There was a problem hiding this comment.
The tests for int.MaxValue and int.MinValue only verify that the values are written correctly using PlayerPrefs.GetInt(), but don't test reading them back using the ReadKey() or GetKeyType() methods. This misses a critical bug in the type inference logic (see comment on PlayerPrefs.cs lines 56-75) where these edge case values would be incorrectly identified as strings.
Add assertions like:
var readResult = _tool.ReadKey(TestKeyInt);
ResultValidationExpected(readResult, value.ToString(), "type: int");
var typeResult = _tool.GetKeyType(TestKeyInt);
ResultValidationExpected(typeResult, "type: int");
PlayerPrefsEx is advanced version of PlayerPrefs and it maintains basic types such as Bool, Int, Float, String, etc. In the same time it support complex data models. It would be much more flexible to utilize this API. |
|
@Ficksik please don't worry about failed tests, that happens because your forked repository doesn't have required Secrets Variables configured. If that won't be too much it would be cool if you can add them to make the test process simpler. If not - that is fine I will be able to test it using a temporary branch in this repo.
|
|
@IvanMurzak I'm done with both PP. There is no time to add test secrets at the moment. |
2 participants
Summary
Tool_PlayerPrefsfor managing Unity's PlayerPrefs key-value storage systemExistsKey,GetKeyType,ReadKey,WriteKey,DeleteKey,DeleteAllKeys,SaveNew Files
PlayerPrefs.cs[McpPluginToolType], error messages, andGetKeyValueAndTypehelperPlayerPrefs.ExistsKey.csPlayerPrefs.GetKeyType.csPlayerPrefs.ReadKey.csPlayerPrefs.WriteKey.csPlayerPrefs.DeleteKey.csPlayerPrefs.DeleteAllKeys.csPlayerPrefs.Save.csTest Results
PlayerPrefs_ExistsKeyPlayerPrefs_WriteKey (int)42PlayerPrefs_WriteKey (float)3.14PlayerPrefs_WriteKey (string)Hello MCP!PlayerPrefs_ReadKeyPlayerPrefs_GetKeyTypeint,float,stringtypesPlayerPrefs_DeleteKeyPlayerPrefs_DeleteAllKeysPlayerPrefs_SaveAll 7 operations tested and working correctly.
Known Behavior
During testing, observed that
PlayerPrefs.HasKey()may return a cached result immediately afterDeleteKeyorDeleteAllKeys. This is Unity's internal behavior - the key is actually deleted, butHasKey()may briefly reportTrue. Subsequent calls correctly returnFalse. This does not affect the tool's functionality.Notes
MainThread.Instance.Run()for Unity thread safetyI just don't know yet. Should I put these classes in the editor's folder or the runtime folder?