feat: client network variables #1522
Conversation
mattwalsh-unity
requested review from
0xFA11,
andrews-unity and
SamuelBellomo
…echnologies/com.unity.multiplayer.mlapi into feature/client-network-variables
|
|
||
| public override bool ShouldWrite(ulong clientId, bool isServer) | ||
| { | ||
| return m_IsDirty && !isServer && m_NetworkBehaviour.IsOwner; |
There was a problem hiding this comment.
A host wouldn't be able to write? If my character is client driven, I don't want different code for my host and my clients, both should be able to update their own characters.
There was a problem hiding this comment.
Yeah, hosts should be able to write these, although I still yearn for the day when a "host" is just a client and a server running in the same process but each having their own version of each object.
| @@ -67,6 +67,16 @@ public virtual bool ShouldWrite(ulong clientId, bool isServer) | |||
| return IsDirty() && isServer && CanClientRead(clientId); | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Gets Whether or not a specific client can read to the varaible | |||
There was a problem hiding this comment.
| /// Gets Whether or not a specific client can read to the varaible | |
| /// Gets Whether or not a specific client can write to the variable |
| /// <summary> | ||
| /// A ClientNetworkVariable is special in that: | ||
| /// - only the owner of the variable can write to it | ||
| /// - not even the server can write to it |
There was a problem hiding this comment.
not even the server can write to it
I might have missed this with Owernship, but to me this would be covered by the IsOwner check no? IsOwner checks for
NetworkManager != null && OwnerClientId == NetworkManager.LocalClientId;
This would be false for the server?
| /// A ClientNetworkVariable is special in that: | ||
| /// - only the owner of the variable can write to it | ||
| /// - not even the server can write to it | ||
| /// - it is not snapshotted |
There was a problem hiding this comment.
I thought the direction we were taking was for everything to be snapshotted. AFAIK the client can send snapshots to the server as well.
| /// - it is not snapshotted | ||
| /// - it must be sent reliably | ||
| /// | ||
| /// (This class may be removed in the future when integrated into NetworkVariable natively) |
There was a problem hiding this comment.
Glad I read this comment, I was gonna ask this question.
|
|
||
| // Also, note this is not really very water-tight, if you are running as a host | ||
| // we cannot tell if a ClientNetworkVariable write is happening inside server-ish code | ||
| if (m_NetworkBehaviour && m_NetworkBehaviour.NetworkManager.IsServer) |
There was a problem hiding this comment.
This might be an opportunity to think about how we can decouple this from NetworkBehaviour and NetworkManager. We have NetworkBehaviour doing some initialization work on network variables, what about removing m_NetworkBehaviour and replacing it with internal void SetLocalPermissions(bool iCanRead, bool iCanWrite) and letting NetworkBehaviour tell NetworkVariable what its configuration should be, instead of having NetworkVariable poll for it the other way? This isn't going to change over the variable's lifetime and that would make network variables easier to unit test.
There was a problem hiding this comment.
probably also needs to pass OwnerClientId. Or possibly it could pass in delegates for "can write" and "can read" instead of booleans. That could give us the opportunity to give more advanced permission control to the user and let them write more complicated ownership and authority logic.
mattwalsh-unity
requested review from
becksebenius-unity
and removed request for
andrews-unity and
SamuelBellomo
|
superseded by #1762 |
This PR would re-introduce client network variables. With this feature, client nv's can be used to allow clients to communicate input to the server as an alternative to RPCs, which can be useful especially until we introduce a proper input passing system.
MTT-1933
RFC required, deferring until I get some feedback
PR Checklist
type:backport-release-*label. After you backport the PR, the label changes tostat:backported-release-*.CHANGELOG.mdfile.Changelog
com.unity.netcode.gameobjects
Testing and Documentation