-
Notifications
You must be signed in to change notification settings - Fork 4.3k
[MLA-825] Add default values for SideChannel IncomingMessages methods #3751
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
Conversation
/// <returns></returns> | ||
public string ReadString() | ||
public string ReadString(string defaultValue = default) |
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.
Should we default to ""
instead?
/// <returns></returns> | ||
public IList<float> ReadFloatList() | ||
public IList<float> ReadFloatList(IList<float> defaultValue = default) |
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.
Not sure if there's a better default here.
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.
LGTM
Co-Authored-By: Chris Goy <goyenator@gmail.com>
Co-Authored-By: Chris Goy <christopherg@unity3d.com>
Proposed change(s)
To assist with backwards compatibility across side channels, this adds a default value when reading from IncomingMessages. This will let us add data to the messages in the future, but still allow reading from older versions.
Note that this only falls back to the default if it's at the end of the buffer. Trying to read an int when there's only 1 byte left will still raise/throw, but I think this is the correct behavior in that case.
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
https://jira.unity3d.com/browse/MLA-825
Types of change(s)
Checklist
Other comments