Skip to content

Add custom comparision support to NetworkList to better support #3587 #3657

Closed

Description

@Reag

Is your feature request related to a problem? Please describe.
We have several places in our code where we relied on the old behavior of NetworkLists from before this change. For example, we would query the status of a value based on a primary key in a more complex NetworkSerializable struct. As such, the changes in #3587 broke considerable portions of our update logic. While the changes to 3587 definitely makes sense for the average user, letting power users override this default logic would be a great addition.

Describe the solution you'd like
Ideally, we would like to have some way of passing in an optional equality check for determining if a value update causes a network event. For example, having it take some kind of optional IComparer to use for checking. This way, we could implement a 'bytewise' equality for set operations as needed.

Describe alternatives you've considered
There are two main alternatives we have considered. One is to simply revert our NGO version back a release, and keep it locked until we have time to do a refactor. The second one is to assign that entry in the list 'default' before reassigning it to its new index.

Activity

  1. added
    type:featureNew feature, request or improvement
    stat:awaiting-triageStatus - Awaiting triage from the Netcode team.
    on Sep 9, 2025
  2. EmandM commented on Sep 9, 2025

    @EmandM
    Member

    Hi, thanks for reporting this issue!

    The change we made in #3587 brings the NetworkList closer to the behaviour of the NetworkVariable collections. The intention of the IEquatable constraint on the NetworkList is to be able to avoid unnecessary updates being sent.

    To better understand your use-case, what is the constraint on changing your IEquatable implementation in this case?

  3. added
    stat:awaiting-responseAwaiting response from author. This label should be added manually.
    and removed on Sep 9, 2025
  4. self-assigned this
    on Sep 9, 2025
  5. added
    stat:InvestigatingIssue is currently being investigated
    and removed
    stat:awaiting-triageStatus - Awaiting triage from the Netcode team.
    on Sep 9, 2025
  6. Reag commented on Sep 10, 2025

    @Reag
    Author

    Basically, we have a generic layer system for displaying various pieces of info on our map. All of these layers inherit from the same base class, shown below:

    Code Snip
    public interface INetworkLayerData : INetworkSerializeByMemcpy, IEquatable<INetworkLayerData>, IEquatable<string>
    {
        public FixedString64Bytes Name { get; init; }
        public bool State { get; set; }
    }
    
    [AbsLog]
    [GenerateSerializationForGenericParameter(0)]
    [GenerateSerializationForGenericParameter(1)]
    public abstract class
        LayerServicesNetworkManagerBase<TManager, TStruct, TDefinition> : AbsorberNetworkSingleton<TManager>
        where TManager : SerializedNetworkBehaviour
        where TStruct : unmanaged, INetworkLayerData, IEquatable<TStruct>
        where TDefinition : ServiceDefinition
    {
        [NonSerialized] [ShowInInspector] [HideInEditorMode]
        private NetworkList<TStruct> _layerData = new();
    
        public event Action<NetworkListEvent<TStruct>>? LayerUpdated;
       ...
       ...
       ...
        /// <summary>
        /// Sets the internal network layer list value
        /// </summary>
        /// <param name="layerData"></param>
        private void SetLayerState_ServerOnly(TStruct layerData)
        {
            var state = GetLayerState(layerData.Name.Value);
            if (state == layerData.State)
            {
                // Nothing to update
                return;
            }
        
            int index = _layerData.IndexOf(new TStruct { Name = layerData.Name });
            if (index >= 0)
            {
                _layerData[index] = layerData;
            }
            ...
            ...
         }
    }
    

    The way we use this is by doing a '.Contains()' operation to check if a layer exists, while still listening to ValueChanged events for actually changing the state of the layer. This way, when we need to check if a user defined service exists, we can simply check via string, rather than having to do a much more difficult set of comparisons, while still getting the NetworkListEvents for changes to its state. Is this great code? No, not really. Its a bit of a hack that relied on inefficient code. However, the update broke it, leading to a case where we need to do a fairly large refactor for everything related to our user defined services.

    [Edit]: Some background: my team has been working with NGO since v1.0.0. As such, we have some very old code and some very old design patterns that are no longer best practices with the newer architecture. NetworkLists are already weird (They lack Enumerators for use with Linq or ForEach, along with weird restrictions that don't apply anywhere else in NGO), so we just accepted this was one of its quirks. For now, we're going to downgrade to 2.4.4 until we have the free time to refactor most of our network data systems.

  7. added and removed
    stat:awaiting-responseAwaiting response from author. This label should be added manually.
    on Sep 10, 2025
  8. EmandM commented on Sep 10, 2025

    @EmandM
    Member

    Thanks for the extra information!

    We absolutely can put back a way to do things the old way. We have a couple of options for doing this.

    We can add a public field to the NetworkList that'll ignore the equality check. This has the benefit of globally setting the behaviour and all other code can remain the same.

    var myState = new NetworkList<MyType>();
    myState.CheckEqualityForUpdate = false;

    Otherwise, we can add getter/setter functions alongside the public T this[int index] implementation and the setter can have an override. This has some optimization benefits as the C# compiler doesn't optimize functions inside properties very well. This option is also more inline with NetworkVariable collections.

    var myState = new NetworkList<MyType>();
    myState.Set(index, value, forceUpdate = true);

    Which option would work better for your project?

  9. added
    stat:awaiting-responseAwaiting response from author. This label should be added manually.
    and removed on Sep 10, 2025
  10. 6 remaining items

  11. added
    stat:importedStatus - Issue is tracked internally at Unity
    and removed
    stat:importStatus - Issue is going to be saved internally
    on Sep 15, 2025
  12. removed
    stat:awaiting-responseAwaiting response from author. This label should be added manually.
    stat:InvestigatingIssue is currently being investigated
    on Sep 15, 2025
  13. AliasgarBohra commented on Sep 19, 2025

    @AliasgarBohra

    Is this fixed in v2.5.1?

  14. EmandM commented on Sep 19, 2025

    @EmandM
    Member

    The fix is implemented in #3690 and should land in time for the next release. At this point I've only added the Set function. Let me know if a Get function would also be needed/welcome.

  15. added
    stat:awaiting-responseAwaiting response from author. This label should be added manually.
    and removed on Sep 19, 2025
  16. added a commit that references this issue on Sep 29, 2025
    d68c986
  17. EmandM commented on Oct 16, 2025

    @EmandM
    Member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

stat:importedStatus - Issue is tracked internally at Unitytype:featureNew feature, request or improvement

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions