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

Add calculation of serialized size #48

Merged
merged 2 commits into from
Jun 14, 2020
Merged

Conversation

ejmount
Copy link
Contributor

@ejmount ejmount commented Jun 6, 2020

I'm working on a server project that relies on this crate to handle NBT fields in the network protocol. To write out packets efficiently, it relies on being able to tell how long each data item will be before serialization begins, but there didn't seem to be a way to do that for NBT values with their existing API. (Beyond extracting sub-values and manually calculating it)

Since other people are likely to need this functionality, I thought I'd submit it as a PR. (I've not made any style changes or warning fixes in other code)

Specifically, the public API addition is that Value and Blob now have a serialized_size method that reports the number of bytes they will serialize into, without any compression. There is also a new test in tests.rs that constructs a Blob with at least one field of each existing tag, and asserts that the reported serialized size matches the length of the actual serialization output.

Hope this is useful to you, let me know if you'd like any changes.

@atheriel
Copy link
Collaborator

atheriel commented Jun 8, 2020

You may be amused to know that this crate used to provide a method for this, but I removed it in 8fb456c because I did not think anyone would actually use it. However, you have presented a pretty compelling use case, so I'm happy to see it re-added.

I do have a preference for len_bytes() over serialized_size() as a name, though.

@ejmount
Copy link
Contributor Author

ejmount commented Jun 9, 2020

Done. (After correcting a suppression that got left in)

@atheriel atheriel merged commit 968cd2d into PistonDevelopers:master Jun 14, 2020
@atheriel
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants