C#•4w ago

✅ Class instance reference

I'm having some issues figuring out why my code is behaving like it's taking a "snapshot" of an instance instead of directly referencing it. So I created a custom attribute so that I could check a class for methods with it and add the methods to the list. I had to "register" my methods after creating the instance so that they could access the instance members, and so in the class constructor I added:

     public UdpConnection()
    {
      MessageHandler.RegisterListeners(typeof(UdpConnection), this);
    }

     public UdpConnection()
    {
      MessageHandler.RegisterListeners(typeof(UdpConnection), this);
    }

For context, this is how a listener looks:

 [MessageListener(ServerPackets.S_UdpPort)]
    private void AssignLocalPort(S_UdpPortPayload payload)
    {
      #region AssignLocalPort
     // Code here
      #endregion
    }

 [MessageListener(ServerPackets.S_UdpPort)]
    private void AssignLocalPort(S_UdpPortPayload payload)
    {
      #region AssignLocalPort
     // Code here
      #endregion
    }

In my MessageHandler class I have this dictionary:

    public static Dictionary<ServerPackets, MessageListener<IPayload>> messageListeners = new();

    public static Dictionary<ServerPackets, MessageListener<IPayload>> messageListeners = new();

36 Replies

AdiseOP•4w ago

This is the registration code:

 private static void RegisterMessageListeners(Type target, object? instance)
    {
      var methods = target.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static);

      foreach (var method in methods)
      {
        MessageListenerAttribute foundAttribute = method.GetCustomAttribute<MessageListenerAttribute>();
        if (foundAttribute == null) continue;

        ServerPackets packetId = foundAttribute.packetId;

        try
        {
          var parameters = method.GetParameters();
          if (parameters.Length != 1 || !typeof(IPayload).IsAssignableFrom(parameters[0].ParameterType))
          {
            throw new InvalidOperationException(
                $"Method {method.Name} must have exactly one parameter implementing IPayload");
          }

          var payloadType = parameters[0].ParameterType;
          var delegateType = typeof(MessageListener<>).MakeGenericType(payloadType);

          var listener = Delegate.CreateDelegate(delegateType, instance, method);
          MessageListener<IPayload> wrapper = payload =>
             {
               if (payload is not null && payload.GetType() == payloadType)
               {
                 // Use reflection to invoke the strongly-typed delegate
                 listener.DynamicInvoke(payload);
               }
               else
               {
                 Debug.LogError($"Payload type mismatch for {packetId}. " +
                               $"Expected {payloadType}, got {payload?.GetType()}");
               }
             };

          lock (_messageListenerslock)
          {
            if (messageListeners.TryGetValue(packetId, out _)) continue;

            messageListeners.Add(packetId, wrapper);
          }

        }
        catch (Exception exception)
        {
          Debug.LogError($"Failed to register message listener {method.Name}: {exception.Message}");
       }
   }
}

 private static void RegisterMessageListeners(Type target, object? instance)
    {
      var methods = target.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static);

      foreach (var method in methods)
      {
        MessageListenerAttribute foundAttribute = method.GetCustomAttribute<MessageListenerAttribute>();
        if (foundAttribute == null) continue;

        ServerPackets packetId = foundAttribute.packetId;

        try
        {
          var parameters = method.GetParameters();
          if (parameters.Length != 1 || !typeof(IPayload).IsAssignableFrom(parameters[0].ParameterType))
          {
            throw new InvalidOperationException(
                $"Method {method.Name} must have exactly one parameter implementing IPayload");
          }

          var payloadType = parameters[0].ParameterType;
          var delegateType = typeof(MessageListener<>).MakeGenericType(payloadType);

          var listener = Delegate.CreateDelegate(delegateType, instance, method);
          MessageListener<IPayload> wrapper = payload =>
             {
               if (payload is not null && payload.GetType() == payloadType)
               {
                 // Use reflection to invoke the strongly-typed delegate
                 listener.DynamicInvoke(payload);
               }
               else
               {
                 Debug.LogError($"Payload type mismatch for {packetId}. " +
                               $"Expected {payloadType}, got {payload?.GetType()}");
               }
             };

          lock (_messageListenerslock)
          {
            if (messageListeners.TryGetValue(packetId, out _)) continue;

            messageListeners.Add(packetId, wrapper);
          }

        }
        catch (Exception exception)
        {
          Debug.LogError($"Failed to register message listener {method.Name}: {exception.Message}");
       }
   }
}

Finally, when I call the stored listener:

 private static void RouteMessage(ServerPackets packetId, IPayload payload)
    {
      #region RouteMessage
      Debug.Log($"Routing message {packetId}");
      lock (_messageListenerslock)
      {

        bool isListenerFound = messageListeners.TryGetValue(packetId, out var listener);
        Debug.Log($"Listener found:  {isListenerFound}");
        if (!isListenerFound) return;

        listener.DynamicInvoke(payload);
      }
      #endregion
    }

 private static void RouteMessage(ServerPackets packetId, IPayload payload)
    {
      #region RouteMessage
      Debug.Log($"Routing message {packetId}");
      lock (_messageListenerslock)
      {

        bool isListenerFound = messageListeners.TryGetValue(packetId, out var listener);
        Debug.Log($"Listener found:  {isListenerFound}");
        if (!isListenerFound) return;

        listener.DynamicInvoke(payload);
      }
      #endregion
    }

