Multiple Action calls when using OnEnable/OnDisable to subscribe/unsubscribe

Hi.

I’m working on a weapon system where the player can switch weapons. I do this by enabling/disabling the weapon gameobjects. But when I enable the previous selected weapon again the inputs are called multiple times instead of just one time.

I’ve looked though documentation and tutorials but none seem to have the same issue I’m having.

Unity version: 2019.3.0f3
InputSystem version: 1.0.0-preview.3

Here’s an example snippet of what I’m doing to check for inputs. This script sits on the weapon prefab:

using UnityEngine;
using UnityEngine.InputSystem;

public class Example : MonoBehaviour {
  private InputControls inputs;

  private virtual void Awake () {
    inputs = new InputControls();
  }

  private void OnEnable () {
    inputs.Weapon.Fire.Enable();
    inputs.Weapon.Fire.performed += context => OnInput(context);
  }

  private void OnDisable () {
    inputs.Weapon.Fire.Disable();
    inputs.Weapon.Fire.performed -= context => OnInput(context);
  }

  private void OnInput (InputAction.CallbackContext _context) {
    // ...
  }
}

I had a similar problem, it’s seams like the system doesn’t unsubscribe from these events,
like you do in inputs.Weapon.Fire.performed -= context => OnInput(context);
my work around was to flag the object (weapon in your case) active or not and then check that flag in the method attached to the event.

using UnityEngine;
using UnityEngine.InputSystem;
public class Example : MonoBehaviour {
  private InputControls inputs;
  private bool active; // <--------------- add a flag

  private virtual void Awake () {
    inputs = new InputControls();
  }
  private void OnEnable () {
    active = true;
    inputs.Weapon.Fire.Enable();
    inputs.Weapon.Fire.performed += context => OnInput(context);
  }
  private void OnDisable () {
    active = false;
    inputs.Weapon.Fire.Disable();
    inputs.Weapon.Fire.performed -= context => OnInput(context);
  }
  private void OnInput (InputAction.CallbackContext _context) {
     if(!active) return;  //<------------------ check if active, if not exit the function
     // ...
  }
}

Thanks for the reply NanushTol. Your example seem like a good alternative to unsubscribing.

I tried a couple of things over the days. I ended up moving the subscribe events (+=) to the Awake() and removing the unsubscribe events completely from OnDisable. So my OnEnable/OnDisable only have the .Enable()/.Disable() calls now. Seems to work alright, but I’d rather have the unsubscribe events actually work.

1 Like

I do agree that it should work, i don’t really like this work around, hope they will fix this bug

The reason here is that the lambas are not the same object. When you subscribe to the performed event, using a lamba, you’re creating an anonymous method. When you unsubscribe, you’re creating another, different anonymous method to unsubscribe. Since the method passed to the unsubscribe doesn’t match the original method (ie they are each unique anonymous methods - even though their code does the same thing), the original method stays in the delegate invocation list.

But as far as I can see, there’s no reason to use lambas here. OnInput matches the call signature of the performed event. Try subscribing and unsubscribing directly to the OnInput. eg

inputs.Weapon.Fire.performed += OnInput;
inputs.Weapon.Fire.performed -= OnInput;
1 Like