feat(Lua): Mcp server - #1000
Open
karolak6612 wants to merge 7 commits into
Open
Conversation
- Added C++ backend using Boost.Asio/Beast to handle HTTP JSON-RPC MCP requests on a background thread. - Thread-safe queue dispatcher placed in `MainFrame::OnIdle` to process tools safely on the main UI thread. - Exposed `startMcpServer`, `stopMcpServer`, and `setMcpHandler` to `lua_api_app.cpp`. - Authored pure Lua script `mcp_server.lua` that registers 30 robust map manipulation tools (tile changes, selections, spawning, drawing, object info, etc) allowing external AI editors (Cursor/Claude) to interact directly with the map editor. Co-authored-by: karolak6612 <88726580+karolak6612@users.noreply.github.com>
- Added Boost.Asio/Beast HTTP backend running on a background thread. - Asynchronously processes JSON-RPC requests on the main thread queue via `wxWakeUpIdle()` to avoid GUI freezing. - Exposed `app.startMcpServer(port)` and `app.setMcpHandler(callback)` to Lua context. - Implemented 30 powerful map manipulation tools natively in `mcp_server.lua` allowing Cursor/Claude to edit the map natively. - Added a Node.js stdio-to-HTTP bridge (`mcp_bridge.js`) so AI clients can connect to RME using standard MCP stdio protocol config `mcp.json`. Co-authored-by: karolak6612 <88726580+karolak6612@users.noreply.github.com>
- Added Boost.Asio/Beast HTTP backend running on a background thread. - Added `boost-beast` to `vcpkg.json` and `conanfile.py` to fix CI compilation. - Thread-safe queues securely route requests to the Main UI thread using `wxWakeUpIdle()` avoiding GUI freezes. - `app.startMcpServer`, `app.stopMcpServer`, and `app.setMcpHandler` exposed to Lua. - Implemented 30 powerful map manipulation tools completely in `mcp_server.lua` providing live-editable AI commands for mapping tasks. - Included `mcp.json` and a Node.js `mcp_bridge.js` to translate line-delimited `stdio` JSON-RPC (used by Cursor/Claude Desktop) into HTTP POST requests. Co-authored-by: karolak6612 <88726580+karolak6612@users.noreply.github.com>
- Rewrote `mcp_bridge.js` to ensure the Node.js syntax is correct and gracefully handles HTTP timeouts and parsing errors with proper JSON-RPC error responses as requested by Code Review. - Removed the garbage `.js` script files (`fix_mcp.js`, `fix_mcp_bridge.js`, `fix_save.js`) that were inadvertently left in the workspace and tracked in the previous commit. - Polished `data/scripts/mcp/mcp_server.lua` by cleaning up the awkward double negation (`not not app.selection.isEmpty`) into standard `app.selection.isEmpty` syntax. Co-authored-by: karolak6612 <88726580+karolak6612@users.noreply.github.com>
- Updated CMake `find_package(Boost ...)` to version `1.66.0` to guarantee `boost-beast` compatibility in older Linux workflows. - Rewrote the JSON response mechanisms in `mcp_server.cpp` and `lua_api_app.cpp` to correctly deserialize the incoming `id` and reflect it in JSON-RPC error responses. - Optimized RME's `MainFrame::OnIdle` to not blindly invoke `event.RequestMore()`, drastically lowering UI thread CPU spin overhead. - Removed the dangerous `execute_lua` tool. - Refactored `mcp_server.lua`'s `replace_in_selection` loop to iterate backwards via standard indexing, making in-place list modification totally safe. - Corrected the parameter schema of `set_light` and removed missing global `map.getTowns` calls. - Fixed `mcp_bridge.js` correctly routing timeout/error promises. Co-authored-by: karolak6612 <88726580+karolak6612@users.noreply.github.com>
Refactored MCP server to use new tile helper functions (getTile, getOrCreateTile) replacing deprecated Tile(Position()) API. Updated all tile manipulation tools to use property-based access (t.ground, t.items, t.houseId) instead of method calls. Added 6 new discovery tools: search_items, list_brushes, get_brush_info, list_creatures, find_item_id, get_brush_count. Implemented fill_empty_area tool for smart-fill using neighbor analysis. Moved server file from data/scripts/mcp/ to scripts/mcp/.
|
🤖 Hi @karolak6612, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
📝 WalkthroughWalkthroughThis PR adds MCP (Model Context Protocol) server support to the editor, enabling remote tool invocation via JSON-RPC 2.0. It includes a C++ HTTP server using Boost.Beast, Lua-based MCP tools for map and editor operations, an HTTP-to-stdio bridge, and updated Boost dependencies. Changes
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant Bridge as mcp_bridge.js
participant MCPServer as C++ MCP Server<br/>(Boost.Beast)
participant LuaHandler as Lua Handler<br/>(mcp_server.lua)
participant Editor as Editor App<br/>(app context)
Client->>Bridge: JSON-RPC request (stdin)
Bridge->>Bridge: Parse JSON-RPC, extract id
Bridge->>MCPServer: HTTP POST /
MCPServer->>MCPServer: Queue PendingRequest
MCPServer-->>Bridge: HTTP 200 (immediate)
Editor->>MCPServer: processPendingRequests()
MCPServer->>LuaHandler: setHandler callback<br/>(request string)
LuaHandler->>Editor: Execute tool<br/>(e.g., set tile, fill area)
Editor-->>LuaHandler: Result
LuaHandler-->>MCPServer: JSON-RPC response
MCPServer->>MCPServer: Send async HTTP response
Bridge->>Bridge: Read response body
Bridge-->>Client: JSON-RPC response (stdout)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
🤖 I'm sorry @karolak6612, but I was unable to process your request. Please see the logs for more details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
scripts/mcp/mcp_server.lua (1)
1061-1061: Note: Templates are volatile across script reloads.The
activeTemplatestable is local to the script, so captured templates will be lost if the script is reloaded. This is probably acceptable for session-scoped templates, but worth documenting in the tool description if users might expect persistence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/mcp/mcp_server.lua` at line 1061, The activeTemplates table is defined as a local variable (activeTemplates) so its contents are lost on script reloads; update the tool/docs to state that templates are volatile across script reloads (session-scoped only) and, if persistence is required, replace or augment activeTemplates with a persistent store (e.g., save/load from disk or external state) or expose a persistence API; mention activeTemplates and the session-scoped behavior in the tool description so users know templates are not retained after reloads.source/mcp/mcp_server.cpp (1)
201-209: Consider adding a read timeout to prevent connection hangs.The
http::async_readhas no timeout, so a malicious or slow client could hold a connection open indefinitely, exhausting server resources over time.Add timeout using beast's expires_after
void read_request() { auto self = shared_from_this(); + // Set a 30-second timeout for reading the request + stream.expires_after(std::chrono::seconds(30)); http::async_read(stream, buffer, req, [self](beast::error_code ec, std::size_t bytes_transferred) { boost::ignore_unused(bytes_transferred); if (!ec) { self->process_request(); } }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/mcp/mcp_server.cpp` around lines 201 - 209, Add a read timeout around the async read in read_request(): before calling http::async_read on stream, call stream.expires_after(std::chrono::seconds(N)) to set a sensible timeout (e.g. 30s); in the async_read completion lambda clear the deadline (stream.expires_never()) and proceed with process_request() on success; also ensure timeout events close the connection (handle expired deadline by shutting down/closing the socket or letting the async_read return with operation_aborted) so malicious/slow clients cannot hang the connection. Use the existing read_request(), stream, buffer, and req symbols to locate and update the call and its completion handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/scripts/mcp/mcp_server.lua`:
- Around line 1-1267: This file is a near-exact duplicate of the other MCP
server (causing duplicated logic and duplicated bugs like the undefined
isNpcType), so pick a single canonical implementation (keep one copy with
mcp.registerTool and mcp.handleRequest as the source of truth) and remove the
duplicate file OR refactor this file to simply require and return the canonical
module; ensure the canonical module returns the mcp table (exposing
mcp.registerTool, mcp.handleRequest, etc.), remove duplicate server-start logic
from the duplicate, and update any callers/imports to require the single module
so tools and functions (e.g., getTile/getOrCreateTile, fill_empty_area,
capture_building_template, and isNpcType usage) are only defined once.
In `@mcp_bridge.js`:
- Around line 58-63: The current res.on('end' ...) handler forwards raw
responseData to stdout (via console.log(responseData.trim())) which can emit
invalid MCP protocol when backend returns non-JSON; update the res.on('end'...)
logic (the responded flag, responseData handling) to validate and parse
responseData as JSON (and optionally confirm JSON-RPC shape if expected) before
forwarding: attempt JSON.parse(responseData.trim()), if parsing succeeds (and
passes any minimal JSON-RPC checks) then console.log the serialized JSON output,
otherwise do not write the raw body to stdout—log a clear error or write a
structured MCP error response instead and ensure responded is set to true to
avoid duplicate responses.
- Around line 25-29: When parsing incoming lines, distinguish JSON-RPC
notifications by checking parsed.id === undefined (instead of mapping missing id
to null) and record an isNotification boolean; set reqId only when parsed.id !==
undefined (or set reqId = parsed.id when present, otherwise leave null) so you
can tell notifications apart from requests. Then, before sending any error or
timeout responses in the existing send/error paths (the blocks that currently
always emit errors after parsing and on timeouts), conditionally suppress those
responses when isNotification is true; keep emitting Parse error responses from
the JSON.parse catch (since you cannot determine notification state on malformed
JSON). Update the code paths that reference reqId/parsed (the JSON.parse block
that sets reqId and the error/timeout response emitters) to use the new
isNotification check to avoid replying to notifications.
In `@mcp.json`:
- Line 5: The args entry currently uses a non-absolute placeholder
"<PATH_TO_RME_REPO>/mcp_bridge.js" which will fail at runtime; fix by either
replacing that placeholder in the "args" array with the absolute path to
mcp_bridge.js or add a "cwd" field to the same mcp.json object so the relative
path resolves (i.e., update the "args" array item or add "cwd" so the process
can locate mcp_bridge.js).
In `@scripts/mcp/mcp_server.lua`:
- Around line 768-769: The code calls an undefined function isNpcType in the
list_creatures tool (when computing matchNpc), which will error at runtime; add
a local function named isNpcType(name) near the helpers/top of the file that
implements the desired NPC detection (for now implement a simple heuristic such
as downcasing the name and checking for substrings like "npc", "citizen",
"merchant" or other project-specific markers) or, if the NPC filter is not
ready, remove the npcOnly filter usage in list_creatures (the matchNpc line) so
the feature is disabled until a proper implementation exists; ensure the symbol
isNpcType is declared local and referenced by list_creatures.
In `@source/mcp/mcp_server.cpp`:
- Around line 75-108: The stop() method returns before the background
server_thread has finished, allowing Sessions to run and enqueue requests after
the queue was drained; modify stop() to call ioc.stop() and then if
(server_thread.joinable()) server_thread.join() so the method blocks until the
background thread exits, ensuring processPendingRequests() and handler teardown
(e.g., Application::setHandler(nullptr)) cannot run against a still-active
thread—use the existing server_thread identifier and check joinable() to avoid
deadlocks or double-joins.
---
Nitpick comments:
In `@scripts/mcp/mcp_server.lua`:
- Line 1061: The activeTemplates table is defined as a local variable
(activeTemplates) so its contents are lost on script reloads; update the
tool/docs to state that templates are volatile across script reloads
(session-scoped only) and, if persistence is required, replace or augment
activeTemplates with a persistent store (e.g., save/load from disk or external
state) or expose a persistence API; mention activeTemplates and the
session-scoped behavior in the tool description so users know templates are not
retained after reloads.
In `@source/mcp/mcp_server.cpp`:
- Around line 201-209: Add a read timeout around the async read in
read_request(): before calling http::async_read on stream, call
stream.expires_after(std::chrono::seconds(N)) to set a sensible timeout (e.g.
30s); in the async_read completion lambda clear the deadline
(stream.expires_never()) and proceed with process_request() on success; also
ensure timeout events close the connection (handle expired deadline by shutting
down/closing the socket or letting the async_read return with operation_aborted)
so malicious/slow clients cannot hang the connection. Use the existing
read_request(), stream, buffer, and req symbols to locate and update the call
and its completion handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 740c293f-763d-4b96-a913-62e432fcfcb2
📒 Files selected for processing (13)
CMakeLists.txtconanfile.pydata/scripts/mcp/mcp_server.luamcp.jsonmcp_bridge.jsscripts/mcp/mcp_server.luasource/CMakeLists.txtsource/app/application.cppsource/lua/lua_api_app.cppsource/mcp/mcp_server.cppsource/mcp/mcp_server.hsource/ui/main_frame.cppvcpkg.json
Comment on lines
+1
to
+1267
| local MCP_PORT = 8080 | ||
|
|
||
| local mcp = {} | ||
| mcp.tools = {} | ||
|
|
||
| -- Register a tool | ||
| function mcp.registerTool(name, description, inputSchema, callback) | ||
| mcp.tools[name] = { | ||
| name = name, | ||
| description = description, | ||
| inputSchema = inputSchema, | ||
| callback = callback | ||
| } | ||
| end | ||
|
|
||
| -- Handle incoming JSON-RPC | ||
| function mcp.handleRequest(requestStr) | ||
| local ok, req = pcall(json.decode, requestStr) | ||
| if not ok or type(req) ~= "table" then | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| error = { code = -32700, message = "Parse error" }, | ||
| id = nil | ||
| }) | ||
| end | ||
|
|
||
| local id = req.id | ||
|
|
||
| if req.method == "initialize" then | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| result = { | ||
| protocolVersion = "2024-11-05", | ||
| capabilities = { tools = {} }, | ||
| serverInfo = { name = "RME-MCP", version = "1.0.0" } | ||
| }, | ||
| id = id | ||
| }) | ||
| elseif req.method == "notifications/initialized" then | ||
| return "" -- No response needed for notifications | ||
| elseif req.method == "tools/list" then | ||
| local toolList = {} | ||
| for _, t in pairs(mcp.tools) do | ||
| table.insert(toolList, { | ||
| name = t.name, | ||
| description = t.description, | ||
| inputSchema = t.inputSchema | ||
| }) | ||
| end | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| result = { tools = toolList }, | ||
| id = id | ||
| }) | ||
| elseif req.method == "tools/call" then | ||
| if not req.params or not req.params.name then | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| error = { code = -32602, message = "Invalid params: name is required" }, | ||
| id = id | ||
| }) | ||
| end | ||
|
|
||
| local tool = mcp.tools[req.params.name] | ||
| if not tool then | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| error = { code = -32601, message = "Tool not found" }, | ||
| id = id | ||
| }) | ||
| end | ||
|
|
||
| local args = req.params.arguments or {} | ||
| local success, result = pcall(tool.callback, args) | ||
|
|
||
| if not success then | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| result = { | ||
| content = {{ type = "text", text = "Error: " .. tostring(result) }}, | ||
| isError = true | ||
| }, | ||
| id = id | ||
| }) | ||
| end | ||
|
|
||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| result = { | ||
| content = {{ type = "text", text = type(result) == "string" and result or json.encode(result) }} | ||
| }, | ||
| id = id | ||
| }) | ||
| else | ||
| return json.encode({ | ||
| jsonrpc = "2.0", | ||
| error = { code = -32601, message = "Method not found" }, | ||
| id = id | ||
| }) | ||
| end | ||
| end | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Helper: get a tile from map (read-only, returns nil if not found) | ||
| -------------------------------------------------------------------------------- | ||
| local function getTile(x, y, z) | ||
| if not app.hasMap() then return nil end | ||
| return app.map:getTile(x, y, z) | ||
| end | ||
|
|
||
| -- Helper: get or create a tile (for modifications) | ||
| local function getOrCreateTile(x, y, z) | ||
| if not app.hasMap() then return nil end | ||
| return app.map:getOrCreateTile(x, y, z) | ||
| end | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- MCP Tools Implementation | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| mcp.registerTool("get_map_info", "Returns basic map information like width, height, and description.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| if not app.hasMap() then return "No map loaded." end | ||
| local m = app.map | ||
| return json.encode({ | ||
| name = m.name, | ||
| width = m.width, | ||
| height = m.height, | ||
| description = m.description, | ||
| tileCount = m.tileCount | ||
| }) | ||
| end) | ||
|
|
||
| mcp.registerTool("get_tile", "Returns detailed info of items on a specific tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, | ||
| y = { type = "number" }, | ||
| z = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| local t = getTile(args.x, args.y, args.z) | ||
| if not t then return "Tile not found." end | ||
| local items = {} | ||
| for _, item in ipairs(t.items) do | ||
| table.insert(items, {id = item.id, name = item.name}) | ||
| end | ||
| return json.encode({ | ||
| ground = t.ground and t.ground.id or nil, | ||
| items = items, | ||
| houseId = t.houseId, | ||
| hasCreature = t.hasCreature, | ||
| hasSpawn = t.hasSpawn | ||
| }) | ||
| end) | ||
|
|
||
| mcp.registerTool("set_tile_ground", "Replaces the ground item of a tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, | ||
| y = { type = "number" }, | ||
| z = { type = "number" }, | ||
| itemId = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z", "itemId" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.transaction("Set Ground", function() | ||
| local t = getOrCreateTile(args.x, args.y, args.z) | ||
| if t then | ||
| t.ground = args.itemId | ||
| end | ||
| end) | ||
| return "Ground set successfully." | ||
| end) | ||
|
|
||
| mcp.registerTool("add_item", "Adds an item to a specific tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, | ||
| y = { type = "number" }, | ||
| z = { type = "number" }, | ||
| itemId = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z", "itemId" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.transaction("Add Item", function() | ||
| local t = getOrCreateTile(args.x, args.y, args.z) | ||
| if t then | ||
| t:addItem(args.itemId) | ||
| end | ||
| end) | ||
| return "Item added." | ||
| end) | ||
|
|
||
| mcp.registerTool("clear_tile", "Removes all items from a tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, | ||
| y = { type = "number" }, | ||
| z = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| local t = getTile(args.x, args.y, args.z) | ||
| if not t then return "Tile not found (already empty)." end | ||
| app.transaction("Clear Tile", function() | ||
| -- Remove all items | ||
| local items = t.items | ||
| for i = #items, 1, -1 do | ||
| t:removeItem(items[i]) | ||
| end | ||
| -- Remove ground | ||
| t.ground = nil | ||
| end) | ||
| return "Tile cleared." | ||
| end) | ||
|
|
||
| mcp.registerTool("get_selection", "Returns the bounding box of the current selection.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| if app.selection.isEmpty then return "No selection." end | ||
| local startPos = app.selection.minPosition | ||
| local endPos = app.selection.maxPosition | ||
| return string.format("Selection from (X:%d Y:%d Z:%d) to (X:%d Y:%d Z:%d)", startPos.x, startPos.y, startPos.z, endPos.x, endPos.y, endPos.z) | ||
| end) | ||
|
|
||
| mcp.registerTool("fill_selection_ground", "Fills the selected area ground with a specific item ID.", { | ||
| type = "object", | ||
| properties = { itemId = { type = "number" } }, | ||
| required = { "itemId" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
| local map = app.map | ||
| app.transaction("Fill Selection Ground", function() | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| for z = minP.z, maxP.z do | ||
| local t = map:getOrCreateTile(x, y, z) | ||
| if t then | ||
| t.ground = args.itemId | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end) | ||
| return "Selection filled." | ||
| end) | ||
|
|
||
| mcp.registerTool("create_castle_template", "Creates a simple castle wall boundary around the current selection.", { | ||
| type = "object", | ||
| properties = { | ||
| wallId = { type = "number" }, | ||
| floorId = { type = "number" } | ||
| }, | ||
| required = { "wallId", "floorId" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
|
|
||
| app.transaction("Create Castle", function() | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| local map = app.map | ||
|
|
||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| for z = minP.z, maxP.z do | ||
| local t = map:getOrCreateTile(x, y, z) | ||
| if t then | ||
| t.ground = args.floorId | ||
| if x == minP.x or x == maxP.x or y == minP.y or y == maxP.y then | ||
| t:addItem(args.wallId) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end) | ||
| return "Castle template created in selection." | ||
| end) | ||
|
|
||
| mcp.registerTool("undo", "Undoes the last map action.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| app.editor:undo() | ||
| return "Action undone." | ||
| end) | ||
|
|
||
| mcp.registerTool("redo", "Redoes the last undone action.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| app.editor:redo() | ||
| return "Action redone." | ||
| end) | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Start Server | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| if not app.isMcpServerRunning() then | ||
| app.setMcpHandler(mcp.handleRequest) | ||
| if app.startMcpServer(MCP_PORT) then | ||
| print("MCP Server successfully started on port " .. tostring(MCP_PORT) .. "!") | ||
| else | ||
| print("Failed to start MCP Server. Port might be in use.") | ||
| end | ||
| else | ||
| -- Server already running, just update the handler and tools | ||
| app.setMcpHandler(mcp.handleRequest) | ||
| print("MCP Server tools reloaded successfully.") | ||
| end | ||
|
|
||
| mcp.registerTool("replace_in_selection", "Replaces one item ID with another within the selected area.", { | ||
| type = "object", | ||
| properties = { | ||
| oldId = { type = "number" }, | ||
| newId = { type = "number" } | ||
| }, | ||
| required = { "oldId", "newId" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
| local count = 0 | ||
| local map = app.map | ||
| app.transaction("Replace Selection", function() | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| for z = minP.z, maxP.z do | ||
| local t = map:getTile(x, y, z) | ||
| if t then | ||
| -- Check ground | ||
| if t.ground and t.ground.id == args.oldId then | ||
| t.ground = args.newId | ||
| count = count + 1 | ||
| end | ||
| -- Check top items | ||
| local items = t.items | ||
| for i = #items, 1, -1 do | ||
| local it = items[i] | ||
| if it.id == args.oldId then | ||
| t:removeItem(it) | ||
| t:addItem(args.newId) | ||
| count = count + 1 | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end) | ||
| return "Replaced " .. tostring(count) .. " items." | ||
| end) | ||
|
|
||
| mcp.registerTool("count_items_in_selection", "Counts the occurrences of a specific item ID in the selection.", { | ||
| type = "object", | ||
| properties = { itemId = { type = "number" } }, | ||
| required = { "itemId" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
| local count = 0 | ||
| local map = app.map | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| for z = minP.z, maxP.z do | ||
| local t = map:getTile(x, y, z) | ||
| if t then | ||
| if t.ground and t.ground.id == args.itemId then count = count + 1 end | ||
| for _, it in ipairs(t.items) do | ||
| if it.id == args.itemId then count = count + 1 end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| return "Found " .. tostring(count) .. " items." | ||
| end) | ||
|
|
||
| mcp.registerTool("set_selection", "Sets the map selection box.", { | ||
| type = "object", | ||
| properties = { | ||
| x1 = { type = "number" }, y1 = { type = "number" }, z1 = { type = "number" }, | ||
| x2 = { type = "number" }, y2 = { type = "number" }, z2 = { type = "number" } | ||
| }, | ||
| required = { "x1", "y1", "z1", "x2", "y2", "z2" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.selection:clear() | ||
| local minX = math.min(args.x1, args.x2) | ||
| local maxX = math.max(args.x1, args.x2) | ||
| local minY = math.min(args.y1, args.y2) | ||
| local maxY = math.max(args.y1, args.y2) | ||
| local minZ = math.min(args.z1, args.z2) | ||
| local maxZ = math.max(args.z1, args.z2) | ||
| local map = app.map | ||
|
|
||
| app.selection:start() | ||
| for x = minX, maxX do | ||
| for y = minY, maxY do | ||
| for z = minZ, maxZ do | ||
| local t = map:getTile(x, y, z) | ||
| if t then | ||
| app.selection:add(t) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| app.selection:finish() | ||
| return "Selection set." | ||
| end) | ||
|
|
||
| mcp.registerTool("remove_item", "Removes a specific item from a tile by ID.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" }, | ||
| itemId = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z", "itemId" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| local t = getTile(args.x, args.y, args.z) | ||
| if not t then return "Tile not found." end | ||
| local removed = false | ||
| app.transaction("Remove Item", function() | ||
| for _, it in ipairs(t.items) do | ||
| if it.id == args.itemId then | ||
| t:removeItem(it) | ||
| removed = true | ||
| break | ||
| end | ||
| end | ||
| end) | ||
| return removed and "Item removed." or "Item not found on tile." | ||
| end) | ||
|
|
||
| mcp.registerTool("draw_brush", "Simulates drawing with a specific brush at a coordinate.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" }, | ||
| brushName = { type = "string" } | ||
| }, | ||
| required = { "x", "y", "z", "brushName" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.setBrush(args.brushName) | ||
| app.transaction("Draw Brush " .. args.brushName, function() | ||
| local t = getOrCreateTile(args.x, args.y, args.z) | ||
| if t then | ||
| t:applyBrush(args.brushName) | ||
| end | ||
| end) | ||
| return "Brush drawn." | ||
| end) | ||
|
|
||
| mcp.registerTool("draw_doodad", "Simulates drawing a doodad at a coordinate.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" }, | ||
| doodadName = { type = "string" } | ||
| }, | ||
| required = { "x", "y", "z", "doodadName" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.setBrush(args.doodadName) | ||
| app.transaction("Draw Doodad " .. args.doodadName, function() | ||
| local t = getOrCreateTile(args.x, args.y, args.z) | ||
| if t then | ||
| t:applyBrush(args.doodadName) | ||
| end | ||
| end) | ||
| return "Doodad drawn." | ||
| end) | ||
|
|
||
| mcp.registerTool("draw_wall", "Draws a line of walls between two points.", { | ||
| type = "object", | ||
| properties = { | ||
| x1 = { type = "number" }, y1 = { type = "number" }, z1 = { type = "number" }, | ||
| x2 = { type = "number" }, y2 = { type = "number" }, z2 = { type = "number" }, | ||
| wallId = { type = "number" } | ||
| }, | ||
| required = { "x1", "y1", "z1", "x2", "y2", "z2", "wallId" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| if args.z1 ~= args.z2 then return "Points must be on the same Z level." end | ||
| local map = app.map | ||
| local points = geo.bresenhamLine(args.x1, args.y1, args.x2, args.y2) | ||
| app.transaction("Draw Wall", function() | ||
| for _, pt in ipairs(points) do | ||
| local t = map:getOrCreateTile(pt.x, pt.y, args.z1) | ||
| if t then | ||
| t:addItem(args.wallId) | ||
| end | ||
| end | ||
| end) | ||
| return "Wall drawn." | ||
| end) | ||
|
|
||
| mcp.registerTool("get_camera_position", "Gets the current map view camera center position.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| return "Camera positioning query not directly supported via bounds yet." | ||
| end) | ||
|
|
||
| mcp.registerTool("set_camera_position", "Sets the map view camera to center on a position.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z" } | ||
| }, function(args) | ||
| app.setCameraPosition(args.x, args.y, args.z) | ||
| return "Camera moved." | ||
| end) | ||
|
|
||
| mcp.registerTool("create_house", "Assigns a selection of tiles to a new house ID.", { | ||
| type = "object", | ||
| properties = { houseId = { type = "number" } }, | ||
| required = { "houseId" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
| local map = app.map | ||
| app.transaction("Create House", function() | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| for z = minP.z, maxP.z do | ||
| local t = map:getOrCreateTile(x, y, z) | ||
| if t then | ||
| t.houseId = args.houseId | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end) | ||
| return "Tiles assigned to house " .. tostring(args.houseId) | ||
| end) | ||
|
|
||
| mcp.registerTool("get_house", "Returns the house ID of a given tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| local t = getTile(args.x, args.y, args.z) | ||
| if not t then return "Tile not found." end | ||
| return t.houseId > 0 and tostring(t.houseId) or "No house at this tile." | ||
| end) | ||
|
|
||
| mcp.registerTool("clear_selection", "Clears the current map selection.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| app.selection:clear() | ||
| return "Selection cleared." | ||
| end) | ||
|
|
||
| mcp.registerTool("save_map", "Saves the current map.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| if not app.hasMap() then return "No map loaded." end | ||
| return "Error: Programmatic save not directly exposed. Please ask the user to press CTRL+S." | ||
| end) | ||
|
|
||
| mcp.registerTool("get_spawn", "Gets spawn details at a given coordinate.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| local t = getTile(args.x, args.y, args.z) | ||
| if not t then return "Tile not found." end | ||
| if not t.hasSpawn then return "No spawn found." end | ||
| return "Spawn found with radius " .. tostring(t.spawn.radius) | ||
| end) | ||
|
|
||
| mcp.registerTool("add_spawn", "Creates a spawn point at a tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" }, | ||
| radius = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z", "radius" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.transaction("Add Spawn", function() | ||
| local t = getOrCreateTile(args.x, args.y, args.z) | ||
| if t then | ||
| t:setSpawn(args.radius) | ||
| end | ||
| end) | ||
| return "Spawn point created." | ||
| end) | ||
|
|
||
| mcp.registerTool("add_creature", "Adds a creature to a tile.", { | ||
| type = "object", | ||
| properties = { | ||
| x = { type = "number" }, y = { type = "number" }, z = { type = "number" }, | ||
| name = { type = "string" }, spawnTime = { type = "number" } | ||
| }, | ||
| required = { "x", "y", "z", "name", "spawnTime" } | ||
| }, function(args) | ||
| if not app.hasMap() then return "No map loaded." end | ||
| app.transaction("Add Creature", function() | ||
| local t = getOrCreateTile(args.x, args.y, args.z) | ||
| if t then | ||
| t:setCreature(args.name, args.spawnTime) | ||
| end | ||
| end) | ||
| return "Creature added." | ||
| end) | ||
|
|
||
| mcp.registerTool("get_item_info", "Returns basic properties of an item by its ID.", { | ||
| type = "object", | ||
| properties = { itemId = { type = "number" } }, | ||
| required = { "itemId" } | ||
| }, function(args) | ||
| local info = Items.getInfo(args.itemId) | ||
| if not info then return "Item not found." end | ||
| return json.encode(info) | ||
| end) | ||
|
|
||
| mcp.registerTool("set_light", "Sets the global ambient light.", { | ||
| type = "object", | ||
| properties = { level = { type = "number" }, }, | ||
| required = { "level" } | ||
| }, function(args) | ||
| app.setAmbientLightLevel(args.level) | ||
| return "Ambient light set." | ||
| end) | ||
|
|
||
| mcp.registerTool("refresh_view", "Refreshes the editor rendering.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| app.refresh() | ||
| return "View refreshed." | ||
| end) | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Lookup / Discovery Tools | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| mcp.registerTool("search_items", "Searches for items by name. Returns matching item IDs, names, and properties. Use this to find proper ground tiles, walls, decorations, etc.", { | ||
| type = "object", | ||
| properties = { | ||
| query = { type = "string", description = "Search term (case-insensitive, partial match)" }, | ||
| maxResults = { type = "number", description = "Maximum results to return (default 25)" } | ||
| }, | ||
| required = { "query" } | ||
| }, function(args) | ||
| local results = Items.findByName(args.query, args.maxResults or 25) | ||
| if #results == 0 then return "No items found matching '" .. args.query .. "'." end | ||
|
|
||
| local detailed = {} | ||
| for _, r in ipairs(results) do | ||
| local info = Items.getInfo(r.id) | ||
| if info then | ||
| table.insert(detailed, { | ||
| id = info.id, | ||
| name = info.name, | ||
| isGround = info.isGroundTile, | ||
| isWall = info.isWall, | ||
| isBorder = info.isBorder, | ||
| isDoor = info.isDoor, | ||
| isTable = info.isTable, | ||
| isCarpet = info.isCarpet, | ||
| isBlocking = not info.isMoveable and not info.isPickupable | ||
| }) | ||
| end | ||
| end | ||
| return json.encode(detailed) | ||
| end) | ||
|
|
||
| mcp.registerTool("list_brushes", "Lists available brushes, optionally filtered by type. Brush types: ground, wall, doodad, door, table, carpet, creature, spawn, raw, house, waypoint, eraser, terrain.", { | ||
| type = "object", | ||
| properties = { | ||
| typeFilter = { type = "string", description = "Filter by brush type (e.g. 'ground', 'wall', 'doodad'). Leave empty for all." }, | ||
| nameFilter = { type = "string", description = "Filter by name substring (case-insensitive). Leave empty for all." }, | ||
| maxResults = { type = "number", description = "Maximum results to return (default 50)" } | ||
| } | ||
| }, function(args) | ||
| local allNames = Brushes.getNames() | ||
| local maxResults = args.maxResults or 50 | ||
| local typeFilter = args.typeFilter and args.typeFilter:lower() or nil | ||
| local nameFilter = args.nameFilter and args.nameFilter:lower() or nil | ||
| local results = {} | ||
|
|
||
| for _, name in ipairs(allNames) do | ||
| if #results >= maxResults then break end | ||
|
|
||
| local brush = Brushes.get(name) | ||
| if brush then | ||
| local bType = brush.type | ||
| local bName = brush.name | ||
|
|
||
| local matchType = not typeFilter or bType == typeFilter | ||
| local matchName = not nameFilter or bName:lower():find(nameFilter, 1, true) | ||
|
|
||
| if matchType and matchName then | ||
| table.insert(results, { | ||
| name = bName, | ||
| type = bType, | ||
| lookId = brush.lookId | ||
| }) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| if #results == 0 then return "No brushes found." end | ||
| return json.encode(results) | ||
| end) | ||
|
|
||
| mcp.registerTool("get_brush_info", "Gets detailed information about a specific brush by name.", { | ||
| type = "object", | ||
| properties = { | ||
| name = { type = "string", description = "Exact brush name" } | ||
| }, | ||
| required = { "name" } | ||
| }, function(args) | ||
| local brush = Brushes.get(args.name) | ||
| if not brush then return "Brush '" .. args.name .. "' not found." end | ||
| return json.encode({ | ||
| name = brush.name, | ||
| type = brush.type, | ||
| id = brush.id, | ||
| lookId = brush.lookId, | ||
| needBorders = brush:needBorders(), | ||
| canDrag = brush:canDrag(), | ||
| canSmear = brush:canSmear() | ||
| }) | ||
| end) | ||
|
|
||
| mcp.registerTool("list_creatures", "Lists available creature names, optionally filtered. Use this to find creatures to place on the map.", { | ||
| type = "object", | ||
| properties = { | ||
| nameFilter = { type = "string", description = "Filter by name substring (case-insensitive). Leave empty for all." }, | ||
| maxResults = { type = "number", description = "Maximum results to return (default 50)" }, | ||
| npcOnly = { type = "boolean", description = "If true, only return NPCs. If false, only return monsters. Omit for both." } | ||
| } | ||
| }, function(args) | ||
| -- Use Brushes to find creature brushes (they correspond to creatures) | ||
| local allNames = Brushes.getNames() | ||
| local maxResults = args.maxResults or 50 | ||
| local nameFilter = args.nameFilter and args.nameFilter:lower() or nil | ||
| local results = {} | ||
|
|
||
| for _, name in ipairs(allNames) do | ||
| if #results >= maxResults then break end | ||
|
|
||
| local brush = Brushes.get(name) | ||
| if brush and brush.type == "creature" then | ||
| local bName = brush.name | ||
| local matchName = not nameFilter or bName:lower():find(nameFilter, 1, true) | ||
|
|
||
| if matchName then | ||
| local isNpc = isNpcType(bName) | ||
| local matchNpc = args.npcOnly == nil or args.npcOnly == isNpc | ||
|
|
||
| if matchNpc then | ||
| table.insert(results, { | ||
| name = bName, | ||
| isNpc = isNpc, | ||
| lookId = brush.lookId | ||
| }) | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| if #results == 0 then return "No creatures found." end | ||
| return json.encode(results) | ||
| end) | ||
|
|
||
| mcp.registerTool("find_item_id", "Finds the first item ID matching an exact name (case-insensitive). Falls back to partial match if no exact match.", { | ||
| type = "object", | ||
| properties = { | ||
| name = { type = "string", description = "Item name to search for" } | ||
| }, | ||
| required = { "name" } | ||
| }, function(args) | ||
| local id = Items.findIdByName(args.name) | ||
| if not id then return "No item found matching '" .. args.name .. "'." end | ||
| local info = Items.getInfo(id) | ||
| if info then | ||
| return json.encode(info) | ||
| end | ||
| return "Item ID " .. tostring(id) .. " found but no info available." | ||
| end) | ||
|
|
||
| mcp.registerTool("get_brush_count", "Returns the total number of brushes available.", { | ||
| type = "object", properties = {} | ||
| }, function() | ||
| return "Total brushes: " .. tostring(Brushes.count()) | ||
| end) | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Fill Empty Area (Smart Fill / WFC-lite) | ||
| -- | ||
| -- Analyzes existing tiles in the selection, then fills empty tiles by | ||
| -- copying ground types and decorations from nearby neighbors. | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| mcp.registerTool("fill_empty_area", "Fills empty tiles in the selection by analyzing surrounding tiles and reproducing the same ground types and decorations. Works like a smart-fill / wave function collapse.", { | ||
| type = "object", | ||
| properties = { | ||
| scatterDoodads = { type = "boolean", description = "If true (default), scatter decorative items found in the area onto filled tiles." }, | ||
| doodadDensity = { type = "number", description = "Probability (0.0-1.0) of placing a doodad on each filled tile. Default 0.15." } | ||
| } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
|
|
||
| local map = app.map | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| local scatterDoodads = args.scatterDoodads ~= false | ||
| local doodadDensity = args.doodadDensity or 0.15 | ||
|
|
||
| -- Phase 1: Scan existing tiles | ||
| local groundFreq = {} -- groundId -> count | ||
| local totalGroundTiles = 0 | ||
| local doodadItems = {} -- list of item IDs found as decorations | ||
| local doodadSet = {} -- dedup set | ||
| local emptyPositions = {} -- {x, y, z} of empty tiles | ||
|
|
||
| -- Build a lookup grid for quick neighbor checks | ||
| local groundGrid = {} -- [x][y] = groundId or nil | ||
|
|
||
| for x = minP.x, maxP.x do | ||
| groundGrid[x] = {} | ||
| for y = minP.y, maxP.y do | ||
| local t = map:getTile(x, y, minP.z) | ||
| if t and t.ground then | ||
| local gid = t.ground.id | ||
| groundGrid[x][y] = gid | ||
| groundFreq[gid] = (groundFreq[gid] or 0) + 1 | ||
| totalGroundTiles = totalGroundTiles + 1 | ||
|
|
||
| -- Collect decorative items (non-ground, non-wall, non-border) | ||
| if scatterDoodads then | ||
| for _, item in ipairs(t.items) do | ||
| local iid = item.id | ||
| if not doodadSet[iid] then | ||
| -- Only pick up items that aren't walls/borders | ||
| if not item.isWall and not item.isBorder then | ||
| table.insert(doodadItems, iid) | ||
| doodadSet[iid] = true | ||
| end | ||
| end | ||
| end | ||
| end | ||
| else | ||
| table.insert(emptyPositions, {x = x, y = y, z = minP.z}) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| if totalGroundTiles == 0 then | ||
| return "No existing tiles found to learn from." | ||
| end | ||
| if #emptyPositions == 0 then | ||
| return "No empty tiles to fill." | ||
| end | ||
|
|
||
| -- Build sorted frequency list for fallback | ||
| local groundList = {} | ||
| for gid, count in pairs(groundFreq) do | ||
| table.insert(groundList, {id = gid, weight = count}) | ||
| end | ||
| table.sort(groundList, function(a, b) return a.weight > b.weight end) | ||
|
|
||
| -- Phase 2: Fill empty tiles using neighbor context | ||
| -- We do multiple passes so newly filled tiles help inform later ones | ||
| local filled = 0 | ||
| local seed = os.time() | ||
|
|
||
| -- Simple PRNG (LCG) for speed | ||
| local function rng() | ||
| seed = (seed * 1103515245 + 12345) % 2147483648 | ||
| return seed / 2147483648 | ||
| end | ||
|
|
||
| -- Weighted random pick from a frequency table | ||
| local function weightedPick(freq) | ||
| local total = 0 | ||
| for _, w in pairs(freq) do total = total + w end | ||
| if total == 0 then return groundList[1].id end | ||
|
|
||
| local roll = rng() * total | ||
| local acc = 0 | ||
| for gid, w in pairs(freq) do | ||
| acc = acc + w | ||
| if roll <= acc then return gid end | ||
| end | ||
| -- fallback | ||
| return groundList[1].id | ||
| end | ||
|
|
||
| app.transaction("Fill Empty Area", function() | ||
| local maxPasses = 5 | ||
| for pass = 1, maxPasses do | ||
| local filledThisPass = 0 | ||
|
|
||
| for _, pos in ipairs(emptyPositions) do | ||
| if not groundGrid[pos.x][pos.y] then | ||
| -- Check 8 neighbors | ||
| local neighborFreq = {} | ||
| local hasNeighbor = false | ||
| for dx = -1, 1 do | ||
| for dy = -1, 1 do | ||
| if not (dx == 0 and dy == 0) then | ||
| local nx = pos.x + dx | ||
| local ny = pos.y + dy | ||
| if groundGrid[nx] and groundGrid[nx][ny] then | ||
| local nGid = groundGrid[nx][ny] | ||
| -- Cardinal neighbors get double weight | ||
| local weight = (dx == 0 or dy == 0) and 2 or 1 | ||
| neighborFreq[nGid] = (neighborFreq[nGid] or 0) + weight | ||
| hasNeighbor = true | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| if hasNeighbor then | ||
| -- Pick ground based on neighbor frequency | ||
| local chosenGround = weightedPick(neighborFreq) | ||
|
|
||
| local t = map:getOrCreateTile(pos.x, pos.y, pos.z) | ||
| if t then | ||
| t.ground = chosenGround | ||
| groundGrid[pos.x][pos.y] = chosenGround | ||
|
|
||
| -- Maybe scatter a doodad | ||
| if scatterDoodads and #doodadItems > 0 and rng() < doodadDensity then | ||
| local doodadId = doodadItems[math.floor(rng() * #doodadItems) + 1] | ||
| t:addItem(doodadId) | ||
| end | ||
|
|
||
| filled = filled + 1 | ||
| filledThisPass = filledThisPass + 1 | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| -- If nothing was filled this pass, remaining tiles have no neighbors yet | ||
| -- Use global frequency as fallback | ||
| if filledThisPass == 0 then | ||
| for _, pos in ipairs(emptyPositions) do | ||
| if not groundGrid[pos.x][pos.y] then | ||
| local chosenGround = weightedPick(groundFreq) | ||
| local t = map:getOrCreateTile(pos.x, pos.y, pos.z) | ||
| if t then | ||
| t.ground = chosenGround | ||
| groundGrid[pos.x][pos.y] = chosenGround | ||
|
|
||
| if scatterDoodads and #doodadItems > 0 and rng() < doodadDensity then | ||
| local doodadId = doodadItems[math.floor(rng() * #doodadItems) + 1] | ||
| t:addItem(doodadId) | ||
| end | ||
|
|
||
| filled = filled + 1 | ||
| end | ||
| end | ||
| end | ||
| break | ||
| end | ||
| end | ||
| end) | ||
|
|
||
| return string.format("Filled %d empty tiles. Learned from %d existing tiles with %d ground types, %d doodad types.", | ||
| filled, totalGroundTiles, #groundList, #doodadItems) | ||
| end) | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Raw Tile Dump | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| mcp.registerTool("get_selection_tiles", "Dumps all tiles in the current selection, including their ground and items. Useful for learning structural patterns.", { | ||
| type = "object", | ||
| properties = { | ||
| maxTiles = { type = "number", description = "Max number of tiles to return (default 1000)" }, | ||
| onlyNonEmpty = { type = "boolean", description = "If true, skips tiles without ground or items" } | ||
| } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
|
|
||
| local map = app.map | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
| local maxTiles = args.maxTiles or 1000 | ||
| local onlyNonEmpty = args.onlyNonEmpty | ||
|
|
||
| local tiles = {} | ||
| local count = 0 | ||
|
|
||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| if count >= maxTiles then break end | ||
|
|
||
| local t = map:getTile(x, y, minP.z) | ||
| if t then | ||
| local hasContent = t.ground or #t.items > 0 | ||
| if not onlyNonEmpty or hasContent then | ||
| local tileInfo = { | ||
| x = x, y = y, z = minP.z, | ||
| items = {} | ||
| } | ||
| if t.ground then | ||
| tileInfo.ground = { id = t.ground.id, name = t.ground.name } | ||
| end | ||
| for _, item in ipairs(t.items) do | ||
| table.insert(tileInfo.items, { | ||
| id = item.id, | ||
| name = item.name, | ||
| isWall = item.isWall, | ||
| isDoor = item.isDoor, | ||
| isBorder = item.isBorder, | ||
| isBlocking = item.isBlocking, | ||
| isStackable = item.isStackable | ||
| }) | ||
| end | ||
| table.insert(tiles, tileInfo) | ||
| count = count + 1 | ||
| end | ||
| elseif not onlyNonEmpty then | ||
| -- Return empty tile | ||
| table.insert(tiles, { x = x, y = y, z = minP.z, ground = nil, items = {} }) | ||
| count = count + 1 | ||
| end | ||
| end | ||
| if count >= maxTiles then break end | ||
| end | ||
|
|
||
| return json.encode({ | ||
| tiles = tiles, | ||
| totalReturned = count, | ||
| selectionSize = { | ||
| width = maxP.x - minP.x + 1, | ||
| height = maxP.y - minP.y + 1 | ||
| } | ||
| }) | ||
| end) | ||
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Architectural Template System | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| local activeTemplates = {} | ||
|
|
||
| local function getTopWall(t) | ||
| if not t then return nil end | ||
| for i = t.itemCount, 1, -1 do | ||
| local item = t:getItemAt(i) | ||
| if item and item.isWall then | ||
| return item.id | ||
| end | ||
| end | ||
| return nil | ||
| end | ||
|
|
||
| local function getHighestFreq(freqTable) | ||
| local bestId = nil | ||
| local maxCount = -1 | ||
| for id, count in pairs(freqTable) do | ||
| if count > maxCount then | ||
| maxCount = count | ||
| bestId = id | ||
| end | ||
| end | ||
| return bestId | ||
| end | ||
|
|
||
| mcp.registerTool("capture_building_template", "Analyzes the current selected building and saves its structural footprint (walls, floors, corners) under a given template name.", { | ||
| type = "object", | ||
| properties = { | ||
| templateName = { type = "string", description = "Name to save the template under." } | ||
| }, | ||
| required = { "templateName" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
|
|
||
| local map = app.map | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
|
|
||
| if maxP.x - minP.x < 2 or maxP.y - minP.y < 2 then | ||
| return "Selection is too small to capture a template. Need at least 3x3 to identify corners and interior." | ||
| end | ||
|
|
||
| local floorFreq = {} | ||
| local nWallFreq = {} | ||
| local sWallFreq = {} | ||
| local wWallFreq = {} | ||
| local eWallFreq = {} | ||
|
|
||
| local template = { | ||
| corners = { | ||
| nw = nil, ne = nil, sw = nil, se = nil | ||
| }, | ||
| door = nil | ||
| } | ||
|
|
||
| -- Extract Corners | ||
| template.corners.nw = getTopWall(map:getTile(minP.x, minP.y, minP.z)) | ||
| template.corners.ne = getTopWall(map:getTile(maxP.x, minP.y, minP.z)) | ||
| template.corners.sw = getTopWall(map:getTile(minP.x, maxP.y, minP.z)) | ||
| template.corners.se = getTopWall(map:getTile(maxP.x, maxP.y, minP.z)) | ||
|
|
||
| -- Extract Edges and Interior | ||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| local t = map:getTile(x, y, minP.z) | ||
| if t then | ||
| -- Interior Floors | ||
| if x > minP.x and x < maxP.x and y > minP.y and y < maxP.y then | ||
| if t.ground then | ||
| floorFreq[t.ground.id] = (floorFreq[t.ground.id] or 0) + 1 | ||
| end | ||
| end | ||
|
|
||
| -- Check for doors | ||
| for i = 1, t.itemCount do | ||
| local item = t:getItemAt(i) | ||
| if item and item.isDoor then | ||
| template.door = item.id | ||
| end | ||
| end | ||
|
|
||
| -- Edges (exclude corners) | ||
| local wallId = getTopWall(t) | ||
| if wallId then | ||
| if y == minP.y and x > minP.x and x < maxP.x then | ||
| nWallFreq[wallId] = (nWallFreq[wallId] or 0) + 1 | ||
| elseif y == maxP.y and x > minP.x and x < maxP.x then | ||
| sWallFreq[wallId] = (sWallFreq[wallId] or 0) + 1 | ||
| elseif x == minP.x and y > minP.y and y < maxP.y then | ||
| wWallFreq[wallId] = (wWallFreq[wallId] or 0) + 1 | ||
| elseif x == maxP.x and y > minP.y and y < maxP.y then | ||
| eWallFreq[wallId] = (eWallFreq[wallId] or 0) + 1 | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| template.floor = getHighestFreq(floorFreq) | ||
| template.walls = { | ||
| n = getHighestFreq(nWallFreq), | ||
| s = getHighestFreq(sWallFreq), | ||
| e = getHighestFreq(eWallFreq), | ||
| w = getHighestFreq(wWallFreq), | ||
| } | ||
|
|
||
| -- basic validation | ||
| if not template.floor and not template.walls.n then | ||
| return "Could not identify floor or walls. Make sure selection tightly wraps the building." | ||
| end | ||
|
|
||
| activeTemplates[args.templateName] = template | ||
|
|
||
| return json.encode({ | ||
| status = "Template captured successfully", | ||
| templateName = args.templateName, | ||
| signature = template | ||
| }) | ||
| end) | ||
|
|
||
| mcp.registerTool("build_house_template", "Uses a previously captured template to draw a building filling the current selection.", { | ||
| type = "object", | ||
| properties = { | ||
| templateName = { type = "string", description = "Name of the captured template to build." } | ||
| }, | ||
| required = { "templateName" } | ||
| }, function(args) | ||
| if not app.hasMap() or app.selection.isEmpty then return "No map or selection." end | ||
|
|
||
| local template = activeTemplates[args.templateName] | ||
| if not template then return "Template '" .. args.templateName .. "' not found. Capture it first." end | ||
|
|
||
| local map = app.map | ||
| local minP = app.selection.minPosition | ||
| local maxP = app.selection.maxPosition | ||
|
|
||
| if maxP.x - minP.x < 2 or maxP.y - minP.y < 2 then | ||
| return "Selection is too small to build a house (needs at least 3x3)." | ||
| end | ||
|
|
||
| local houseZ = minP.z | ||
| local tilesPlaced = 0 | ||
|
|
||
| app.transaction("Build Template House", function() | ||
| -- 1. Fill Floor | ||
| if template.floor then | ||
| for x = minP.x, maxP.x do | ||
| for y = minP.y, maxP.y do | ||
| local t = map:getOrCreateTile(x, y, houseZ) | ||
| if t then | ||
| t.ground = template.floor | ||
| tilesPlaced = tilesPlaced + 1 | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| -- 2. Edges | ||
| local function placeWall(x, y, id) | ||
| if not id then return end | ||
| local t = map:getOrCreateTile(x, y, houseZ) | ||
| if t then | ||
| for i = t.itemCount, 1, -1 do | ||
| local it = t:getItemAt(i) | ||
| if it and it.isWall then | ||
| t:removeItem(it) | ||
| end | ||
| end | ||
| t:addItem(id) | ||
| tilesPlaced = tilesPlaced + 1 | ||
| end | ||
| end | ||
|
|
||
| for x = minP.x + 1, maxP.x - 1 do | ||
| placeWall(x, minP.y, template.walls.n) | ||
| placeWall(x, maxP.y, template.walls.s) | ||
| end | ||
|
|
||
| for y = minP.y + 1, maxP.y - 1 do | ||
| placeWall(minP.x, y, template.walls.w) | ||
| placeWall(maxP.x, y, template.walls.e) | ||
| end | ||
|
|
||
| -- 3. Corners | ||
| placeWall(minP.x, minP.y, template.corners.nw) | ||
| placeWall(maxP.x, minP.y, template.corners.ne) | ||
| placeWall(minP.x, maxP.y, template.corners.sw) | ||
| placeWall(maxP.x, maxP.y, template.corners.se) | ||
|
|
||
| -- 4. Door | ||
| if template.door then | ||
| -- Find a spot on south wall middle | ||
| local doorX = math.floor((minP.x + maxP.x) / 2) | ||
| local doorY = maxP.y | ||
| local t = map:getOrCreateTile(doorX, doorY, houseZ) | ||
| if t then | ||
| for i = t.itemCount, 1, -1 do | ||
| local it = t:getItemAt(i) | ||
| if it and it.isWall then t:removeItem(it) end | ||
| end | ||
| t:addItem(template.door) | ||
| end | ||
| end | ||
| end) | ||
|
|
||
| return "Building generated successfully using template '" .. args.templateName .. "'. Placed " .. tostring(tilesPlaced) .. " components." | ||
| end) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication with scripts/mcp/mcp_server.lua.
This file appears to be an exact duplicate of scripts/mcp/mcp_server.lua. Having two copies creates maintenance burden and bug synchronization issues (e.g., the same isNpcType undefined error exists in both).
Consider:
- Keeping only one canonical location
- Or having one file
requirethe other if both locations are needed for different deployment scenarios
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/scripts/mcp/mcp_server.lua` around lines 1 - 1267, This file is a
near-exact duplicate of the other MCP server (causing duplicated logic and
duplicated bugs like the undefined isNpcType), so pick a single canonical
implementation (keep one copy with mcp.registerTool and mcp.handleRequest as the
source of truth) and remove the duplicate file OR refactor this file to simply
require and return the canonical module; ensure the canonical module returns the
mcp table (exposing mcp.registerTool, mcp.handleRequest, etc.), remove duplicate
server-start logic from the duplicate, and update any callers/imports to require
the single module so tools and functions (e.g., getTile/getOrCreateTile,
fill_empty_area, capture_building_template, and isNpcType usage) are only
defined once.
Comment on lines
+25
to
+29
| let reqId = null; | ||
| try { | ||
| const parsed = JSON.parse(line); | ||
| reqId = parsed.id !== undefined ? parsed.id : null; | ||
| } catch (e) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JSON-RPC 2.0, should a server emit any response (including timeout/transport errors) for a valid notification request that has no id?
💡 Result:
No, in JSON-RPC 2.0, a server should not emit any response for a valid notification request that has no "id" member. The official specification states: "A Notification is a Request object without an 'id' member. [...] The Server MUST NOT reply to a Notification, including those that are within a batch request." This applies even if there are errors in processing the notification, as confirmed by secondary sources like JSON-RPC Tools: "The server must not reply to a notification, including when the notification is invalid." The server also should not send transport-level errors or timeouts as responses; for HTTP, it may return a 200 OK with empty body or 204 No Content, but no JSON-RPC response object.
Citations:
- 1: https://www.jsonrpc.org/specification
- 2: https://json-rpc.dev/learn/examples/notifications
- 3: https://www.jsonrpc.org/historical/json-rpc-2-0.html
- 4: https://en.wikipedia.org/wiki/JSON-RPC
- 5: Omitting
idin JSON rpc request getting empty response paritytech/substrate#4862
🏁 Script executed:
# First, locate the mcp_bridge.js file
find . -name "mcp_bridge.js" -type fRepository: karolak6612/remeres-map-editor-redux
Length of output: 95
🏁 Script executed:
# Read the file to verify the code structure
cat -n ./mcp_bridge.js | head -100Repository: karolak6612/remeres-map-editor-redux
Length of output: 3105
Emit no response for JSON-RPC 2.0 notifications on error or timeout.
JSON-RPC 2.0 specification requires the server to not emit any response for notifications (requests without an id field), including error/timeout responses. The current code sends error responses unconditionally at lines 71–75 and 83–87, and parse errors at lines 30–34, which violates this requirement for valid notifications that lack an id member.
Distinguish notifications by tracking when parsed.id === undefined, then conditionally suppress error responses only for notifications:
Proposed fix
let reqId = null;
+ let isNotification = false;
try {
const parsed = JSON.parse(line);
+ isNotification = parsed.id === undefined;
- reqId = parsed.id !== undefined ? parsed.id : null;
+ reqId = isNotification ? null : parsed.id;
} catch (e) {
console.log(JSON.stringify({
jsonrpc: "2.0",
error: { code: -32700, message: "Parse error" },
id: null
}));
return;
} req.on('timeout', () => {
if (!responded) {
responded = true;
+ if (!isNotification) {
console.log(JSON.stringify({
jsonrpc: "2.0",
error: { code: -32001, message: "Request timeout" },
id: reqId
}));
+ }
req.destroy();
}
}); req.on('error', (e) => {
if (!responded) {
responded = true;
+ if (!isNotification) {
console.log(JSON.stringify({
jsonrpc: "2.0",
error: { code: -32002, message: "Connection error: " + e.message },
id: reqId
}));
+ }
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp_bridge.js` around lines 25 - 29, When parsing incoming lines, distinguish
JSON-RPC notifications by checking parsed.id === undefined (instead of mapping
missing id to null) and record an isNotification boolean; set reqId only when
parsed.id !== undefined (or set reqId = parsed.id when present, otherwise leave
null) so you can tell notifications apart from requests. Then, before sending
any error or timeout responses in the existing send/error paths (the blocks that
currently always emit errors after parsing and on timeouts), conditionally
suppress those responses when isNotification is true; keep emitting Parse error
responses from the JSON.parse catch (since you cannot determine notification
state on malformed JSON). Update the code paths that reference reqId/parsed (the
JSON.parse block that sets reqId and the error/timeout response emitters) to use
the new isNotification check to avoid replying to notifications.
Comment on lines
+58
to
+63
| res.on('end', () => { | ||
| if (!responded) { | ||
| responded = true; | ||
| if (responseData.trim()) { | ||
| console.log(responseData.trim()); | ||
| } |
There was a problem hiding this comment.
Do not forward raw HTTP bodies directly to MCP stdout.
If the backend returns non-JSON (or non-JSON-RPC), this writes invalid protocol output to the client stream.
Proposed hardening for response validation
res.on('end', () => {
if (!responded) {
responded = true;
if (responseData.trim()) {
- console.log(responseData.trim());
+ try {
+ JSON.parse(responseData);
+ console.log(responseData.trim());
+ } catch (_) {
+ console.log(JSON.stringify({
+ jsonrpc: "2.0",
+ error: { code: -32003, message: "Invalid backend response" },
+ id: reqId
+ }));
+ }
}
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp_bridge.js` around lines 58 - 63, The current res.on('end' ...) handler
forwards raw responseData to stdout (via console.log(responseData.trim())) which
can emit invalid MCP protocol when backend returns non-JSON; update the
res.on('end'...) logic (the responded flag, responseData handling) to validate
and parse responseData as JSON (and optionally confirm JSON-RPC shape if
expected) before forwarding: attempt JSON.parse(responseData.trim()), if parsing
succeeds (and passes any minimal JSON-RPC checks) then console.log the
serialized JSON output, otherwise do not write the raw body to stdout—log a
clear error or write a structured MCP error response instead and ensure
responded is set to true to avoid duplicate responses.
| "mcpServers": { | ||
| "rme-editor": { | ||
| "command": "node", | ||
| "args": ["<PATH_TO_RME_REPO>/mcp_bridge.js"], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For MCP client configuration (e.g., Claude Desktop), how are argspaths resolved for a server command: absolute-only, relative to working directory, or relative to config file? Is acwd field supported?
💡 Result:
For MCP client configuration in Claude Desktop (claude_desktop_config.json), paths in the args array for server commands must be absolute paths. Relative paths do not work reliably and are not recommended; all examples and troubleshooting guides explicitly require absolute paths (e.g., "/Users/username/Desktop"). Paths are resolved relative to the process's working directory when the subprocess is spawned (likely Claude Desktop's installation directory or app bundle), but relative paths fail because they don't resolve correctly from there. A cwd field is supported in the configuration as an optional property alongside command, args, and env. It specifies the working directory for the server subprocess, allowing relative args paths to be resolved from that directory. Examples include: "custom-server": { "command": "python3", "args": ["server.py"], "cwd": "/path/to/your" } This is documented in configuration guides for Claude Desktop, though not always listed in minimal examples focused on npx servers with absolute paths. Note: VS Code's MCP config (mcp.json) does not explicitly list cwd but supports variables like ${workspaceFolder}; however, the query focuses on Claude Desktop.
Citations:
- 1: https://modelcontextprotocol.io/docs/develop/connect-local-servers
- 2: https://explainmcp.com/mcp-servers/claude-desktop-config-json-guide/
- 3: https://blog.egmond.dev/mcp-server-claude
- 4: https://mcpplaygroundonline.com/blog/how-to-setup-mcp-claude-desktop
- 5: https://www.mcpbundles.com/blog/claude-desktop-mcp
- 6: https://aiagentskit.com/blog/claude-desktop-mcp-setup/
- 7: https://mcpcn.com/en/docs/quickstart/user
- 8: https://linuxbeast.com/blog/how-to-configure-mcp-servers-in-vs-code-claude-desktop-and-claude-code-on-wsl-2/
Replace placeholder path with absolute path or use cwd field.
The args path "<PATH_TO_RME_REPO>/mcp_bridge.js" is not absolute and will fail at runtime; MCP configuration requires absolute paths for args. Either replace the placeholder with an absolute path to the bridge file, or add a cwd field to the configuration to specify the working directory for relative path resolution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp.json` at line 5, The args entry currently uses a non-absolute placeholder
"<PATH_TO_RME_REPO>/mcp_bridge.js" which will fail at runtime; fix by either
replacing that placeholder in the "args" array with the absolute path to
mcp_bridge.js or add a "cwd" field to the same mcp.json object so the relative
path resolves (i.e., update the "args" array item or add "cwd" so the process
can locate mcp_bridge.js).
Comment on lines
+768
to
+769
| local isNpc = isNpcType(bName) | ||
| local matchNpc = args.npcOnly == nil or args.npcOnly == isNpc |
There was a problem hiding this comment.
Undefined function isNpcType will cause runtime error.
The list_creatures tool calls isNpcType(bName) on line 768, but this function is never defined in the script. This will throw a Lua error when the tool is invoked with an npcOnly filter.
Define the missing function or remove the feature
Option 1 - Define a placeholder (adjust logic as needed):
-- Add near the top of the file after helpers
local function isNpcType(name)
-- Simple heuristic: check if name contains common NPC indicators
local lowerName = name:lower()
return lowerName:find("npc") or lowerName:find("citizen") or lowerName:find("merchant")
endOption 2 - Remove the NPC filter feature until properly implemented:
mcp.registerTool("list_creatures", "Lists available creature names, optionally filtered. Use this to find creatures to place on the map.", {
type = "object",
properties = {
nameFilter = { type = "string", description = "Filter by name substring (case-insensitive). Leave empty for all." },
- maxResults = { type = "number", description = "Maximum results to return (default 50)" },
- npcOnly = { type = "boolean", description = "If true, only return NPCs. If false, only return monsters. Omit for both." }
+ maxResults = { type = "number", description = "Maximum results to return (default 50)" }
}
}, function(args)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/mcp/mcp_server.lua` around lines 768 - 769, The code calls an
undefined function isNpcType in the list_creatures tool (when computing
matchNpc), which will error at runtime; add a local function named
isNpcType(name) near the helpers/top of the file that implements the desired NPC
detection (for now implement a simple heuristic such as downcasing the name and
checking for substrings like "npc", "citizen", "merchant" or other
project-specific markers) or, if the NPC filter is not ready, remove the npcOnly
filter usage in list_creatures (the matchNpc line) so the feature is disabled
until a proper implementation exists; ensure the symbol isNpcType is declared
local and referenced by list_creatures.
Comment on lines
+75
to
+108
| void stop() { | ||
| if (!running.exchange(false)) return; | ||
|
|
||
| beast::error_code ec; | ||
| acceptor.cancel(ec); | ||
| acceptor.close(ec); | ||
|
|
||
| // Clean up queued requests | ||
| { | ||
| std::lock_guard<std::mutex> lock(queue_mutex); | ||
| while (!request_queue.empty()) { | ||
| auto req = request_queue.front(); | ||
| request_queue.pop(); | ||
| if (req->on_complete) { | ||
| nlohmann::json err = { | ||
| {"jsonrpc", "2.0"}, | ||
| {"id", req->id}, | ||
| {"error", { | ||
| {"code", -32000}, | ||
| {"message", "Server shutting down"} | ||
| }} | ||
| }; | ||
| std::string err_str = err.dump(); | ||
| net::post(ioc, [req, err_str]() { | ||
| req->on_complete(err_str); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ioc.stop(); | ||
| ioc.restart(); | ||
| spdlog::info("MCP Server stopped."); | ||
| } |
There was a problem hiding this comment.
Race condition: stop() returns before background thread joins.
The stop() method calls ioc.stop() but doesn't wait for server_thread to finish before returning. In Application::OnExit(), setHandler(nullptr) is called immediately after stop(), creating a window where:
- The background thread may still be processing a Session
- Sessions can enqueue requests after
stop()drains the queue processPendingRequests()might run with a stale or null handler
Consider explicitly joining the thread in stop():
Proposed fix
void stop() {
if (!running.exchange(false)) return;
beast::error_code ec;
acceptor.cancel(ec);
acceptor.close(ec);
// Clean up queued requests
{
std::lock_guard<std::mutex> lock(queue_mutex);
while (!request_queue.empty()) {
// ... existing drain logic ...
}
}
ioc.stop();
+
+ // Wait for the I/O thread to finish before returning
+ if (server_thread.joinable()) {
+ server_thread.join();
+ }
+
ioc.restart();
spdlog::info("MCP Server stopped.");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/mcp/mcp_server.cpp` around lines 75 - 108, The stop() method returns
before the background server_thread has finished, allowing Sessions to run and
enqueue requests after the queue was drained; modify stop() to call ioc.stop()
and then if (server_thread.joinable()) server_thread.join() so the method blocks
until the background thread exits, ensuring processPendingRequests() and handler
teardown (e.g., Application::setHandler(nullptr)) cannot run against a
still-active thread—use the existing server_thread identifier and check
joinable() to avoid deadlocks or double-joins.
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit