Skip to content
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

Jump to bottom

Play Solo Auto Connect #468

Closed
wants to merge 16 commits into from
Closed

Play Solo Auto Connect #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b753568
Add playSoloAutoConnect setting
claylittlehorse Jul 1, 2021
79bb63b
Keep track of active connections
claylittlehorse Jul 2, 2021
9b3cbf1
Reconnect in play solo if connected in edit mode
claylittlehorse Aug 11, 2021
2b1647d
Move connection logic from NotConnected to PlaySoloAutoConnector
claylittlehorse Aug 11, 2021
3826447
Remove playSoloAutoConnect from ServeSession
claylittlehorse Aug 11, 2021
bff2fe7
Remove left over settings prop in NotConnectedPage
claylittlehorse Aug 11, 2021
52ee653
Make removeActiveConnection not be in state
claylittlehorse Aug 11, 2021
6d695c2
Make removeActiveConnection not be in state
claylittlehorse Aug 11, 2021
e6096be
Remove unused project name field from active connection
claylittlehorse Aug 11, 2021
8435b7c
Update plugin/src/App/Components/PlaySoloAutoConnector.lua
claylittlehorse Aug 12, 2021
4da8e8b
Update plugin/src/App/PluginSettings.lua
claylittlehorse Aug 12, 2021
6b23d09
Fix double unmounting
claylittlehorse Aug 12, 2021
8ee87a4
Merge branch 'master' of https://github.com/zoebasil/rojo into master
claylittlehorse Aug 12, 2021
7664422
Use BindToClose for backup unmounting
claylittlehorse Sep 3, 2021
06df3a6
Merge branch 'master' of https://github.com/rojo-rbx/rojo into master
claylittlehorse Nov 30, 2021
8b50463
Only bind app unmount to close if is server
claylittlehorse Nov 30, 2021
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
38 changes: 38 additions & 0 deletions plugin/src/App/Components/PlaySoloAutoConnector.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
local RunService = game:GetService("RunService")

local Rojo = script:FindFirstAncestor("Rojo")

local Roact = require(Rojo.Roact)

local e = Roact.createElement

local PlaySoloAutoConnector = Roact.Component:extend("PlaySoloAutoConnector")

function PlaySoloAutoConnector:render()
claylittlehorse marked this conversation as resolved.
Show resolved Hide resolved
return nil
end

function PlaySoloAutoConnector:didMount()
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of this architecture, as it uses prop drilling and a component that just calls a function for the app. Instead, we can use the architecture that we now use for many other features- a call from App:init() that uses self:startSession()

local settings = self.props.settings

if RunService:IsServer() and RunService:IsRunning() and settings:get("playSoloAutoConnect") then
local connectionId = workspace:GetAttribute("__RojoConnectionId")

if connectionId then
local activeConnections = settings:get("activeConnections", true)
local activeConnection
for _, connection in ipairs(activeConnections) do
if connection.connectionId == connectionId then
activeConnection = connection
break
end
end

if activeConnection then
self.props.onConnect(activeConnection.host, activeConnection.port, activeConnection.sessionId)
end
end
end
end

return PlaySoloAutoConnector
10 changes: 8 additions & 2 deletions plugin/src/App/PluginSettings.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ local Rojo = script:FindFirstAncestor("Rojo")
local Roact = require(Rojo.Roact)

local defaultSettings = {
activeConnections = {},
openScriptsExternally = false,
playSoloAutoConnect = false,
twoWaySync = false,
}

Expand Down Expand Up @@ -35,7 +37,11 @@ function Settings.fromPlugin(plugin)
}, Settings)
end

function Settings:get(name)
function Settings:get(name, dontUseCache)
if dontUseCache then
return self.__plugin:GetSetting("Rojo_" .. name)
end

if defaultSettings[name] == nil then
error("Invalid setings name " .. tostring(name), 2)
end
Expand Down Expand Up @@ -118,4 +124,4 @@ end
return {
StudioProvider = StudioProvider,
with = with,
}
}
8 changes: 8 additions & 0 deletions plugin/src/App/StatusPages/Settings.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,14 @@ function SettingsPage:render()
layoutOrder = 2,
}),

