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.

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

C# codegen making state relative to a DbConnection #1869

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

lcodes
Copy link
Contributor

@lcodes lcodes commented Oct 17, 2024

Description of Changes

Make the static fields indexing primary keys instance fields.

API and ABI breaking changes

Expected complexity level and risk

Testing

Codegen compiles. Same code as before but going through a this pointer instead of a static .

Context from Boppy: The Unity testsuite exercises this feature in the testsuite: clockworklabs/com.clockworklabs.spacetimedbsdk#168

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Tested via clockworklabs/com.clockworklabs.spacetimedbsdk#176

Code looks good to me, thanks @lcodes!

@jdetter jdetter self-requested a review October 22, 2024 15:50
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Ingvar left some requested changes here: clockworklabs/com.clockworklabs.spacetimedbsdk#168 (comment)

I'm temporarily removing my approval so that he has an opportunity to review here

@bfops bfops removed their request for review October 23, 2024 16:24
@jdetter jdetter self-requested a review October 24, 2024 05:35
@lcodes lcodes force-pushed the jeremie/cs-connection-state-fix branch from 70d929c to 7b083e2 Compare October 25, 2024 15:18
@lcodes
Copy link
Contributor Author

lcodes commented Oct 25, 2024

@RReverser updated, should be good to go now

Copy link
Member

@RReverser RReverser left a comment

Choose a reason for hiding this comment

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

Minor nits, but fine to go as-is too.

@lcodes lcodes force-pushed the jeremie/cs-connection-state-fix branch from 405cd7a to 65220bf Compare October 29, 2024 17:45
@lcodes lcodes force-pushed the jeremie/cs-connection-state-fix branch from 2caee9e to 2264653 Compare October 30, 2024 19:07
@jdetter jdetter dismissed their stale review October 30, 2024 20:54

I am not blocking this from merging

@lcodes lcodes added this pull request to the merge queue Oct 30, 2024
Merged via the queue into master with commit 1328b5d Oct 30, 2024
8 checks passed
@lcodes lcodes deleted the jeremie/cs-connection-state-fix branch October 30, 2024 21:13
jdetter added a commit to clockworklabs/com.clockworklabs.spacetimedbsdk that referenced this pull request Oct 31, 2024
## 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. `MonoBehaviour`s 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*

- clockworklabs/SpacetimeDB#1869

## Testsuite

SpacetimeDB branch name: master

## Testing
*Write instructions for a test that you performed for this PR*

- [x] 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


![image](https://github.com/user-attachments/assets/09ef5835-f2c7-41f1-af6b-e612ac5e0497)

---------

Co-authored-by: John Detter <no-reply@boppygames.gg>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Co-authored-by: Jeremie Pelletier <jeremiep@gmail.com>
Co-authored-by: Zeke Foppa <bfops@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reconnections in C# SDK
4 participants