Skip to content

feat: owner-writable NetworkVariable<T> #1762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Mar 17, 2022
Merged

feat: owner-writable NetworkVariable<T> #1762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d174d40
`enum NetworkVariableWritePermission`
0xFA11 Mar 12, 2022
d874f80
Merge branch 'develop' into feat/ownernetworkvariable
0xFA11 Mar 13, 2022
cf748af
`clientId` -> `targetClientId`
0xFA11 Mar 13, 2022
de83e62
`OwnerOnly` -> `Owner`
0xFA11 Mar 13, 2022
e863de6
mvp
0xFA11 Mar 14, 2022
a17eac1
Merge branch 'develop' into feat/ownernetworkvariable
0xFA11 Mar 14, 2022
d797a24
minor refactor
0xFA11 Mar 14, 2022
7b3590f
minor fixes
0xFA11 Mar 14, 2022
46eb120
Merge branch 'develop' into feat/ownernetworkvariable
0xFA11 Mar 16, 2022
b248189
syntax fix
0xFA11 Mar 16, 2022
ea4f2da
bug fix
0xFA11 Mar 16, 2022
9eabf22
implement `NetworkVariablePermissionTests`
0xFA11 Mar 16, 2022
a5542b9
Merge branch 'develop' into feat/ownernetworkvariable
0xFA11 Mar 16, 2022
770c176
Merge branch 'develop' into feat/ownernetworkvariable
0xFA11 Mar 17, 2022
9726583
delete ctor tests
0xFA11 Mar 17, 2022
533dd6a
update changelog
0xFA11 Mar 17, 2022
b1c2b64
minor changelog update
0xFA11 Mar 17, 2022
a4fc4bc
fix `NetworkVariableTestComponent`
0xFA11 Mar 17, 2022
a646f46
add PR number to the changelog entry
0xFA11 Mar 17, 2022
aaffe62
assert on newvalue change
0xFA11 Mar 17, 2022
6327e3c
Merge branch 'develop' into feat/ownernetworkvariable
0xFA11 Mar 17, 2022
07afa2d
implement `NetworkVariablePermissionTests.ServerCannotChangeOwnerWrit…
0xFA11 Mar 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Added

- Added `NetworkVariableWritePermission` to `NetworkVariableBase` and implemented `Owner` client writable netvars. (#1762)

### Changed

### Fixed

- Fixed user never being notified in the editor that a NetworkBehaviour requires a NetworkObject to function properly. (#1808)
- Fixed PlayerObjects and dynamically spawned NetworkObjects not being added to the NetworkClient's OwnedObjects (#1801)
- Fixed issue where NetworkManager would continue starting even if the NetworkTransport selected failed. (#1780)
Expand Down
34 changes: 17 additions & 17 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth

if (clientRpcParams.Send.TargetClientIds != null)
{
foreach (var clientId in clientRpcParams.Send.TargetClientIds)
foreach (var targetClientId in clientRpcParams.Send.TargetClientIds)
{
if (clientId == NetworkManager.ServerClientId)
if (targetClientId == NetworkManager.ServerClientId)
{
shouldSendToHost = true;
break;
Expand All @@ -174,9 +174,9 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
}
else if (clientRpcParams.Send.TargetClientIdsNativeArray != null)
{
foreach (var clientId in clientRpcParams.Send.TargetClientIdsNativeArray)
foreach (var targetClientId in clientRpcParams.Send.TargetClientIdsNativeArray)
{
if (clientId == NetworkManager.ServerClientId)
if (targetClientId == NetworkManager.ServerClientId)
{
shouldSendToHost = true;
break;
Expand Down Expand Up @@ -493,12 +493,10 @@ internal void InitializeVariables()

m_VarInit = true;

FieldInfo[] sortedFields = GetFieldInfoForType(GetType());

var sortedFields = GetFieldInfoForType(GetType());
for (int i = 0; i < sortedFields.Length; i++)
{
Type fieldType = sortedFields[i].FieldType;

var fieldType = sortedFields[i].FieldType;
if (fieldType.IsSubclassOf(typeof(NetworkVariableBase)))
{
var instance = (NetworkVariableBase)sortedFields[i].GetValue(this);
Expand Down Expand Up @@ -559,21 +557,21 @@ internal void PostNetworkVariableWrite()
}
}

internal void VariableUpdate(ulong clientId)
internal void VariableUpdate(ulong targetClientId)
{
if (!m_VarInit)
{
InitializeVariables();
}

PreNetworkVariableWrite();
NetworkVariableUpdate(clientId, NetworkBehaviourId);
NetworkVariableUpdate(targetClientId, NetworkBehaviourId);
}

internal readonly List<int> NetworkVariableIndexesToReset = new List<int>();
internal readonly HashSet<int> NetworkVariableIndexesToResetSet = new HashSet<int>();

private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
private void NetworkVariableUpdate(ulong targetClientId, int behaviourIndex)
{
if (!CouldHaveDirtyNetworkVariables())
{
Expand All @@ -595,9 +593,11 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
var shouldSend = false;
for (int k = 0; k < NetworkVariableFields.Count; k++)
{
if (NetworkVariableFields[k].ShouldWrite(clientId, IsServer))
var networkVariable = NetworkVariableFields[k];
if (networkVariable.IsDirty() && networkVariable.CanClientRead(targetClientId))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the rationale for copying the code from ShouldWrite and editing it here, as opposed to improving/adjusting/modifying ShouldWrite? Just wondering what will make the code more easily maintainable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because there is no ShouldWrite anymore. there are CanClientRead and CanClientWrite for perm checks, whether or not to write (send) is determined outside, here.

{
shouldSend = true;
break;
}
}

Expand All @@ -608,15 +608,15 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
NetworkObjectId = NetworkObjectId,
NetworkBehaviourIndex = NetworkObject.GetNetworkBehaviourOrderIndex(this),
NetworkBehaviour = this,
ClientId = clientId,
TargetClientId = targetClientId,
DeliveryMappedNetworkVariableIndex = m_DeliveryMappedNetworkVariableIndices[j]
};
// TODO: Serialization is where the IsDirty flag gets changed.
// Messages don't get sent from the server to itself, so if we're host and sending to ourselves,
// we still have to actually serialize the message even though we're not sending it, otherwise
// the dirty flag doesn't change properly. These two pieces should be decoupled at some point
// so we don't have to do this serialization work if we're not going to use the result.
if (IsServer && clientId == NetworkManager.ServerClientId)
if (IsServer && targetClientId == NetworkManager.ServerClientId)
{
var tmpWriter = new FastBufferWriter(MessagingSystem.NON_FRAGMENTED_MESSAGE_MAX_SIZE, Allocator.Temp, MessagingSystem.FRAGMENTED_MESSAGE_MAX_SIZE);
using (tmpWriter)
Expand All @@ -626,7 +626,7 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
}
else
{
NetworkManager.SendMessage(ref message, m_DeliveryTypesForNetworkVariableGroups[j], clientId);
NetworkManager.SendMessage(ref message, m_DeliveryTypesForNetworkVariableGroups[j], targetClientId);
}
}
}
Expand Down Expand Up @@ -655,7 +655,7 @@ internal void MarkVariablesDirty()
}
}

internal void WriteNetworkVariableData(FastBufferWriter writer, ulong clientId)
internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClientId)
{
if (NetworkVariableFields.Count == 0)
{
Expand All @@ -664,7 +664,7 @@ internal void WriteNetworkVariableData(FastBufferWriter writer, ulong clientId)

for (int j = 0; j < NetworkVariableFields.Count; j++)
{
bool canClientRead = NetworkVariableFields[j].CanClientRead(clientId);
bool canClientRead = NetworkVariableFields[j].CanClientRead(targetClientId);

if (canClientRead)
{
Expand Down
35 changes: 15 additions & 20 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ internal void GenerateGlobalObjectIdHash()
/// </summary>
internal NetworkManager NetworkManagerOwner;

private ulong m_NetworkObjectId;

/// <summary>
/// Gets the unique Id of this object that is synced across the network
/// </summary>
Expand Down Expand Up @@ -288,7 +286,6 @@ public void NetworkHide(ulong clientId)
throw new VisibilityChangeException("Cannot hide an object from the server");
}


Observers.Remove(clientId);

if (NetworkManager.NetworkConfig.UseSnapshotSpawn)
Expand Down Expand Up @@ -319,7 +316,7 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
throw new ArgumentNullException("At least one " + nameof(NetworkObject) + " has to be provided");
}

NetworkManager networkManager = networkObjects[0].NetworkManager;
var networkManager = networkObjects[0].NetworkManager;

if (!networkManager.IsServer)
{
Expand Down Expand Up @@ -364,27 +361,27 @@ private void OnDestroy()
throw new NotServerException($"Destroy a spawned {nameof(NetworkObject)} on a non-host client is not valid. Call {nameof(Destroy)} or {nameof(Despawn)} on the server/host instead.");
}

if (NetworkManager != null && NetworkManager.SpawnManager != null && NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out var networkObject))
if (NetworkManager != null && NetworkManager.SpawnManager != null &&
NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out var networkObject))
{
NetworkManager.SpawnManager.OnDespawnObject(networkObject, false);
}
}

private SnapshotDespawnCommand GetDespawnCommand()
{
var command = new SnapshotDespawnCommand();
command.NetworkObjectId = NetworkObjectId;

return command;
return new SnapshotDespawnCommand { NetworkObjectId = NetworkObjectId };
}

private SnapshotSpawnCommand GetSpawnCommand()
{
var command = new SnapshotSpawnCommand();
command.NetworkObjectId = NetworkObjectId;
command.OwnerClientId = OwnerClientId;
command.IsPlayerObject = IsPlayerObject;
command.IsSceneObject = (IsSceneObject == null) || IsSceneObject.Value;
var command = new SnapshotSpawnCommand
{
NetworkObjectId = NetworkObjectId,
OwnerClientId = OwnerClientId,
IsPlayerObject = IsPlayerObject,
IsSceneObject = (IsSceneObject == null) || IsSceneObject.Value
};

ulong? parent = NetworkManager.SpawnManager.GetSpawnParentId(this);
if (parent != null)
Expand Down Expand Up @@ -415,8 +412,7 @@ private void SnapshotSpawn()
private void SnapshotSpawn(ulong clientId)
{
var command = GetSpawnCommand();
command.TargetClientIds = new List<ulong>();
command.TargetClientIds.Add(clientId);
command.TargetClientIds = new List<ulong> { clientId };
NetworkManager.SnapshotSystem.Spawn(command);
}

Expand All @@ -429,8 +425,7 @@ internal void SnapshotDespawn()
internal void SnapshotDespawn(ulong clientId)
{
var command = GetDespawnCommand();
command.TargetClientIds = new List<ulong>();
command.TargetClientIds.Add(clientId);
command.TargetClientIds = new List<ulong> { clientId };
Copy link
Contributor

@jeffreyrainy jeffreyrainy Mar 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, yeah, those are great changes, but they make the PR harder to process and possibly harder to merge later. We should try to have PRs that address just one thing and side clean-up PRs are fine too :-). < /nitpick> < /jeffshouldgethiscoffee>

NetworkManager.SnapshotSystem.Despawn(command);
}

Expand Down Expand Up @@ -821,13 +816,13 @@ internal List<NetworkBehaviour> ChildNetworkBehaviours
}
}

internal void WriteNetworkVariableData(FastBufferWriter writer, ulong clientId)
internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClientId)
{
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
var behavior = ChildNetworkBehaviours[i];
behavior.InitializeVariables();
behavior.WriteNetworkVariableData(writer, clientId);
behavior.WriteNetworkVariableData(writer, targetClientId);
}
}

Expand Down
Loading