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

public HasMessagesToSend, unused field, fixed an assertion #115

Merged
merged 3 commits into from Dec 18, 2018

Conversation

Projects
None yet
2 participants
@geneotech
Copy link
Contributor

geneotech commented Dec 18, 2018

Hello Glenn,

I'm currently testing yojimbo for my top-down shooter as it looks like a really solid network stack.
I already did some gameplay tests and I think I'll stay with this library for quite some time to come!
I made some commits along the way that I think could perhaps be useful.

The first commit makes the channels' HasMessagesToSend methods accessible to the library's user
the same way that CanSendMessage methods are.
One use-case I can think of if is the initial state of the world is reaally huge, say, it takes several seconds to transfer.
The next server update, whether it's a delta-compressed snapshot or a list of inputs -
might grow so huge during the initial transfer that it's probably best sent using another block message, so the server would simply check if the initial block has gotten through using HasMessagesToSend, and if it did, send a correction that will get through a lot faster, and only then begin to send regular messages.

The second commit removes an unused field, which is blockData inside the SendBlockData. It compiles just fine without the field, and it's quite logical not to need it - as far as I understand, it is the library's user that allocates the necessary memory for the block through AllocateBlock, and later fills that memory. This is in contrast to ReceiveBlockData, which needs a working space ready whenever a block starts coming in.

The third commit is self-explanatory.
The tests pass - at least for me!

Cheers!

@geneotech geneotech changed the title public HasMessagesToSend, unused variable, fixed an assertion public HasMessagesToSend, unused field, fixed an assertion Dec 18, 2018

geneotech added some commits Dec 6, 2018

Fix a length assertion in serialize_string_internal
This fixes an assertion hit when serializing a perfectly valid string like this:

char cool[5] = { 'c', 'o', 'o', 'l', '\0' };
serialize_string(stream, cool, sizeof(cool));

If we'd check that the length < buffer_size - 1,
we'd need to put the '\0' a byte earlier, like so:

char cool[5] = { 'c', 'o', 'o', '\0', '\0' };

Which necessarily wastes a single byte of our buffer.

@geneotech geneotech force-pushed the TeamHypersomnia:master branch from 96c3dcf to c285f2a Dec 18, 2018

@gafferongames gafferongames merged commit ce9573c into networkprotocol:master Dec 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment