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

fix: Use proper length for number of total cells in area encoder #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

okdistribute
Copy link

@okdistribute okdistribute commented Nov 21, 2020

For a simple area with 3 nodes, I ran the encoder and got a lot of zeros at the end using the latest code on the master branch. (I can send this file to you in a gist if you want).

Looking at the code, it seems like there is a calculation for the size of an area which I don't entirely understand. It seems like it should allocate for the # of cells encoded as varints? I'm not exactly sure what the old code was calculating -- if we need it, probably best we add a bit of documentation or comment about the 🧙 magic 🌟 numbers 👍 😄

EDIT: it isn't a big deal, I think if the length is too long.. then the output could just be trimmed once it's determined that the cell length is actually smaller than predicted.

Before:

03950283e4da06041983c7c26f1623421683c7c2251723421583c7c2f51823421983c7c26f162342010200010e3d4a616d656c277320486f75736500000000000000000000

After:

03950283e4da06041983c7c26f1623421683c7c2251723421583c7c2f51823421983c7c26f162342010200010e3d4a616d656c277320486f75736500

@okdistribute okdistribute requested review from mk30 and a user February 4, 2021 02:37
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.

1 participant