PlaySoloAutoConnect = e(Setting, {
id = "playSoloAutoConnect",
name = "Play Solo Auto Connect",
description = "Attempt to automatically connect to the rojo server in Play Solo if connected in edit mode",
transparency = self.props.transparency,
layoutOrder = 2,
}),

Layout = e("UIListLayout", {
FillDirection = Enum.FillDirection.Vertical,
SortOrder = Enum.SortOrder.LayoutOrder,
Expand Down
74 changes: 66 additions & 8 deletions plugin/src/App/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
local HttpService = game:GetService("HttpService")
local RunService = game:GetService("RunService")

local Rojo = script:FindFirstAncestor("Rojo")
local Plugin = Rojo.Plugin

Expand All @@ -16,6 +19,7 @@ local Theme = require(script.Theme)
local PluginSettings = require(script.PluginSettings)

local Page = require(script.Page)
local PlaySoloAutoConnector = require(script.Components.PlaySoloAutoConnector)
local StudioToolbar = require(script.Components.Studio.StudioToolbar)
local StudioToggleButton = require(script.Components.Studio.StudioToggleButton)
local StudioPluginGui = require(script.Components.Studio.StudioPluginGui)
Expand Down Expand Up @@ -43,14 +47,14 @@ function App:init()
})
end

function App:startSession(host, port, sessionOptions)
function App:startSession(host, port, settings, expectedSessionId)
local baseUrl = ("http://%s:%s"):format(host, port)
local apiContext = ApiContext.new(baseUrl)

local serveSession = ServeSession.new({
apiContext = apiContext,
openScriptsExternally = sessionOptions.openScriptsExternally,
twoWaySync = sessionOptions.twoWaySync,
openScriptsExternally = settings:get("openScriptsExternally"),
twoWaySync = settings:get("twoWaySync"),
})

serveSession:onStatusChanged(function(status, details)
Expand All @@ -59,15 +63,57 @@ function App:startSession(host, port, sessionOptions)
appStatus = AppStatus.Connecting,
})
elseif status == ServeSession.Status.Connected then
if expectedSessionId and details.sessionId ~= expectedSessionId then
ServeSession:stop()
end

local address = ("%s:%s"):format(host, port)

if RunService:IsClient() and RunService:IsServer() and not RunService:IsRunning() then
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this just RunService:IsEdit()?

Copy link
Author

@claylittlehorse claylittlehorse Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TIL lol

local connectionId = HttpService:GenerateGUID()
local activeConnections = settings:get("activeConnections", true)
table.insert(activeConnections, {
connectionId = connectionId,
sessionId = details.sessionId,
host = host,
port = port
})

if #activeConnections > 15 then
table.remove(activeConnections, 1)
end

settings:set("activeConnections", activeConnections)
Copy link
Member

@boatbomber boatbomber Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't understand why we have this array and write it to settings.json.

You're creating a GUID to key the host/port/sessionId within this array, then sharing the GUID via the Attribute, then linearly searching the array to find the host/port/sessionId with this GUID when play solo.
(Why do a linear search? You can turn this array into a dict using the GUID as a key.)

I think this array and disc write is totally avoidable though, unless there's something I'm missing.
You can write the connection info to an easily parse-able string in the Attribute, so that the server window of the test can grab the host/port/sessionId without having to search through other connections from other projects in other windows.

IE:

-- When connecting
workspace:SetAttribute("__RojoConnectionInfo", host .. "|" .. port .. "|" .. details.sessionId)

-- When in play solo
local connectionInfo = workspace:GetAttribute("__RojoConnectionInfo")
if not connectionInfo then return end
local host, port, sessionId = table.unpack(string.split(connectionInfo, "|"))

Copy link
Author

@claylittlehorse claylittlehorse Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that the disc write isn’t the way to go. It felt shaky even when I implemented it.

