search

lidgren / lidgren-network-gen3

Lidgren Network Library
https://groups.google.com/forum/#!forum/lidgren-network-gen3
MIT License
1.19k stars 331 forks source link

Specifying a channel for non-sequenced Unreliable command will make it UnreliableSequenced #87

Open aienabled opened 7 years ago

aienabled commented 7 years ago

Hello, in RELEASE build there is no assert that non-zero channel must be used for Unreliable command (there is such assert in DEBUG build). It passes this assert and then something bad (IMHO) happens - the channel number added to this enum entry at NetConnection.EnqueueMessage() and it becomes UnreliableSequenced. For me, it was a surprising behavior (I found it out only now, three years after release of my product! And it was in critical part of code which expects to work as Unreliable, not as UnreliableSequenced).

I would prefer if in RELEASE build there will be this code:

if (method == NetDeliveryMethod.Unreliable)
{
    sequenceChannel = 0;
}

in both the NetPeer.SendMessage() methods. (throwing exception in DEBUG build is ok).

Also I found quite misleading this statement:

Duplicate packets detected and dropped for all delivery methods (even unreliable)

(from https://github.com/lidgren/lidgren-network-gen3/wiki/Features ) I checked the code and it seems there are no duplicate detection for completely Unreliable messages, only for UnreliableSequenced. Will you update the wiki or code accordingly?

aienabled commented 7 years ago

There also no check if the sequence channel number exceeds that from NetConstants.NetChannelsPerDeliveryMethod in NetPeer.SendMessage() for multiple recipients (only for the method with single recipient it's done).