Fix Reconnection Logic #168

jdetter · 2024-10-12T12:18:19Z

Description of Changes

Describe what has been changed, any new features or bug fixes

switched our "already connected" logic to using a reference to a MonoBehaviour instead of just a bool. MonoBehaviours are typically destroyed when a scene reload happens and in this case we'll want to allow developers to spawn a new SpacetimeDBNetworkManager if the previous one has been destroyed.

API

This is not an API break.

This is an API breaking change to the SDK

If the API is breaking, please state below what will break

Requires SpacetimeDB PRs

List any PRs here that are required for this SDK change to work

C# codegen making state relative to a DbConnection SpacetimeDB#1869

Testsuite

SpacetimeDB branch name: master

Testing

Write instructions for a test that you performed for this PR

I have added in several new tests here, one of which is a reconnection test. Also, the reason why we couldn't have more than 1 test before this is that it was required for reconnections to be working in order to have multiple tests running in the testsuite. Now that we have fixed reconnections I have enabled all of the tests.

Testsuite passes

jdetter · 2024-10-12T13:09:46Z

~~This only fixes some surface level issues, there are some deeper problems here with how we handle client side indexes that will need to be fixed.~~

^ This has been fixed by Jeremie's SpacetimeDB PR

jdetter · 2024-10-22T11:37:51Z