As for the rest of your questions— I don’t have good answers for those, mostly because I can’t remember and I’d need to read back through the whole PR, which I’m not really interested in doing right now. (nor am I interested in continually addressing those issues in this PR)

I think it would be better for you to take over the PR and implement the suggestions you’ve put forward

Copy link
Member

Choose a reason for hiding this comment

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

Yup, I opened #840 for that. Thanks for getting this started though!


workspace:SetAttribute("__RojoConnectionId", connectionId)
Copy link
Member

@boatbomber boatbomber Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is putting temporary data into a permanent location- could we use a non-Archivable Instance for this, or do they disappear when you press Play Solo? It might simplify the cleanup logic.

Edit: non-Archivable Instances don't persist when running


self.removeActiveConnection = function()
workspace:SetAttribute("__RojoConnectionId", nil)
local activeConnections = settings:get("activeConnections", true)
for i = #activeConnections, 1, -1 do
local activeConnection = activeConnections[i]
if activeConnection.connectionId == connectionId then
table.remove(activeConnections, i)
break
end
end

settings:set("activeConnections", activeConnections)
end
end

self:setState({
appStatus = AppStatus.Connected,
projectName = details,
projectName = details.projectName,
address = address,
})
elseif status == ServeSession.Status.Disconnected then
self.serveSession = nil

if self.removeActiveConnection then
self.removeActiveConnection()
end

-- Details being present indicates that this
-- disconnection was from an error.
if details ~= nil then
Expand Down Expand Up @@ -111,6 +157,15 @@ function App:render()
e(PluginSettings.StudioProvider, {
plugin = self.props.plugin,
}, {
autoConnector = PluginSettings.with(function(settings)
return e(PlaySoloAutoConnector, {
settings = settings,

onConnect = function(host, port, expectedSessionId)
self:startSession(host, port, settings, expectedSessionId)
end,
})
end),
gui = e(StudioPluginGui, {
id = pluginName,
title = pluginName,
Expand Down Expand Up @@ -139,10 +194,7 @@ function App:render()
NotConnectedPage = PluginSettings.with(function(settings)
return createPageElement(AppStatus.NotConnected, {
onConnect = function(host, port)
self:startSession(host, port, {
openScriptsExternally = settings:get("openScriptsExternally"),
twoWaySync = settings:get("twoWaySync"),
})
self:startSession(host, port, settings)
end,

onNavigateSettings = function()
Expand Down Expand Up @@ -223,4 +275,10 @@ function App:render()
})
end

function App:willUnmount()
if self.removeActiveConnection then
self.removeActiveConnection()
end
end

return App
5 changes: 4 additions & 1 deletion plugin/src/ServeSession.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,10 @@ function ServeSession:start()

self.__apiContext:connect()
:andThen(function(serverInfo)
self:__setStatus(Status.Connected, serverInfo.projectName)
self:__setStatus(Status.Connected, {
projectName = serverInfo.projectName,
sessionId = serverInfo.sessionId,
})
self:__applyGameAndPlaceId(serverInfo)

local rootInstanceId = serverInfo.rootInstanceId
Expand Down
21 changes: 16 additions & 5 deletions plugin/src/init.server.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
local RunService = game:GetService("RunService")

if not plugin then
return
end
Expand All @@ -16,16 +18,25 @@ local Config = require(script.Config)
local App = require(script.App)

local app = Roact.createElement(App, {
plugin = plugin
plugin = plugin,
})
local tree = Roact.mount(app, nil, "Rojo UI")

plugin.Unloading:Connect(function()
Roact.unmount(tree)
end)
local unmounted = false
function unmount()
if not unmounted then
Roact.unmount(tree)
unmounted = true
end
end

plugin.Unloading:Connect(unmount)
if RunService:IsServer() then
game:BindToClose(unmount)
end

if Config.isDevBuild then
local TestEZ = require(script.Parent.TestEZ)

require(script.runTests)(TestEZ)
end
end