feat!: remove client network permissions [MTT-1019] #1051
Conversation
mattwalsh-unity
force-pushed
the
feature/container_cleanup
branch
2 times, most recently
from
c3ad039 to
aeb320c
Compare
mattwalsh-unity
changed the title
com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariablePermission.cs
Show resolved
Hide resolved
| NetworkVariableFields[k] | ||
| .IsDirty(); // cache this here. You never know what operations users will do in the dirty methods | ||
|
|
||
| // if I'm dirty AND a client, write (server always has all permissions) |
There was a problem hiding this comment.
Is this comment still valid?
| } | ||
|
|
||
| if (networkManager.IsServer && !networkVariableList[i].CanClientWrite(clientId)) | ||
| if (networkManager.IsServer)// ?? && !networkVariableList[i].CanClientWrite(clientId)) |
There was a problem hiding this comment.
Is this even needed anymore? because essentially its checking to see if the client can actually write to this value and if not log it... I am not sure this would be possible anymore correct?
|
So one thing that I think is overlooked with the way things are now is that because of the changed of Collections to using unmanaged and the underlying container hasn't changed, and the access APIs haven't changed are we okay with the large amount of copying that will now happen? like before NetworkList[0] could be a copy but now its always a copy, NetworkList.Contains is always going to be a copy. Not that its an issue but I think its something to callout that the semantics of the collections have changed some. For example if you previously put strings or even bools in a NetworkSet that is not supported now since neither is a blittable type. So I think it would be worth wild making sure that we add to the changelog what the implications of this change is with regards to collections and how they work. |
mattwalsh-unity
force-pushed
the
feature/container_cleanup
branch
from
e405abd to
a304dcc
Compare
|
Ok, though my journey of cleanup / iteration my other PR has now become the real one. #1074 |
No description provided.