The listener gets called and the correct payload is passed and everything, but it seems like it can't access the instance members (in this case the UdpConnection class instance members). If Instead of registering the handler in the constructor I call it later, the class members from that moment will be accessible when the listener is called. Is there a way to fix this so that the instance is stored as a reference and not this "snapshot" behavior here?

mtreit•4w ago

I read this multiple times and don't quite understand what you are saying the problem is. Is UdpConnection a struct?

bighugemassive3•4w ago

What do you mean by can't access instance members? In the handler callback?

AdiseOP•4w ago

No so, the TL,DR is: I have a class with a method that has the [MessageListener] attribute. I then Register (Store) the methods that have this attribute on a dictionary to be called when an event happens. When this even fires, the methods ARE called, but for some reason they "exist" in another class instance, it's like when I store the created delegate, even if I'm passing the instance it creates a new one. Example flow: - Instanciate class - Set class member foo = "bar" - Register listener - Set class member foo = "changed" - Event gets called - Listener gets triggered - In the listener, if I access the class member foo the value will be bar instead of changed. You can see however, that in the RegisterListeners function I am not creating a new instance at any point, so I don't understand why this is happening. Am I just not using the Delegate.CreateDelegate(delegateType, instance, method); correctly? As u can see I am passing it the instance of the class, so when I call the created delegate it should exist in the same instance no?

Mayor McCheese•4w ago

What are you expecting to happen?

bighugemassive3•4w ago

UdpConnection is a class right not struct?

AdiseOP•4w ago

For the listner to exist in the same instance, so that even if values are changed after registering it, it will reside in the instance scope and so have access to them. In the example above, I'd expect foo to be changed when accesing it though the listener Yep

mtreit•4w ago

What you are describing only makes sense if it's a struct. Unless I'm missing something. There is no "snapshot" behavior for reference types.

AdiseOP•4w ago

Nope, it does not make sense, that's why I'm here haha I've been battling this all day And I can see, through debugger that this when executing the listener does not have the updated members

mtreit•4w ago

Then the reference you are using is to change the value of the member must be pointing to a different instance.

bighugemassive3•4w ago

I would double check your test code, you might be doing something wrong in there instead

Mayor McCheese•4w ago

Could you have an unexptected closure?

AdiseOP•4w ago

I'm not even testing yet, I was just writing it, I don't like TDD

mtreit•4w ago

Variable capture in a closure could be an issue, yeah.

bighugemassive3•4w ago

But how do you know there's even an issue if you didn't test it??

Mayor McCheese•4w ago

that would be snapshot-ish

AdiseOP•4w ago

the Udpconnection class does inherit from IDisposable, but I did check and the instance is not disposed if that's what you are refering to

Mayor McCheese•4w ago

AdiseOP•4w ago

Run the code -> Code don't work xdd

bighugemassive3•4w ago

That's what i meant by test

Mayor McCheese•4w ago

a closure captures scope, if it's intended that's great, if it's not...

bighugemassive3•4w ago

Put a breakpoint on the UdpConnection constructor and see if another instance is created

AdiseOP•4w ago

Ah, I thought u meant literal test code, since u suggested I checked my test code trying now

mtreit•4w ago

I think you are either doing something wrong or completely mis-diagnosing the issue because changing the value of a member of a reference type will absolutely update the instance being referred to 🙂 Unless you have some struct copy thing happening. Like a classic bug is this:

public struct Point
{
    public int X { get; set; }
    public int Y { get; set; }
}

public class Widget
{
    public Point Location { get; set; }
}

public struct Point
{
    public int X { get; set; }
    public int Y { get; set; }
}

public class Widget
{
    public Point Location { get; set; }
}

and then you have code like this:

var w = new Widget();
w.Location.X = 42;

var w = new Widget();
w.Location.X = 42;

...and it doesn't get "updated" because of copy semantics with value types.

Mayor McCheese•4w ago

I mean based on the code I'm seeing you have a private static method that "registers types", and it's adding to a private static list/dictionary, and in that scenario, when are you calling register handlers? I could be reading it wrong, but I'm sorta wondering how'd you'd expect something different than what is happening.

AdiseOP•4w ago

