Play Solo Auto Connect #468

claylittlehorse · 2021-08-11T03:03:03Z

Adds a setting to automatically re-connect to the rojo server when entering play solo

plugin/src/init.server.lua

plugin/src/App/Components/PlaySoloAutoConnector.lua

plugin/src/App/PluginSettings.lua

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

* Implement ChangeBatcher * Use ChangeBatcher for two-way sync * Pause updates during patch application * I can English good * Break after encountering a nil Parent change This prevents __flush from erroring out when an instance's Parent is changed to nil and it has other property changes in the same batch. * Update rbx_dom_lua * Don't connect changed listeners in a running game #468 made me realize how bad of an idea this is in general... * Update TestEZ and fix sibling Ref reification test * Add ChangeBatcher tests * Test instance unpausing by breaking functionality out to __cycle * Break up the module a bit and improve tests * Shuffle requires around and edit comment * Break out more stuff, rename createChangePatch -> createPatchSet * Make ChangeBatcher responsible for unpausing all paused instances This somewhat improves the situation (of course, it would preferrable to not have to hack around this problem with Source at all). It also sets us up nicely if we come across any other properties that do anything similar. * Remove old reference to pausedBatchInstances * Use RenderStepped instead of Heartbeat and trash multi-frame pauses I probably should have done this in the first place... ChangeBatcher still needs to unpause instances, but we don't need to hold pauses for any longer than one cycle. * Remove useless branch * if not next(x) -> if next(x) == nil * Add InstanceMap:unpauseAllInstances, use it in ChangeBatcher * Move IsRunning check to InstanceMap:__maybeFireInstanceChanged

sasial-dev · 2022-07-01T05:22:49Z

Other than merge conflicts, is there anything else this PR needs to get merged?

LPGhatguy · 2022-07-02T02:23:50Z

Other than merge conflicts, is there anything else this PR needs to get merged?

In general, yeah! There's a bit of feature design work that I think we need to do, especially around making sure Rojo won't try to overwrite stuff that changes at runtime.

I've tagged PRs on the repo to try to indicate what their current status is.

* Implement ChangeBatcher * Use ChangeBatcher for two-way sync * Pause updates during patch application * I can English good * Break after encountering a nil Parent change This prevents __flush from erroring out when an instance's Parent is changed to nil and it has other property changes in the same batch. * Update rbx_dom_lua * Don't connect changed listeners in a running game rojo-rbx#468 made me realize how bad of an idea this is in general... * Update TestEZ and fix sibling Ref reification test * Add ChangeBatcher tests * Test instance unpausing by breaking functionality out to __cycle * Break up the module a bit and improve tests * Shuffle requires around and edit comment * Break out more stuff, rename createChangePatch -> createPatchSet * Make ChangeBatcher responsible for unpausing all paused instances This somewhat improves the situation (of course, it would preferrable to not have to hack around this problem with Source at all). It also sets us up nicely if we come across any other properties that do anything similar. * Remove old reference to pausedBatchInstances * Use RenderStepped instead of Heartbeat and trash multi-frame pauses I probably should have done this in the first place... ChangeBatcher still needs to unpause instances, but we don't need to hold pauses for any longer than one cycle. * Remove useless branch * if not next(x) -> if next(x) == nil * Add InstanceMap:unpauseAllInstances, use it in ChangeBatcher * Move IsRunning check to InstanceMap:__maybeFireInstanceChanged

jackTabsCode · 2024-01-28T01:32:25Z

I'm curious what's blocking this feature from happening. This is something I am looking for! Would save me a decent chunk of time when testing.

Dekkonot · 2024-01-29T16:29:15Z

I don't see a reason this is blocked beyond the fact that I've not reviewed this PR and it's old. I've asked @boatbomber to take a look since he's more in tune with Rojo's plugin source.

We could solve this in a few ways and this is just one of them, but I'd like his opinion since it'll inevitably be him that maintains any code for it we add (unless something critical breaks).

claylittlehorse · 2024-01-29T18:18:14Z

Since writing this, and having some experience using it in my own local fork of Rojo, I ran into some issues and weird behaviors. It's been too long since I've used this feature personally to remember what those issues are, so I can't provide much insight into solutions. I wonder if other approaches to solving the problem would be more reliable.

boatbomber

I have some concerns about implementation, and I'm not sure if we ever addressed LPG's note about avoiding overwriting dynamic content? We might need to force ignoreUnknownInstances on everything during a Play Solo connection.

This PR is also 3 years old and out of date, it might just be easier to start a new one.

boatbomber · 2024-01-30T00:09:40Z

plugin/src/App/Components/PlaySoloAutoConnector.lua

+	return nil
+end
+
+function PlaySoloAutoConnector:didMount()


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()

boatbomber · 2024-01-30T00:11:19Z

plugin/src/App/init.lua

 			local address = ("%s:%s"):format(host, port)
+
+			if RunService:IsClient() and RunService:IsServer() and not RunService:IsRunning() then


