Feeddback: NetworkVariable<T>.OnValueChanged delegate could use a NetworkBehavior as an argument.

Hello!

I've found myself in a situation where I have dozens of ScriptableObjects that define object behaviors that are added to and removed from NetworkBehavior based objects at runtime. These ScriptableObjects add and remove OnValueChanged event handlers to many of the underlying NetworkVariables, almost all of which need to know the object that sends the event. Here's an example:

Code (CSharp):

1.  
2. public class WorldObjectHealthConceptDefinition : BaseHealthConceptDefinition, IInitializableConcept
3.     {
4.         [SerializeField]
5.         private AnimationClip deathAnimation;
6.  
7.         public void OnAddedToObject(IConceptStateProvider stateProvider)
8.         {
9.             var state = stateProvider as WorldObjectConceptState;
10.             state.CurrentHealth.Value = MaxHealth;
11.             state.CurrentHealth.OnValueChanged += (oldValue, newValue) => { HandleOnObjectHealthChanged(state, oldValue, newValue); };
12.         }
13.  
14.         public void OnRemovedFromObject(IConceptStateProvider stateProvider)
15.         {
16.             var state = stateProvider as WorldObjectConceptState;
17.            
18.             // TODO: Pray this changes...
19.             //state.CurrentHealth.OnValueChanged -= (value, newValue) => { };
20.         }
21.  
22.         private void HandleOnObjectHealthChanged(WorldObjectConceptState state, int oldValue, int newValue)
23.         {
24.             if (NetworkManager.Singleton.IsClient && oldValue > 0 && newValue == 0)
25.             {
26.                 var animState = state.Animator.Play(deathAnimation);
27.                 animState.Events.OnEnd = () => animState.IsPlaying = false;
28.             }
29.         }
30.     }
31.  

Now, I don't currently remove the "health" concept from objects at runtime (which is why this code still exists), but this should illustrate the problem. The anonymous lambda that I'm using can't be removed when the HealthConcept is removed from an object.

My workaround has been to have a ton of boilerplate in WorldObjectConceptState that subscribes to the OnValueChanged delegate and invoke a separate delegate, passing the state with the changed value as the first parameter.

It would be lovely if either of two things could happen:

1. Make NetworkVariable<T>.OnValueChanged pass the NetworkVariableBase.m_NetworkBehaviour as a parameter or
2. Ideally, expose NetworkVariableBase.m_NetworkBehaviour via a public property and pass the network variable that changes to OnValueChanged.

This might seem a bit pedantic, but I found myself doing the same pattern all over the place in this project. Unreal has the same issue with the ReplicatedUsing attribute, and every project I've worked on has had the same pattern of declaring a new delegate that looks like: (object, oldValue, newValue) and forwarding the OnChanged events to external objects that way. Makes for a lot of unnecessary bloat, especially when Unity has all the data necessary right there in NetworkVariableBase.

I'm not sure if anybody out there would agree with this feedback, but having the sender object in an event handler function is very common and is the recommended guideline from Microsoft: https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/event

My $0.02, hope this helps!

 

Hey that looks like a pretty good suggestion which we could add to OnValueChanged. Could you please also post this as a feature request issue on Github so that it doesn't get lost to the team? https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues/new/choose