Skip to content

Conversation

@lynzrand
Copy link
Contributor

@lynzrand lynzrand commented Jan 20, 2023

Closes #1550.

In this PR, I added a public API so that I can get the encoded length of an integer, and then fixed the issue using the first method I mentioned in #1550.

static MessagePack.MessagePackWriter.IntegerEncodedLength(long value) -> int
static MessagePack.MessagePackWriter.IntegerEncodedLength(ulong value) -> int

I have yet to look into the test folder and add tests at the time of writing. Just added tests.

This is my first PR to this project, so please tell me if I did anything wrong.

@lynzrand lynzrand marked this pull request as ready for review January 20, 2023 09:56
@lynzrand
Copy link
Contributor Author

Just added tests. As expected, the tests failed in master branch and passed in this PR.

Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. It merits some discussion before merging.

@lynzrand
Copy link
Contributor Author

By the way, although I highly doubt that it would ever run on a big-endian device, I don't think the byte order reversing code is correct.

https://github.com/neuecc/MessagePack-CSharp/blob/53d6c1617789488d15e391e7f86aa0eba7ed64b2/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Unity/Extension/UnsafeBlitFormatter.cs#L65-L74

It seems to be reversing the whole data structure instead of reversing each float value.

@AArnott AArnott changed the base branch from master to develop March 12, 2023 22:25
@AArnott AArnott added this to the v2.5 milestone Mar 12, 2023
@AArnott AArnott merged commit 58c4d3c into MessagePack-CSharp:develop Mar 12, 2023
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.

UnsafeBlitFormatter for Unity types is not compatible with the MessagePack specification

2 participants