Isn't this just RunService:IsEdit()?

boatbomber · 2024-01-30T00:12:46Z

plugin/src/App/init.lua

+
+				settings:set("activeConnections", activeConnections)
+
+				workspace:SetAttribute("__RojoConnectionId", connectionId)


~~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

boatbomber · 2024-01-30T00:28:53Z

plugin/src/App/init.lua

+					table.remove(activeConnections, 1)
+				end
+
+				settings:set("activeConnections", activeConnections)


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, "|"))

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

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

Dekkonot · 2024-01-31T22:14:31Z

Implemented in #840

claylittlehorse added 9 commits July 1, 2021 17:41

Add playSoloAutoConnect setting

b753568

Keep track of active connections

79bb63b

Reconnect in play solo if connected in edit mode

9b3cbf1

Move connection logic from NotConnected to PlaySoloAutoConnector

2b1647d

Remove playSoloAutoConnect from ServeSession

3826447

Remove left over settings prop in NotConnectedPage

bff2fe7

Make removeActiveConnection not be in state

52ee653

Make removeActiveConnection not be in state

6d695c2

Remove unused project name field from active connection

e6096be

Kampfkarren reviewed Aug 11, 2021

View reviewed changes

plugin/src/init.server.lua Outdated Show resolved Hide resolved

Kampfkarren reviewed Aug 11, 2021

View reviewed changes

plugin/src/App/Components/PlaySoloAutoConnector.lua Show resolved Hide resolved

Kampfkarren reviewed Aug 11, 2021

View reviewed changes

plugin/src/App/PluginSettings.lua Outdated Show resolved Hide resolved

claylittlehorse and others added 5 commits August 11, 2021 21:02

Update plugin/src/App/Components/PlaySoloAutoConnector.lua

8435b7c

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Update plugin/src/App/PluginSettings.lua

4da8e8b

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Fix double unmounting

6b23d09

Merge branch 'master' of https://github.com/zoebasil/rojo into master

8ee87a4

Use BindToClose for backup unmounting

7664422

claylittlehorse added 2 commits November 30, 2021 13:18

Merge branch 'master' of https://github.com/rojo-rbx/rojo into master

06df3a6

Only bind app unmount to close if is server

8b50463

LPGhatguy mentioned this pull request Jun 22, 2022

Rojo Hot-Reloading #559

Open

LPGhatguy added the status: needs refresh We need to put in a little elbow grease to get this to the finish line! label Jun 29, 2022

sasial-dev mentioned this pull request Jul 23, 2022

Send viewport notification when a game is running without being connected (or a setting) #586

Closed

Dekkonot requested a review from boatbomber January 29, 2024 16:27

boatbomber requested changes Jan 30, 2024

View reviewed changes

boatbomber mentioned this pull request Jan 30, 2024

Play Solo & Test Server auto connect #840

Merged

Dekkonot closed this Jan 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Play Solo Auto Connect #468

Play Solo Auto Connect #468

claylittlehorse commented Aug 11, 2021

sasial-dev commented Jul 1, 2022

LPGhatguy commented Jul 2, 2022

jackTabsCode commented Jan 28, 2024

Dekkonot commented Jan 29, 2024

claylittlehorse commented Jan 29, 2024 •

edited

Loading

boatbomber left a comment

boatbomber Jan 30, 2024

boatbomber Jan 30, 2024

claylittlehorse Jan 30, 2024 •

edited

Loading

boatbomber Jan 30, 2024 •

edited

Loading

boatbomber Jan 30, 2024 •

edited

Loading

claylittlehorse Jan 30, 2024 •

edited

Loading

boatbomber Jan 30, 2024

Dekkonot commented Jan 31, 2024

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

		if RunService:IsClient() and RunService:IsServer() and not RunService:IsRunning() then


		settings:set("activeConnections", activeConnections)

		workspace:SetAttribute("__RojoConnectionId", connectionId)

Play Solo Auto Connect #468

Play Solo Auto Connect #468

Conversation

claylittlehorse commented Aug 11, 2021

sasial-dev commented Jul 1, 2022

LPGhatguy commented Jul 2, 2022

jackTabsCode commented Jan 28, 2024

Dekkonot commented Jan 29, 2024

claylittlehorse commented Jan 29, 2024 • edited Loading

boatbomber left a comment

Choose a reason for hiding this comment

boatbomber Jan 30, 2024

Choose a reason for hiding this comment

boatbomber Jan 30, 2024

Choose a reason for hiding this comment

claylittlehorse Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

boatbomber Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

boatbomber Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

claylittlehorse Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

boatbomber Jan 30, 2024

Choose a reason for hiding this comment

Dekkonot commented Jan 31, 2024

claylittlehorse commented Jan 29, 2024 •

edited

Loading

claylittlehorse Jan 30, 2024 •

edited

Loading

boatbomber Jan 30, 2024 •

edited

Loading

boatbomber Jan 30, 2024 •

edited

Loading

claylittlehorse Jan 30, 2024 •

edited

Loading