noxworld-dev / opennox

OpenNox main repository.
GNU General Public License v3.0
440 stars 23 forks source link

Fix packet reading routine (increase buffer size) #699

Closed angrykirc closed 1 month ago

angrykirc commented 1 month ago

Addresses issue #693

Required sign-off

angrykirc commented 1 month ago

@angrykirc Thanks a lot for tracking this down! 🚀

But I must admit I do not understand why it works. Is the old buffer too small?

Ideally, we should avoid buffer allocation in net layer. So if there's an alternative that will still pass destination buffer as argument, that would be preferable.

Yep, the old buffer was just 1024 bytes, that's 1022 bytes from the error message, minus the packet 'header'. PacketConn.ReadFrom limits amount of data it reads depending on the given slice's length, and simply discards everything afterwards, that is the root cause. And the more players on the server, the bigger one of "join game" packets is. About the buffer allocations, I'll look into that. I get it now why you chose to pass the buffer by argument. :)

angrykirc commented 1 month ago

When joining a server with 10 players, the size of the join packet is measured at 2050 bytes. And for a server with 1 player, it's 825 bytes. So if the increase is roughly 136 bytes per player, that gives 1097 bytes for 3 players and 5041 bytes for 32 players. That is the upper cap of OpenNox engine, for now. Based on these estimations, I think it is possible to safely reduce buffer size to 8192 bytes. But better to be safe than sorry... and take double of that.