I'm invoking the listeners when an even happens:

  public static void HandleTcpPacket(ServerPackets packetId, Packet packet)
    {
      #region HandleTcpPacket
      NetworkManager.Singleton.FireOnPacket(packetId, packet);


      bool foundReader = packetReaders.TryGetValue(packetId, out var reader);

      if (!foundReader)
      {
        Debug.LogError($"Reader not found for packet {packetId}");
        return;
      }


      IPayload payload = (IPayload)reader.DynamicInvoke(packet);

      NetworkManager.Singleton.FireOnTcpMessage(packetId, payload);
      NetworkManager.Singleton.FireOnMessage(packetId, payload);

      if (packet.messageId == 0) return;
      HandleAwaitedResponse(packet.messageId, payload);
      #endregion
    }

  public static void HandleTcpPacket(ServerPackets packetId, Packet packet)
    {
      #region HandleTcpPacket
      NetworkManager.Singleton.FireOnPacket(packetId, packet);


      bool foundReader = packetReaders.TryGetValue(packetId, out var reader);

      if (!foundReader)
      {
        Debug.LogError($"Reader not found for packet {packetId}");
        return;
      }


      IPayload payload = (IPayload)reader.DynamicInvoke(packet);

      NetworkManager.Singleton.FireOnTcpMessage(packetId, payload);
      NetworkManager.Singleton.FireOnMessage(packetId, payload);

      if (packet.messageId == 0) return;
      HandleAwaitedResponse(packet.messageId, payload);
      #endregion
    }

But that is not the issue since the listeners do get correctly called

Mayor McCheese•4w ago

lock (_messageListenerslock)
          {
            if (messageListeners.TryGetValue(packetId, out _)) continue;

            messageListeners.Add(packetId, wrapper);
          }

lock (_messageListenerslock)
          {
            if (messageListeners.TryGetValue(packetId, out _)) continue;

            messageListeners.Add(packetId, wrapper);
          }

wrapper is sitting in a static dictionary each listener isn't going to be recreated every usage. oic what you're saying I thought you had an activator for some reason

AdiseOP•4w ago

Yeah that's something I'll change later on, but as of now this is by design. I do think I may have find it tho. As @bighugemassive3 suggested I set a breakpoint in the constructor and I do think it's being called more than once, leaving an outdated reference in the dictionary (With it not accepting more listerners per packetId). I'm creating the instance directly in a class member: UdpConnection connection = new UdpConnection() This code is being run by unity and from when I can see in the debugger it looks like it creates the instance when it compiles, but then it does it again when entering playMode which I was not aware was a thing.

Mayor McCheese•4w ago

MessageListener<IPayload> wrapper = payload =>
             {
               if (payload is not null && payload.GetType() == payloadType)
               {
                 // Use reflection to invoke the strongly-typed delegate
                 listener.DynamicInvoke(payload);
               }
               else
               {
                 Debug.LogError($"Payload type mismatch for {packetId}. " +
                               $"Expected {payloadType}, got {payload?.GetType()}");
               }
             };

MessageListener<IPayload> wrapper = payload =>
             {
               if (payload is not null && payload.GetType() == payloadType)
               {
                 // Use reflection to invoke the strongly-typed delegate
                 listener.DynamicInvoke(payload);
               }
               else
               {
                 Debug.LogError($"Payload type mismatch for {packetId}. " +
                               $"Expected {payloadType}, got {payload?.GetType()}");
               }
             };

@mtreit that's a closure problem with listener innit?

AdiseOP•4w ago

I still gota do a propper pass for correctly closing and disposing of everything aswell

MODiX•4w ago

Mayor McCheese

REPL Result: Success

var actions = new List<Action>();

for (int i = 0; i < 3; i++)
{
    actions.Add(() => Console.WriteLine(i));
}

foreach (var action in actions)
{
    action();
}

var actions = new List<Action>();

for (int i = 0; i < 3; i++)
{
    actions.Add(() => Console.WriteLine(i));
}

foreach (var action in actions)
{
    action();
}

Console Output

3
3
3

3
3
3

Compile: 423.742ms | Execution: 117.371ms | React with ❌ to remove this embed.

Mayor McCheese•4w ago

you're doing the same thing

MODiX•4w ago

Mayor McCheese

REPL Result: Success

var actions = new List<Action>();

for (int i = 0; i < 3; i++)
{
     var listenerInstance = i;
    actions.Add(() => Console.WriteLine(listenerInstance));
}

foreach (var action in actions)
{
    action();
}

var actions = new List<Action>();

for (int i = 0; i < 3; i++)
{
     var listenerInstance = i;
    actions.Add(() => Console.WriteLine(listenerInstance));
}

foreach (var action in actions)
{
    action();
}

Console Output

0
1
2

0
1
2

Compile: 471.284ms | Execution: 90.481ms | React with ❌ to remove this embed.

Mayor McCheese•4w ago

in your lambda it's looking like you're not capturing the scope, but I could be wrong

AdiseOP•4w ago

Yep, definetly this. I've now made sure to initialize the parent class on unity's start method instead of directly and works as expected. I think the root of the problem is that the Udp class is a member of a client class that is a member of a monoBehaviour, and it trigered when compiled but it re-triggered on entering play, resulting in the old listener to ocupy the packetId spot on the dictionary As always, it turns out to be the stupidest crap Thank yo so much everyone for helping me out! !solved

Mayor McCheese•4w ago

/close

Gaming

Programming

✅ Class instance reference

Did you find this page helpful?