Duplicate packets with reliable_unordered

[Ploppz]

In my game I noticed this bug where packets were sometimes received twice or even more. Right now I'm struggling with a packet getting repeated indefinitely and hope that this is related to this issue.

I managed to reproduce the bug in a minimal test case, attached. It only happens under certain conditions; I could only summon the bug when the communicating parts kept sending some other packets (unreliable and reliable_unordered). That is, I could only reproduce it when I added those additional packet streams.
The Join message is received twice at the server, whereas according to the guarantees of reliable_unordered, it should only be received once.

minimal_test_case.zip

[TimonPost]

I can confirm this, I did some research but can not directly find the golden key to figure out why this is happening. I can say the following:

1. After 36 loops the client, sents or the server just receives the other join packet. This will not be affected by sleep duration or whatsoever.

    for _ in 0..36 {
        client_update(&mut client, srv_addr);
        // std::thread::sleep(std::time::Duration::new(0, 8_000_000));
        server_update(&mut server, cli_addr);
        std::thread::sleep(std::time::Duration::from_millis(10));
    }

2. The problem is highly withing reliable unordered. When disabling the sending of a reliable unordered packet in the server update method, we won't see a double join even happening.

My guess it that it might stick around in the acknowledgment buffer @jstnlef ? I am able to test that in a later stage.

[kstrafe]

The acknowledgement buffer looks fine @TimonPost. @Ploppz the packet gets re-sent because the initial join message assumes (by being reliable) that the recipient already has the client added to its connection table. Because it does not, the packet's sequence number is not stored on the server. The server sends a message back to the client (thus adding it to the connection table), however, this message back doesn't contain the bitfield trigger from the initial message because the server was unable to remember it (because of the DoS protection).

Solution: Use unreliable as the first message, then reply with a reliable message.

[TimonPost]

I can 'acknowledge' that there is nothing wrong with the behavior of laminar in this case. The problem is that you first have to make sure that the client sents a packet to the server, whereafter the server sents a packet back to the client. When you do not do this, the server will never register the client as a connection. As @BourgondAries said, make sure you have an initial packet, and that the server reacts to that initial packet by sending a packet back to the client.

I don't like this approach very much and is a bit confusing, for now, we should definitely write it in our documentation somewhere because those are the issues a user of the library will not be able to just guess by looking at the API. I'll update my PR: #219 and merge that.

I'll close this issue for now, if you have more questions feel free to ask them.

[Ploppz]

Thank you both for the explanation.
I do think it would be nice if laminar could hide this from the user, but I don't know how much that would take.

[TimonPost]

Totaly true, this will be done with #156, which will is on my schedule.