point-platform / dasher

Fast, lightweight, cross platform serialisation tool
Apache License 2.0
7 stars 6 forks source link

Custom unpacker #4

Closed andysturrock closed 8 years ago

andysturrock commented 8 years ago

The custom unpacker seems to fail when reading the map length and then a string. This first became apparent when I tried using the custom unpacker with the strict deserialiser. I'll take a look but I wondered whether you knew about this anyway. Or am I using it incorrectly?

andysturrock commented 8 years ago

OK fixed the bug now. There was a missing _nextByte = -1; in TryReadMapLength. All tests pass as expected.

I'll try putting the custom unpacker in the strict deserialiser next. Maybe then we can merge to master?

drewnoakes commented 8 years ago

Apologies for the delay in responding to your issues/PRs. Have been catching up with family in Oz and my parent's internet was out for a few days. Back online now.

I had a few uninterrupted hours to work on this on the flight, somewhere over Iran I think. I found the issue I think you're referring to in this PR and fixed it in 31f4dd394ade088f6ecf8b003d1cd4ee6497c4fa. There's a solid test case there that stresses all combinations of values to draw out these kinds of issues.

I also switched to the custom unpacker in 09df0ff3d7bc9ec51373155c164e16070fd24c4d. This removes the dependency upon MsgPack.Cli in that project.

This PR was closed automatically when I deleted the previously merged custom-unpacker branch. From looking at it, I think all of this ended up on master one way or another.

Will respond to your other issues now.