-
Notifications
You must be signed in to change notification settings - Fork 110
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
Fixed exceptions in C# SDK when someone disconnects or when a transaction originates from CLI #1461
Conversation
…tion originates from CLI
@@ -712,6 +712,8 @@ pub fn autogen_csharp_globals(items: &[GenItem], namespace: &str) -> Vec<(String | |||
); | |||
} | |||
writeln!(output, "\"<none>\" => null,"); | |||
writeln!(output, "\"__identity_disconnected__\" => null,"); |
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.
why only __identity_disconnected__
here and not also __identity_connected__
?
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.
Added it
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.
FWIW in modules we treat all events that start with __
and end with __
as "special" in various places.
Could do the same here instead of hardcoding a few names.
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.
IMO We should allow users to subscribe to these events instead of just suppressing the errors here, I've created an issue in the C# SDK: clockworklabs/com.clockworklabs.spacetimedbsdk#102
I can understand the BitCraft team not wanting to have to deal with these errors as they're doing development on the game so I'm fine to approve this as-is for now.
…C# SDK when someone disconnects or when a transaction originates from CLI Co-authored-by: Steve Boytsun <[email protected]>
Trivial
Description of Changes
Expected complexity level and risk
Complexity: 1/5
Risk: 1/5
This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!