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.

Sign up for GitHub

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

Jump to bottom

fix: Adding INetworkSerializeByMemcpy where recommended for network serialization [MTT-5912] #822

Merged
merged 5 commits into from
Apr 20, 2023
Merged

fix: Adding INetworkSerializeByMemcpy where recommended for network serialization [MTT-5912] #822

merged 5 commits into from
Apr 20, 2023

Conversation

fernando-cortez
Copy link
Collaborator

@fernando-cortez fernando-cortez commented Apr 19, 2023 •
edited
Loading

Description

INetworkSerializeByMemcpy interface is recommended usage for network messages containing value types. It for allows some bandwidth optimization techniques (byte packing) that are otherwise unavailable.

It is not recommended in places where object types are serialized, nor when FixedStrings are used.

INetworkSerializeByMemcpy [when used with FixedStrings] struct is not recommended because it will lose the bandwidth optimization - an empty FixedString128Bytes serialized normally or through INetworkSerializable will use 4 bytes of bandwidth, but inside an INetworkSerializeByMemcpy, that same empty value would consume 132 bytes of bandwidth.

Issue Number(s)

MTT-5912

Contribution checklist

  • Tests have been added for boss room and/or utilities pack
  • Release notes have been added to the project changelog file and/or package changelog file
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • JIRA ticket ID is in the PR title or at least one commit message
  • Include the ticket ID number within the body message of the PR to create a hyperlink
  • An Index entry has been added in readme.md if applicable

@fernando-cortez fernando-cortez added 2-Easy This PR is trivial and can be reviewed quickly 1-Needs Review PR needs attention from the assignee and reviewers labels Apr 19, 2023
@fernando-cortez fernando-cortez changed the title fix: Adding INetworkSerializeByMemcpy where recommended for network serialization fix: Adding INetworkSerializeByMemcpy where recommended for network serialization [MTT-5912] Apr 20, 2023
SamuelBellomo
SamuelBellomo reviewed Apr 20, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
SamuelBellomo
SamuelBellomo previously approved these changes Apr 20, 2023
View reviewed changes
SamuelBellomo
SamuelBellomo reviewed Apr 20, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@fernando-cortez fernando-cortez merged commit 0281f5e into develop Apr 20, 2023
@fernando-cortez fernando-cortez deleted the fix/inetworkserializebymemcpy-usage branch April 20, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SamuelBellomo SamuelBellomo SamuelBellomo left review comments

@LPLafontaineB LPLafontaineB LPLafontaineB approved these changes

Assignees

@LPLafontaineB LPLafontaineB

Labels
1-Needs Review PR needs attention from the assignee and reviewers 2-Easy This PR is trivial and can be reviewed quickly
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fernando-cortez @SamuelBellomo @LPLafontaineB