Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.1k stars 430 forks source link

fix: Update API docs for MaxPayloadSize #2877

Closed jabbacakes closed 2 months ago

jabbacakes commented 2 months ago

Adding small remark to MaxPayloadSize to clarify following questions in this issue on the docs repo.

Testing and Documentation

unity-cla-assistant commented 2 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

simon-lemay-unity commented 2 months ago

I'm not sure the term "session" is ideal here. The payload size is allocated once per connection. So on a server with multiple connections at the same time, that means the memory will be allocated multiple times. The term "session" can fit that description, but it could also be understood as "the entire game session, including all connections" which could lead to confusion.

Also I think the key piece of information to add to the documentation about this parameter is that nowadays it's only used for unreliable messages. Given that NGO mostly uses reliable traffic, it means that this setting rarely needs to be modified. It's not something most users should have to fiddle with.

jabbacakes commented 2 months ago

I'm not sure the term "session" is ideal here. The payload size is allocated once per connection. So on a server with multiple connections at the same time, that means the memory will be allocated multiple times. The term "session" can fit that description, but it could also be understood as "the entire game session, including all connections" which could lead to confusion.

This is a good distinction to make. How about changing the remark to : The memory for MaxPayloadSize is allocated once per connection and is released when the connection is closed. I think that's clearer.

Also I think the key piece of information to add to the documentation about this parameter is that nowadays it's only used for unreliable messages. Given that NGO mostly uses reliable traffic, it means that this setting rarely needs to be modified. It's not something most users should have to fiddle with.

We already mention that it's unreliable in the API summary. I agree that this is a small thing that most users won't be interacting with, just wanted to add an additional remark to clear up an existing user query.