Skip to content

Env port fix #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 22, 2025
Merged

Env port fix #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ff507b5
Read HTTP_PORT from a config file on non-win32 systems.
SamyNarrainen May 21, 2025
2867edf
Write out server.env file from C# side.
SamyNarrainen May 21, 2025
876acb9
Replace reading of server settings via system env vars with a config …
SamyNarrainen May 22, 2025
ad4b08a
Update path to read from existing settings file.
SamyNarrainen May 22, 2025
63622d8
Use await on config file parse call.
SamyNarrainen May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 6 additions & 20 deletions Editor/UnityBridge/McpUnitySettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public class McpUnitySettings

private const string EnvUnityPort = "UNITY_PORT";
private const string EnvUnityRequestTimeout = "UNITY_REQUEST_TIMEOUT";
/// <remarks>
/// This file path is also read by the MCP server. Changes here will require updates to it. See mcpUnity.ts
/// </remarks>
private const string SettingsPath = "ProjectSettings/McpUnitySettings.json";

private static McpUnitySettings _instance;
Expand Down Expand Up @@ -72,18 +75,6 @@ public void LoadSettings()
string json = File.ReadAllText(SettingsPath);
JsonUtility.FromJsonOverwrite(json, this);
}

// Check for environment variable PORT
string envPort = System.Environment.GetEnvironmentVariable(EnvUnityPort);
if (!string.IsNullOrEmpty(envPort) && int.TryParse(envPort, out int port))
{
Port = port;
}
string envTimeout = System.Environment.GetEnvironmentVariable(EnvUnityRequestTimeout);
if (!string.IsNullOrEmpty(envTimeout) && int.TryParse(envTimeout, out int timeout))
{
RequestTimeoutSeconds = timeout;
}
}
catch (Exception ex)
{
Expand All @@ -95,21 +86,16 @@ public void LoadSettings()
/// <summary>
/// Save settings to disk
/// </summary>
/// <remarks>
/// WARNING: This file is also read by the MCP server. Changes here will require updates to it. See mcpUnity.ts
/// </remarks>
public void SaveSettings()
{
try
{
// Save settings to McpUnitySettings.json
string json = JsonUtility.ToJson(this, true);
File.WriteAllText(SettingsPath, json);

// Set environment variable PORT for the Node.js process
// EnvironmentVariableTarget.User and EnvironmentVariableTarget.Machine should be used on .NET implementations running on Windows systems only.
// For non-Windows systems, User and Machine are treated as Process.
// Using Process target for broader compatibility.
// see: https://learn.microsoft.com/en-us/dotnet/api/system.environmentvariabletarget?view=net-8.0#remarks
Environment.SetEnvironmentVariable(EnvUnityPort, Port.ToString(), EnvironmentVariableTarget.Process);
Environment.SetEnvironmentVariable(EnvUnityRequestTimeout, RequestTimeoutSeconds.ToString(), EnvironmentVariableTarget.Process);
}
catch (Exception ex)
{
Expand Down
2 changes: 1 addition & 1 deletion Editor/Utils/McpConfigUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static string GenerateMcpConfigJson(bool useTabsIndentation)
}

/// <summary>
/// Gets the absolute path to the Server directory containing package.json
/// Gets the absolute path to the Server directory containing package.json (root server dir).
/// Works whether MCP Unity is installed via Package Manager or directly in the Assets folder
/// </summary>
public static string GetServerPath()
Expand Down
17 changes: 8 additions & 9 deletions Server~/build/unity/mcpUnity.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ export declare class McpUnity {
private port;
private ws;
private pendingRequests;
private readonly REQUEST_TIMEOUT;
private requestTimeout;
private retryDelay;
constructor(logger: Logger);
/**
* Start the Unity connection
* @param clientName Optional name of the MCP client connecting to Unity
*/
start(clientName?: string): Promise<void>;
/**
* Reads our configuration file and sets parameters of the server based on them.
*/
private parseAndSetConfig;
/**
* Connect to the Unity WebSocket
* @param clientName Optional name of the MCP client connecting to Unity
Expand Down Expand Up @@ -48,14 +52,9 @@ export declare class McpUnity {
*/
get isConnected(): boolean;
/**
* Retrieves the UNITY_PORT value from the Windows registry (HKCU\Environment)
* @returns The port value as a string if found, otherwise an empty string
*/
private getUnityPortFromWindowsRegistry;
/**
* Retrieves the UNITY_PORT value from Unix-like system environment variables
* @returns The port value as a string if found, otherwise an empty string
* Read the McpUnitySettings.json file and return its contents as a JSON object.
* @returns a JSON object with the contents of the McpUnitySettings.json file.
*/
private getUnityPortFromUnixRegistry;
private readConfigFileAsJson;
}
export {};
75 changes: 36 additions & 39 deletions Server~/build/unity/mcpUnity.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,26 @@
import WebSocket from 'ws';
import { v4 as uuidv4 } from 'uuid';
import { McpUnityError, ErrorType } from '../utils/errors.js';
import { execSync } from 'child_process';
import { default as winreg } from 'winreg';
import { promises as fs } from 'fs';
import path from 'path';
export class McpUnity {
logger;
port;
port = null;
ws = null;
pendingRequests = new Map();
REQUEST_TIMEOUT;
requestTimeout = 10000;
retryDelay = 1000;
constructor(logger) {
this.logger = logger;
// Initialize port from environment variable or use default
const envRegistry = process.platform === 'win32'
? this.getUnityPortFromWindowsRegistry()
: this.getUnityPortFromUnixRegistry();
const envPort = process.env.UNITY_PORT || envRegistry;
this.port = envPort ? parseInt(envPort, 10) : 8090;
this.logger.info(`Using port: ${this.port} for Unity WebSocket connection`);
// Initialize timeout from environment variable (in seconds; it is the same as Cline) or use default (10 seconds)
const envTimeout = process.env.UNITY_REQUEST_TIMEOUT;
this.REQUEST_TIMEOUT = envTimeout ? parseInt(envTimeout, 10) * 1000 : 10000;
this.logger.info(`Using request timeout: ${this.REQUEST_TIMEOUT / 1000} seconds`);
}
/**
* Start the Unity connection
* @param clientName Optional name of the MCP client connecting to Unity
*/
async start(clientName) {
try {
this.logger.info('Attempting to read startup parameters...');
await this.parseAndSetConfig();
this.logger.info('Attempting to connect to Unity WebSocket...');
await this.connect(clientName); // Pass client name to connect
this.logger.info('Successfully connected to Unity WebSocket');
Expand All @@ -45,6 +36,19 @@ export class McpUnity {
}
return Promise.resolve();
}
/**
* Reads our configuration file and sets parameters of the server based on them.
*/
async parseAndSetConfig() {
const config = await this.readConfigFileAsJson();
const configPort = config.Port;
this.port = configPort ? parseInt(configPort, 10) : 8090;
this.logger.info(`Using port: ${this.port} for Unity WebSocket connection`);
// Initialize timeout from environment variable (in seconds; it is the same as Cline) or use default (10 seconds)
const configTimeout = config.RequestTimeoutSeconds;
this.requestTimeout = configTimeout ? parseInt(configTimeout, 10) * 1000 : 10000;
this.logger.info(`Using request timeout: ${this.requestTimeout / 1000} seconds`);
}
/**
* Connect to the Unity WebSocket
* @param clientName Optional name of the MCP client connecting to Unity
Expand Down Expand Up @@ -74,7 +78,7 @@ export class McpUnity {
this.disconnect();
reject(new McpUnityError(ErrorType.CONNECTION, 'Connection timeout'));
}
}, this.REQUEST_TIMEOUT);
}, this.requestTimeout);
this.ws.onopen = () => {
clearTimeout(connectionTimeout);
this.logger.debug('WebSocket connected');
Expand Down Expand Up @@ -193,12 +197,12 @@ export class McpUnity {
// Create timeout for the request
const timeout = setTimeout(() => {
if (this.pendingRequests.has(requestId)) {
this.logger.error(`Request ${requestId} timed out after ${this.REQUEST_TIMEOUT}ms`);
this.logger.error(`Request ${requestId} timed out after ${this.requestTimeout}ms`);
this.pendingRequests.delete(requestId);
reject(new McpUnityError(ErrorType.TIMEOUT, 'Request timed out'));
}
this.reconnect();
}, this.REQUEST_TIMEOUT);
}, this.requestTimeout);
// Store pending request
this.pendingRequests.set(requestId, {
resolve,
Expand All @@ -225,27 +229,20 @@ export class McpUnity {
return this.ws !== null && this.ws.readyState === WebSocket.OPEN;
}
/**
* Retrieves the UNITY_PORT value from the Windows registry (HKCU\Environment)
* @returns The port value as a string if found, otherwise an empty string
*/
getUnityPortFromWindowsRegistry() {
const regKey = new winreg({ hive: winreg.HKCU, key: '\\Environment' });
let result = '';
regKey.get('UNITY_PORT', (err, item) => {
if (err) {
this.logger.error(`Error getting registry value: ${err.message}`);
}
else {
result = item.value;
}
});
return result;
}
/**
* Retrieves the UNITY_PORT value from Unix-like system environment variables
* @returns The port value as a string if found, otherwise an empty string
* Read the McpUnitySettings.json file and return its contents as a JSON object.
* @returns a JSON object with the contents of the McpUnitySettings.json file.
*/
getUnityPortFromUnixRegistry() {
return execSync('printenv UNITY_PORT', { stdio: ['pipe', 'pipe', 'ignore'] }).toString().trim();
async readConfigFileAsJson() {
const configPath = path.resolve(process.cwd(), '../ProjectSettings/McpUnitySettings.json');
this.logger.debug(`Reading McpUnitySettings.json from ${configPath}`);
try {
const content = await fs.readFile(configPath, 'utf-8');
const json = JSON.parse(content);
return json;
}
catch (err) {
this.logger.debug(`McpUnitySettings.json not found or unreadable: ${err instanceof Error ? err.message : String(err)}`);
return {};
}
}
}
2 changes: 1 addition & 1 deletion Server~/build/utils/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export var ErrorType;
ErrorType["VALIDATION"] = "validation_error";
ErrorType["INTERNAL"] = "internal_error";
ErrorType["TIMEOUT"] = "timeout_error";
})(ErrorType || (ErrorType = {}));
})(ErrorType = ErrorType || (ErrorType = {}));
export class McpUnityError extends Error {
type;
details;
Expand Down
2 changes: 1 addition & 1 deletion Server~/build/utils/logger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export var LogLevel;
LogLevel[LogLevel["INFO"] = 1] = "INFO";
LogLevel[LogLevel["WARN"] = 2] = "WARN";
LogLevel[LogLevel["ERROR"] = 3] = "ERROR";
})(LogLevel || (LogLevel = {}));
})(LogLevel = LogLevel || (LogLevel = {}));
// Check environment variable for logging
const isLoggingEnabled = process.env.LOGGING === 'true';
// Check environment variable for logging in a file
Expand Down
81 changes: 39 additions & 42 deletions Server~/src/unity/mcpUnity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import WebSocket from 'ws';
import { v4 as uuidv4 } from 'uuid';
import { Logger } from '../utils/logger.js';
import { McpUnityError, ErrorType } from '../utils/errors.js';
import { execSync } from 'child_process';
import { default as winreg } from 'winreg';
import { promises as fs } from 'fs';
import path from 'path';

