-
Notifications
You must be signed in to change notification settings - Fork 99
Add method to script editor websocket that lists scripts available for subscription #5182
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?
Add method to script editor websocket that lists scripts available for subscription #5182
Conversation
…ubscription Signed-off-by: WolfGangS <[email protected]>
Signed-off-by: WolfGangS <[email protected]>
marchcat
left a comment
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.
Thank you!
Signed-off-by: WolfGangS <[email protected]>
| return LLSD(); | ||
| }); | ||
| script_connection->registerMethod("script.list", | ||
| [that, connection_id](const std::string&, const LLSD&, const LLSD& params) -> 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.
Mac build complains about unused connection_id here.
error: lambda capture 'connection_id' is not used [-Werror,-Wunused-lambda-capture]
290 | [that, connection_id](const std::string&, const LLSD&, const LLSD& params) -> LLSD
| [that, connection_id](const std::string&, const LLSD&, const LLSD& params) -> LLSD | |
| [that](const std::string&, const LLSD&, const LLSD& params) -> LLSD |
Rider-Linden
left a comment
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.
Just Marchcat's change and the script unsubscribe list thing.
| { | ||
| ++it; | ||
| it->second.mConnectionID = 0; | ||
| it->second.mConnection.reset(); |
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.
This is going to leave the dead subscription record in the map....
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.
mSubscriptions is a map of scripts open in sl available to subscribe to, as they don't have a connection set until a websocket connetion subscribes.
Scripts are added when a user presses edit on the in viewer script editor. If they are removed from mSubscriptions when a websocket unsubscribes, then the user has to press edit in sl again to be able to resubscribe to the script.
For external tools to be able to reconnect, the script must still exist in the subscriptions list to subscribe to them.
So this change makes socket unsubscribe only do the inverse of what subscribing does in handleScriptSubscribe,
So the behavior is
- Add entry to mSubscritpions - Press
Editon in viewer script editor window -subscribeScriptEditor - Remove entry from mSubscriptions - Close in viewer script editor window -
unsubscribeEditor - Set mSubscription connection - Web socket connects and subscribes to script -
handleScriptSubscribe - Unset mSubscription connection - Web socket unsubscribes or closes. -
unsubscribeConnection
If that's not desirable, then this might need a larger rework, to achieve resubscribing.
Description
Adds a method for the web socket server to allow 3rd party tools to query scripts available for subscription. Useful for the sl-vscode-plugin to be able to resubscribe after a connection loss (like the user updated vscode, or similar)
Also useful for plans to add support for a cli version of the plugin, to allow broader editor compatibility.
Related Issues
Issue Link: relates to secondlife/sl-vscode-plugin#43
Checklist
Please ensure the following before requesting review:
Additional Notes
Discussed with @Rider-Linden before, so likely more his wheelhouse for review.
I suspect there may be some issue I am missing with my changes to
LLScriptEditorWSServer::unsubscribeConnectionbut they (or something like them) are needed else a websocket connection closing flushes scripts from the subscription list, even though they are still open in viewer, and on disk in the temp directory.