-
Notifications
You must be signed in to change notification settings - Fork 83
Websockets Server implementation for script editor integration with VSCode or other external editors. #4599
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.
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
base: project/lua_editor
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great if the file would not randomly get re-ordered (again) to reduce merge conflicts for TPVs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly that's a side effect of the autobuild program.
:-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid the churn if possible --manual edit or figure out the sorting (presuming dictionary sorting differences with python llsd.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be all better now... 🤞
0a25b9e
to
c8b0729
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a WebSocket server infrastructure for enabling external script editor integration with the Second Life viewer, using JSON-RPC 2.0 protocol for structured communication.
- Adds WebSocket server and JSON-RPC 2.0 communication layer via WebSocketMgr and JSONRPCServer base classes
- Creates specialized script editor WebSocket connection handling with handshake protocol and editor association
- Integrates WebSocket server startup with existing script editor workflow when opening external editors
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
indra/newview/llscripteditorws.h | Header defining specialized script editor WebSocket server and connection classes |
indra/newview/llscripteditorws.cpp | Implementation of script editor WebSocket communication and editor association |
indra/newview/llpreviewscript.h | Added WebSocket connection methods and member to script container class |
indra/newview/llpreviewscript.cpp | Integrated WebSocket server startup into external editor workflow |
indra/newview/llappviewer.cpp | Added WebSocket manager update call to main event loop |
indra/newview/CMakeLists.txt | Added new script editor WebSocket source files to build |
indra/llcorehttp/llwebsocketmgr.h | Core WebSocket server and connection management infrastructure |
indra/llcorehttp/llwebsocketmgr.cpp | Implementation of WebSocket server with threading and connection handling |
indra/llcorehttp/lljsonrpcws.h | JSON-RPC 2.0 protocol implementation over WebSocket connections |
indra/llcorehttp/lljsonrpcws.cpp | JSON-RPC message processing, method registration, and error handling |
indra/llcorehttp/CMakeLists.txt | Added WebSocket and JSON-RPC source files to llcorehttp build |
indra/cmake/websocketpp.cmake | CMake configuration for websocketpp library dependency |
indra/cmake/CMakeLists.txt | Added websocketpp CMake module to build system |
autobuild.xml | Added websocketpp package dependency configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@Rider-Linden could we get a summary of the JSON-RPC API this exposes? Perhaps it'd be useful to provide a separate *.md documenting the interface for developers to reference. |
I'll make a copy of the one included in the vscode extension. I usually dislike having two copies of the same document, but I'm not sure there is a way around it. |
The server defines the spec: so I would think it would be appropriate to place in here, then link to the document from the extension repo. |
// TODO: Build an actual error message to forward to the external editor | ||
// Explaination: | ||
// Well! Heck! | ||
// As it turns out, the complete error message arrives as either two or three | ||
// separate chat messages from the server. | ||
// 2 if the script is LSL or if it is Lua but not owned by the editing agent | ||
// 3 if the script is Lua and owned by the editing agent. | ||
// | ||
// Message 1: <Object Name> [script:<Script Name>] Script run-time error | ||
// Message 2: <runtime error> | ||
// Message 3: <script>:<line>: <actual error message>\n | ||
// <call stack> | ||
// | ||
// These need to be compositited into a single error message to send to the IDE. | ||
// | ||
//if (lines.size() > 1) | ||
//{ // The second line is the actual error message | ||
// error_message = lines[1]; | ||
// remove_count++; | ||
// if ((error_message == "runtime error") && (lines.size() > 2)) | ||
// { // If the error message is just "runtime error", the next line might actually be the real message: | ||
// // "lua_script:7: attempt to perform arithmetic (sub) on nil" | ||
// static const boost::regex LUA_ERROR_REGEX(R"(^(.+?):(\d+):\s*(.+)$)"); | ||
// | ||
// if (boost::regex_match(first_line, m, RUNTIME_ERR_REGEX_FLEX)) | ||
// { | ||
// line_number = std::stoi(m[2].str()); | ||
// error_message = m[3].str(); | ||
// remove_count++; | ||
// } | ||
// } | ||
// else | ||
// { | ||
// error_message = "Unknown script runtime error"; | ||
// } | ||
//} | ||
//else | ||
//{ | ||
// error_message = "Unknown script runtime error"; | ||
//} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok... This bit has me flumoxed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marchcat Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I've been meaning to merge the last two into a single message, it just means that I need to add a new function to the server that allows sending a chat message to everyone in the sim except a single person. Then we can send most users the general error message, then a more specific one to the owner.
No idea about the first though. Things in-world might be listening for that message currently? Do Mono objects currently get 2 messages on error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Mono gets two messages. So, that will simplify things.
From the slack conversation:
- We'll consolidate this so that it is always 2 messages.
- We can not go to one message since that may break script crash monitors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd want to see a little bit more separation here of concerns into individual files (for example, LLJSONRPCServer and LLJSONRPCConnection), but that's not a blocker. Generally looks fine.
|
||
#include <websocketpp/common/connection_hdl.hpp> | ||
|
||
struct Server_impl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why just forward declare here and not just have Server_impl up here instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want the server implementation to be opaque to anything outside of llwebsocketmgr.cpp while not having to declare it as a void *
in the header.
…connection for all scripts.
Co-authored-by: Copilot <[email protected]>
…values. Added "runtime.debug" and "runtime.error" notifications.
3ce1955
to
5f9dd14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
indra/newview/llpreviewscript.cpp:1
- The variable 'connection_id' is assigned from 'it->second.mConnectionID' after 'it' has been erased, causing undefined behavior. Store the connection ID before erasing the subscription.
/**
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
indra/newview/skins/default/xui/en/floater_scripting_settings.xml
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Re-enable when test is debugged.
… empty lang def is not a success. Hopefully this will package correctly.
Description
Implements a websocket server that may be launched in the viewer to allow external programs two way communication with systems in the viewer.
Allows external code editors to communicate with the viewer and receive compile status and runtime information. It also allows for transfering the current language definitions from the viewer to the editor.
(See: doc/external-editor-json-rpc.md for full protocol.)
Related Issues
Needs a full test plan, should test in coordination with https://github.com/secondlife/sl-vscode-edit/