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.15k
stars
435
forks
source link
fix: networkvariable collections can be modified without write permissions [backport] #3126
This PR resolves the issue where a client or server could modify a collection with no write permissions. The underlying issue is that default .NET collections have no way to be notified when the collection is modified. Since polling each instance is not an option, the lowest risk fix that continues to support standard .NET collections was to:
Add a 3rd internal duplicated current value property that can be used to compare against the current internal value reading (applying) a delta or field value. This helps to avoid edge case scenarios where the current collection value on a client with no write permissions could be modified and a state update is received.
Update NetworkVariable.CheckDirtyState and NetworkVariable.IsDirty such that the same form of comparison occurs to provide users with an additional way to check collection integrity on clients without write permissions while also being able to control how often the collection is checked.
This PR also resolves:
The issue where when one or more clients is/are already connected, entries are added by the current owner, the ownership changes to one of the connected clients, and finally the new client owner adds an entry. Since the new client's previous value always lags behind the last update, the new client will send both the new entry and the previously added entry which causes an exception on non-owner clients (duplicate key entry) and/or additional duplicate entries could be added (i.e. a list).
The issue where changing ownership would mark all NetworkVariables dirty as opposed to:
Marking any owner read permissions NetworkVariables as dirty when sending to the new owner (both within NetworkSpawnManager for server-side update and within the NetworkVariableDeltaMessage).
Sending any pending updates to all clients prior to sending the change in ownership message to assure pending updates are synchronized as the owner.
The issue where a newly synchronizing client would be synchronized with the current NetworkVariable values always which could cause issues with collections if there were any pending state updates. Now, when initially synchronizing a client, if a NetworkVariable has a pending state update it will serialize the previously known value(s) to the synchronizing client so when the pending updates are sent they don't duplicate values.
This PR also changes:
NetworkVariableDeltaMessage now includes the ability for a server to immediately forward delta state updates (to valid clients on a per NetworkVariable field basis) without having to queue them for end of frame (i.e. since messages are processed at the beginning of a frame, this shaves off the state replication time to other clients by close to the total frame time...which depending upon your project could mean 10-16ms faster).
Changelog
Changed: NetworkVariableDeltaMessage so the server now forwards delta state updates (owner write permission based from a client) to other clients immediately as opposed to keeping a NetworkVariable or NetworkList dirty and processing them at the end of the frame or potentially on the next network tick.
Fixed: Issue where a newly synchronizing client would be synchronized with the current NetworkVariable values always which could cause issues with collections if there were any pending state updates. Now, when initially synchronizing a client, if a NetworkVariable has a pending state update it will serialize the previously known value(s) to the synchronizing client so when the pending updates are sent they aren't duplicate values on the newly connected client side.
Fixed: Issue where changing ownership would mark every NetworkVariable dirty. Now, it will only mark any NetworkVariable with owner read permissions as dirty and will send/flush any pending updates to all clients prior to sending the change in ownership message.
Fixed: Issue with NetworkVariable collections where transferring ownership to another client would not update the new owner's previous value to the most current value which could cause the last/previous added value to be detected as a change when adding or removing an entry (as long as the entry removed was not the last/previously added value).
Fixed: Issue where a client (or server) with no write permissions for a NetworkVariable using a standard .NET collection type could still modify the collection which could cause various issues depending upon the modification and collection type.
Testing and Documentation
Includes integration test updates.
Some public documentation updates required (same modifications needed for #3081).
This is a backport of #3081.
This PR resolves the issue where a client or server could modify a collection with no write permissions. The underlying issue is that default .NET collections have no way to be notified when the collection is modified. Since polling each instance is not an option, the lowest risk fix that continues to support standard .NET collections was to:
NetworkVariable.CheckDirtyState
andNetworkVariable.IsDirty
such that the same form of comparison occurs to provide users with an additional way to check collection integrity on clients without write permissions while also being able to control how often the collection is checked.This PR also resolves:
This PR also changes:
NetworkVariableDeltaMessage
now includes the ability for a server to immediately forward delta state updates (to valid clients on a per NetworkVariable field basis) without having to queue them for end of frame (i.e. since messages are processed at the beginning of a frame, this shaves off the state replication time to other clients by close to the total frame time...which depending upon your project could mean 10-16ms faster).Changelog
NetworkVariableDeltaMessage
so the server now forwards delta state updates (owner write permission based from a client) to other clients immediately as opposed to keeping aNetworkVariable
orNetworkList
dirty and processing them at the end of the frame or potentially on the next network tick.NetworkVariable
values always which could cause issues with collections if there were any pending state updates. Now, when initially synchronizing a client, if aNetworkVariable
has a pending state update it will serialize the previously known value(s) to the synchronizing client so when the pending updates are sent they aren't duplicate values on the newly connected client side.NetworkVariable
dirty. Now, it will only mark anyNetworkVariable
with owner read permissions as dirty and will send/flush any pending updates to all clients prior to sending the change in ownership message.NetworkVariable
collections where transferring ownership to another client would not update the new owner's previous value to the most current value which could cause the last/previous added value to be detected as a change when adding or removing an entry (as long as the entry removed was not the last/previously added value).NetworkVariable
using a standard .NET collection type could still modify the collection which could cause various issues depending upon the modification and collection type.Testing and Documentation