src/SpacetimeDBNetworkManager.cs

 	public class SpacetimeDBNetworkManager : MonoBehaviour
 	{
-		private static bool _alreadyInitialized;


This wasn't exposed before to the users so this is not an API break

jdetter · 2024-10-22T11:42:49Z

src/SpacetimeDBNetworkManager.cs

 using SpacetimeDB;
 using UnityEngine;

 namespace SpacetimeDB
 {
-	// This class is only used in Unity projects.


sorry for the whitespace changes here, this file wasn't properly formatted before.

reminder that you can view the diff with whitespace turned off

jdetter · 2024-10-22T11:43:46Z

unity-tests/client/Assets/Scripts/autogen/_Globals/SpacetimeDBClient.cs

@@ -15,7 +15,7 @@ public sealed class RemoteTables
 	{
 		public class CircleHandle : RemoteTableHandle<EventContext, Circle>


this file is automatically generated

RReverser · 2024-10-22T14:31:45Z

src/SpacetimeDBNetworkManager.cs

+        private void ForEachConnection(Action<IDbConnection> action)
+        {
+            // TODO(jdetter): We're doing this for now because we can't break the API during a minor release but
+            // in the future we should just change ActiveConnections to be a list so that we can reverse-iterate


ActiveConnections is already not part of the public API though? Or do you mean some other API?

I thought that ActiveConnections was referenced in the bindings somewhere and I was worried about creating an API break there, but I grep'd in the SpacetimeDB project for ActiveConnections and there's nothing there so maybe you're right. Thanks for the heads up 👍

It has internal visibility so bindings couldn't use it even if they wanted to.

unity-tests~/client/Assets/Scripts/autogen/_Globals/SpacetimeDBClient.cs

**Please do not rebase this PR** ## Description of Changes *Describe what has been changed, any new features or bug fixes* This is very similar to #176 except it imports the circle game as a submodule instead of copying the code over into this repo. This is the SpacetimeDBCircleGame PR that we're dependent on right now: clockworklabs/SpacetimeDBCircleGame#3 - This PR introduces a testsuite which runs in Unity. Right now it just spawns in a circle, eats some food and verifies the decay logic is working correctly. I've also written some reconnection tests but they don't work because reconnections are currently broken. There are also one-off tests but those don't work either because they also require reconnections to be working. Update: reconnections have been fixed via #168. I've used the built-in unity testsuite framework to achieve this, along with the UnityCI tool from GameCI. The documentation for this docker container can be found here: https://game.ci/docs/github/getting-started/ ## API - [ ] This is an API breaking change to the SDK *If the API is breaking, please state below what will break* Not breaking ## Requires SpacetimeDB PRs *List any PRs here that are required for this SDK change to work* None ## Testsuite SpacetimeDB branch name: 0935b7346b825b8cbb9f36d9ed256136b73b5f0b ## Testing *Write instructions for a test that you performed for this PR* - [x] The testsuite is passing: https://github.com/clockworklabs/com.clockworklabs.spacetimedbsdk/actions/runs/11604456943/job/32313229775 You can follow test instructions here to double check my work: clockworklabs/SpacetimeDBCircleGame#3 ## Follow-up Actions - [ ] Rebase the reconnection logic PR onto this PR and re-enable the reconnection tests --------- Co-authored-by: John Detter <no-reply@boppygames.gg>

the future Add testing section to PR description (#164) v Below is a render of what the new PR description looks like *Describe what has been changed, any new features or bug fixes* - [ ] This is an API breaking change to the SDK *If the API is breaking, please state below what will break* *List any PRs here that are required for this SDK change to work* *Write instructions for a test that you performed for this PR* - [ ] Describe a test for this PR that you have completed Working on a more ergonomic CI workflow Workflow runs on staging Small bug fix Test again speedup Use public bindings Use serial instead of license Updated default branch to be master Disable all but one test because we don't support reconnections right now Small update Fixes Testsuite uses local reference to SDK Added comma debugging One more debug Small changes small sed fix Small fix - newline character was messing up branch name Added debug Updated how branch name is populated We'll give this a try to make the C# SDK happy C# tests building! Updated README Small fix Move unity-tests to unity-tests~ Use relative path reference Use relative path reference Small update to the workflow Updated README for new functionality

causing errors. Disconnected connection should be removed from active connections Small fix that has to do with the active connections list being modified during enumeration Small fix Another fix Small simplification, should be working now Small improvement to how OnDestroy works Small fix Compilation fix :shrug-emoji: Updated tests NPM ignore file Fix reconnection issues Small compilation fix Small fix Fixes for reconnections Reformat Self review Restored API - no API breaks here Fix C# SDK Tests Super small polish Updated readme + publish/generate scripts Move unity-tests to unity-tests~ Resolving conflicts Fixing conflicts

src/SpacetimeDBNetworkManager.cs

jdetter mentioned this pull request Oct 12, 2024

Reconnections in C# SDK #175

Closed

jdetter force-pushed the jdetter-fix-reconnection-logic branch 2 times, most recently from c764afb to 0bb78dd Compare October 16, 2024 03:52

jdetter force-pushed the staging branch from f1b8fa8 to c74b1fd Compare October 16, 2024 04:44

bfops added the release-rc1 label Oct 21, 2024

jdetter changed the base branch from staging to jdetter/unity-testsuite October 22, 2024 09:06

jdetter force-pushed the jdetter-fix-reconnection-logic branch from 0bb78dd to 14cf1a3 Compare October 22, 2024 09:30

jdetter force-pushed the jdetter/unity-testsuite branch from 5d47384 to 25a8c1b Compare October 22, 2024 09:53

jdetter force-pushed the jdetter-fix-reconnection-logic branch from 14cf1a3 to 8c9c9e7 Compare October 22, 2024 09:59

jdetter commented Oct 22, 2024

View reviewed changes

jdetter mentioned this pull request Oct 22, 2024

Unity Testsuite #176

Closed

2 tasks

jdetter marked this pull request as ready for review October 22, 2024 12:57

jdetter requested review from RReverser, bfops and lcodes October 22, 2024 12:57

jdetter force-pushed the jdetter/unity-testsuite branch from 746d728 to 547187e Compare October 22, 2024 13:07

jdetter force-pushed the jdetter-fix-reconnection-logic branch from 602d07b to 9c25cf7 Compare October 22, 2024 13:12

RReverser reviewed Oct 22, 2024

View reviewed changes

unity-tests~/client/Assets/Scripts/autogen/_Globals/SpacetimeDBClient.cs Outdated Show resolved Hide resolved

RReverser mentioned this pull request Oct 22, 2024

C# codegen making state relative to a DbConnection clockworklabs/SpacetimeDB#1869

Merged

Centril force-pushed the jdetter/unity-testsuite branch from a39e0a3 to 87fa4fc Compare October 30, 2024 12:05

lcodes force-pushed the jdetter-fix-reconnection-logic branch 2 times, most recently from ed0fcdf to 3dcfbad Compare October 30, 2024 20:33

jdetter mentioned this pull request Oct 30, 2024

Unity testsuite as a git submodule #186

Merged

3 tasks

lcodes changed the base branch from jdetter/unity-testsuite to staging October 31, 2024 17:53

lcodes force-pushed the jdetter-fix-reconnection-logic branch from 3dcfbad to 56f82c3 Compare October 31, 2024 18:07

John Detter and others added 16 commits October 31, 2024 14:23

Update npmignore

9f104df

Use local package ref

f6988f6

Simplify review

34cdb73

Fail on dirty bindings

9163216

pass --anonymous to fix unity tests

c538cee

unity-tests, GameManager, WithCredentials: pass localIdentity

25c41aa

hack: WithCredentials takes Identity?

463d0fc

revert some identity stuff and try a new fix without nullable type

68339ec

Editor code compiles

3125644

Updated csproj so that tests are ignored for dotnet testsuite

07f071f

Rebase fun

7fb5a31

More rebase fun

ed6b3a5

Remove old files

1ab612a

Remove direct reference to submodule

41e0bd5

lcodes force-pushed the jdetter-fix-reconnection-logic branch from 44c95c7 to 41e0bd5 Compare October 31, 2024 18:23

lcodes and others added 2 commits October 31, 2024 16:26

Enable default compile items again

a8605f9

empty commit to bump CI

31c5fbe

lcodes reviewed Oct 31, 2024

View reviewed changes

src/SpacetimeDBNetworkManager.cs Outdated Show resolved Hide resolved

lcodes reviewed Oct 31, 2024

View reviewed changes

src/SpacetimeDBNetworkManager.cs Outdated Show resolved Hide resolved

John Detter added 3 commits October 31, 2024 22:05

Enable all tests

6a59012

Addressing Jeremie's feedback

fa7b2cc

Idk why this file changed - reverting it

3df246e

lcodes approved these changes Oct 31, 2024

View reviewed changes

John Detter added 2 commits October 31, 2024 22:22

Update workflow job names so they're not just 'test'

6264ae4

Update bindings workflow to only check for bindings changes

a43e18e

jdetter enabled auto-merge (squash) October 31, 2024 22:34

jdetter merged commit 3581ed7 into staging Oct 31, 2024
3 of 4 checks passed

jdetter deleted the jdetter-fix-reconnection-logic branch October 31, 2024 22:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Reconnection Logic #168

Fix Reconnection Logic #168

jdetter commented Oct 12, 2024 •

edited by bfops

Loading

jdetter commented Oct 12, 2024 •

edited

Loading

jdetter Oct 22, 2024

jdetter Oct 22, 2024

jdetter Oct 22, 2024

jdetter Oct 22, 2024

RReverser Oct 22, 2024

jdetter Oct 22, 2024

RReverser Oct 22, 2024

		@@ -15,7 +15,7 @@ public sealed class RemoteTables
		{
		public class CircleHandle : RemoteTableHandle<EventContext, Circle>

Fix Reconnection Logic #168

Fix Reconnection Logic #168

Conversation

jdetter commented Oct 12, 2024 • edited by bfops Loading

Description of Changes

API

Requires SpacetimeDB PRs

Testsuite

Testing

jdetter commented Oct 12, 2024 • edited Loading

jdetter Oct 22, 2024

Choose a reason for hiding this comment

jdetter Oct 22, 2024

Choose a reason for hiding this comment

jdetter Oct 22, 2024

Choose a reason for hiding this comment

jdetter Oct 22, 2024

Choose a reason for hiding this comment

RReverser Oct 22, 2024

Choose a reason for hiding this comment

jdetter Oct 22, 2024

Choose a reason for hiding this comment

RReverser Oct 22, 2024

Choose a reason for hiding this comment

jdetter commented Oct 12, 2024 •

edited by bfops

Loading

jdetter commented Oct 12, 2024 •

edited

Loading