-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
change how save file actually works #120
base: main
Are you sure you want to change the base?
Conversation
} | ||
|
||
/// Response of the SaveFileNotification after the user interacts with the file save request | ||
table SaveFileRequest { |
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.
If this is a response, why is it named request?
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.
Dk if to call Response calls that come from only the server or if they can also come from the client
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.
I thought the naming we were using was this:
Request: Something that has 0-1 responses
Response: Always corresponds to a particular request
Notification: This does not correspond to any particular response, we fire and forget them for the most part.
So the notion of responding to a notification doesn't really make sense - then it would have been a request, and not a notification. Can you give an example of how this all works so I can try to better understand what to name it?
/// Response of the SaveFileNotification after the user interacts with the file save request | ||
table SaveFileRequest { | ||
/// ID of the SaveFile, given by SaveFileNotification | ||
id: uint32; |
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.
All primitives should be optional/nullable.
Also, what is your opinion on this: Should it be wrapped in a struct for type safety?
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.
what should be wrapped?
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.
like struct SaveFileId
I ended up not requiring
data
, I deleted it because SlimeVR hasn't used this yet