interface PendingRequest {
resolve: (value: any) => void;
Expand All @@ -30,28 +30,14 @@ interface UnityResponse {

export class McpUnity {
private logger: Logger;
private port: number;
private port: number | null = null;
private ws: WebSocket | null = null;
private pendingRequests: Map<string, PendingRequest> = new Map<string, PendingRequest>();
private readonly REQUEST_TIMEOUT: number;
private requestTimeout = 10000;
Comment on lines +33 to +36
Copy link
Contributor

@coderabbitai coderabbitai bot May 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Possible null port leak

this.port starts as null, and the very first connection attempt relies on it.
Without setting a sensible default (e.g. 8090) at declaration time you must guarantee it is initialised before any use; otherwise ws://localhost:null/McpUnity will be produced.
Because the assignment happens asynchronously (see next comment), the risk is non-theoretical.

🤖 Prompt for AI Agents 
In Server~/src/unity/mcpUnity.ts around lines 33 to 36, the private member
`port` is initialized to null, which can cause an invalid WebSocket URL like
`ws://localhost:null/McpUnity` on the first connection attempt. To fix this,
assign a sensible default port value (e.g., 8090) directly at declaration time
instead of null, ensuring `port` is never null when used to build the WebSocket
URL.

Copy link
Owner Author

Choose a reason for hiding this comment

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

good point, make it default to 8090

Copy link
Contributor

Choose a reason for hiding this comment

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

flowchart TB
    A["Set default port value in McpUnity class"]
    B["Change port declaration to default to 8090"]
    A --> B
Loading

@CoderGamester I'll implement that change for you. The port property will now default to 8090 instead of null, ensuring there's always a valid port value when attempting to connect to the WebSocket server.

The changes will be made to Server~/src/unity/mcpUnity.ts by an agent that will perform the actual code modifications. You'll receive a notification when the changes are ready for review.

⏳ Preparing changes...

Copy link
Contributor

Choose a reason for hiding this comment

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

A summary of the edits CodeRabbit can apply:

The port declaration in Server~/src/unity/mcpUnity.ts will be changed to default to 8090 instead of null.

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

✏️ Server~/src/unity/mcpUnity.ts

Replace the existing line
ts private port: number | null = null;
with
ts private port: number = 8090;
This ensures port is always initialized to a valid default.

Save the file and rebuild/restart the server. Verify that the WebSocket URL logged in connect() (e.g., ws://localhost:8090/McpUnity) uses port 8090 when no configuration override is provided.

A summary of the context that CodeRabbit is considering across the codebase:

Locate the existing port declaration in McpUnity to identify the exact line for modification.

  • ✅ Create PR with these edits
  • 📌 Create commit in current branch
  • 📋 Get copyable edits

private retryDelay = 1000;

constructor(logger: Logger) {
this.logger = logger;

// Initialize port from environment variable or use default
const envRegistry = process.platform === 'win32'
? this.getUnityPortFromWindowsRegistry()
: this.getUnityPortFromUnixRegistry();

const envPort = process.env.UNITY_PORT || envRegistry;
this.port = envPort ? parseInt(envPort, 10) : 8090;
this.logger.info(`Using port: ${this.port} for Unity WebSocket connection`);

// Initialize timeout from environment variable (in seconds; it is the same as Cline) or use default (10 seconds)
const envTimeout = process.env.UNITY_REQUEST_TIMEOUT;
this.REQUEST_TIMEOUT = envTimeout ? parseInt(envTimeout, 10) * 1000 : 10000;
this.logger.info(`Using request timeout: ${this.REQUEST_TIMEOUT / 1000} seconds`);
}

/**
Expand All @@ -60,6 +46,9 @@ export class McpUnity {
*/
public async start(clientName?: string): Promise<void> {
try {
this.logger.info('Attempting to read startup parameters...');
await this.parseAndSetConfig();

this.logger.info('Attempting to connect to Unity WebSocket...');
await this.connect(clientName); // Pass client name to connect
this.logger.info('Successfully connected to Unity WebSocket');
Expand All @@ -77,6 +66,22 @@ export class McpUnity {

return Promise.resolve();
}

/**
* Reads our configuration file and sets parameters of the server based on them.
*/
private async parseAndSetConfig() {
const config = await this.readConfigFileAsJson();

const configPort = config.Port;
this.port = configPort ? parseInt(configPort, 10) : 8090;
this.logger.info(`Using port: ${this.port} for Unity WebSocket connection`);

// Initialize timeout from environment variable (in seconds; it is the same as Cline) or use default (10 seconds)
const configTimeout = config.RequestTimeoutSeconds;
this.requestTimeout = configTimeout ? parseInt(configTimeout, 10) * 1000 : 10000;
this.logger.info(`Using request timeout: ${this.requestTimeout / 1000} seconds`);
}

/**
* Connect to the Unity WebSocket
Expand Down Expand Up @@ -112,7 +117,7 @@ export class McpUnity {
this.disconnect();
reject(new McpUnityError(ErrorType.CONNECTION, 'Connection timeout'));
}
}, this.REQUEST_TIMEOUT);
}, this.requestTimeout);

this.ws.onopen = () => {
clearTimeout(connectionTimeout);
Expand Down Expand Up @@ -249,12 +254,12 @@ export class McpUnity {
// Create timeout for the request
const timeout = setTimeout(() => {
if (this.pendingRequests.has(requestId)) {
this.logger.error(`Request ${requestId} timed out after ${this.REQUEST_TIMEOUT}ms`);
this.logger.error(`Request ${requestId} timed out after ${this.requestTimeout}ms`);
this.pendingRequests.delete(requestId);
reject(new McpUnityError(ErrorType.TIMEOUT, 'Request timed out'));
}
this.reconnect();
}, this.REQUEST_TIMEOUT);
}, this.requestTimeout);

// Store pending request
this.pendingRequests.set(requestId, {
Expand Down Expand Up @@ -282,29 +287,21 @@ export class McpUnity {
// Basic WebSocket connection check
return this.ws !== null && this.ws.readyState === WebSocket.OPEN;
}

/**
* Retrieves the UNITY_PORT value from the Windows registry (HKCU\Environment)
* @returns The port value as a string if found, otherwise an empty string
*/
private getUnityPortFromWindowsRegistry(): string {
const regKey = new winreg({hive: winreg.HKCU, key: '\\Environment'});
let result = '';
regKey.get('UNITY_PORT', (err: Error | null, item: any) => {
if (err) {
this.logger.error(`Error getting registry value: ${err.message}`);
} else {
result = item.value;
}
});
return result;
}

/**
* Retrieves the UNITY_PORT value from Unix-like system environment variables
* @returns The port value as a string if found, otherwise an empty string
* Read the McpUnitySettings.json file and return its contents as a JSON object.
* @returns a JSON object with the contents of the McpUnitySettings.json file.
*/
private getUnityPortFromUnixRegistry(): string {
return execSync('printenv UNITY_PORT', { stdio: ['pipe', 'pipe', 'ignore'] }).toString().trim();
private async readConfigFileAsJson(): Promise<any> {
const configPath = path.resolve(process.cwd(), '../ProjectSettings/McpUnitySettings.json');
this.logger.debug(`Reading McpUnitySettings.json from ${configPath}`);
try {
const content = await fs.readFile(configPath, 'utf-8');
const json = JSON.parse(content);
return json;
} catch (err) {
this.logger.debug(`McpUnitySettings.json not found or unreadable: ${err instanceof Error ? err.message : String(err)}`);
return {};
}
}
}