Unity Input System not Deactivating

[uncut frost]

I am trying to deactivate jumping (for the sake of testing) when the player interacts with an object. However, jumping is still working. What is going on?

PlayerMovement Script:

public class CharacterController : MonoBehaviour
{
    public static CharacterController Instance {get; private set; }

    [SerializeField] private float jumpForce = 500;
  

    private Rigidbody2D playerRb;
    private Collider2D playerCollider;

    private InputSystem_Actions inputSystemActions;

    private void Awake()
    {
        // Singleton
        if (Instance != null && Instance != this)
        {
            Destroy(gameObject);
        }
        Instance = this;

        // References
        playerRb = GetComponent<Rigidbody2D>();
        playerCollider = GetComponent<Collider2D>();

        // Input system
        inputSystemActions = new InputSystem_Actions();
        inputSystemActions.Player.Enable();
        inputSystemActions.Player.Jump.performed += Jump;
    }

    private void Jump(InputAction.CallbackContext context)
    {
        if (playerCollider.IsTouchingLayers(LayerMask.GetMask("Ground", "Interactable"))) //cant do 6, bitmask. 6 is layer 1 and 2
        {
            playerRb.AddForceY(jumpForce);
        }
    }
}

Interact Script:

public class ObjectInteractionController : MonoBehaviour
{
    public static ObjectInteractionController Instance { get; private set; }

    private InputSystem_Actions inputSystemActions;
    private void Awake()
    {
        if (Instance != null && Instance != this)
        {
            Destroy(gameObject);
        }
        Instance = this;

        inputSystemActions = new InputSystem_Actions();
        inputSystemActions.Player.Enable();
        inputSystemActions.Player.Interact.performed += Interact;

void Interact(InputAction.CallbackContext context)
    {
        if(heldObject == null)
        {
            Collider2D _collider = Physics2D.OverlapCircle(overlapCirclePosition, overlapCircleRadius, LayerMask.GetMask("Interactable"));
            if (_collider != null)
            {
                print("Disabling");
                inputSystemActions.Player.Jump.Disable();

                _collider.transform.SetParent(objectHolder.transform);
                _collider.transform.localPosition = Vector3.zero;
                _collider.transform.localRotation = Quaternion.identity;

                heldObject = _collider.gameObject;
                heldObject.layer = LayerMask.NameToLayer("Default");

                Rigidbody2D rb = heldObject.GetComponent<Rigidbody2D>();
                rb.bodyType = RigidbodyType2D.Static;
            }
        }
        else
        {
            Vector2 mouseScreenPos = Mouse.current.position.ReadValue();
            Vector3 mouseWorldPos = Camera.main.ScreenToWorldPoint(new Vector3(mouseScreenPos.x, mouseScreenPos.y, 0f));
            Vector2 forceVector = (mouseWorldPos - transform.position).normalized * flingForce;

            heldObject.transform.parent = null;
            heldObject.gameObject.layer = LayerMask.NameToLayer("Interactable");

            Rigidbody2D rb = heldObject.GetComponent<Rigidbody2D>();
            rb.bodyType = RigidbodyType2D.Dynamic;
            rb.AddForce(forceVector);

            heldObject = null;


            inputSystemActions.Player.Move.Enable();
        }
    }

I havent sent the entire interaction script, just the parts which i think are important

[patent pollen]

you are creating 2 different input actions instances, one in each class

[uncut frost]

ah that's what I thought was happening

[patent pollen]

disabling the jump in the interaction script wont disable it on the instance on the jump script

[uncut frost]

how would I use the same one?

[patent pollen]

You can create one in an InputSystem or something and store it as static so other classes can access it form there

or a singleton manager, dependency injection, there are many ways to approach this

[uncut frost]

great, thanks for the help

[outer kindle]

sounds really easy, but why not just make use of bools instead?

[patent pollen]

because the enabling/disabling of actions is exactly for that, you might be using actions in multiple places, if you just disable it you are fine in all, if you add another bool you would need to do changes in every place it is used

[uncut frost]

@patent pollen would you mind reviewing my fix to see if it's good practise or not?

[patent pollen]

sure

[uncut frost]

public class InputSystem : MonoBehaviour
{
    public static InputSystem Instance { get; private set; }
    private InputSystem_Actions inputSystemActions;

    // The inputs
    public event Action OnJump;
    public event Action OnDash;
    public event Action OnCrouch;
    public event Action OnInteract;

    private void Awake()
    {
        if (Instance != null && Instance != this)
        {
            Destroy(gameObject);
        }
        Instance = this;
        DontDestroyOnLoad(gameObject);

        inputSystemActions = new InputSystem_Actions();

        inputSystemActions.Player.Jump.performed += ctx => OnJump?.Invoke();
        inputSystemActions.Player.DashLeft.performed += ctx => OnDash?.Invoke();
        inputSystemActions.Player.DashRight.performed += ctx => OnDash?.Invoke(); // Figure dash out
        inputSystemActions.Player.Crouch.performed += ctx => OnCrouch?.Invoke();
        inputSystemActions.Player.Interact.performed += ctx => OnInteract?.Invoke(); // Do in other script


        inputSystemActions.Player.Enable();
    }

    public float GetMovementInput()
    {
        return inputSystemActions.Player.Move.ReadValue<Vector2>().x;
    }

    private void DisableMovement()
    {
        inputSystemActions.Player.Move.Disable();
    }

    private void DisableDash()
    {

    }
}

So I made a separate singleton class which manages the inputs which other classes can then refer to

I'm not sure if this is the correct way to solve the problem

[patent pollen]

yeah its a way you can do that, I see no problem with that

[uncut frost]

alright thanks

[patent pollen]

A bit weird to have one event trigger another event that you subscribe to from the outside, but its not inherently bad

but I would not discard the ctx, there is valuable info in there 😄

[scenic coral]

i dont really think this is a great way of handling it. you're basically just exposing a limited set of what the input system already offers except through events

[uncut frost]

how would you go about handling it?

[patent pollen]

i guess only expose the inputSystemactions and let other classes subscribe to it directly

without the proxy events

So your InputSystems only purpose would be storing the instance of inputSystemActions

[uncut frost]

ah ok

[scenic coral]

directly just call the methods you want, this input system class is going to be on a game object anyways. You'd likely have some kind of player or character class on the same object already

that class can pass along the input however it needs

Then if you end up making npc/AI's, they can have similar logic where some AI controller passes input by directly calling methods on their own player/character class. The only thing you would need to swap between making an AI vs a player is the code you've shown above
in your current system, you must have a class that specifically reads the values of human input

[uncut frost]

If I'm understanding correctly, the same solution as the person above proposed?

[scenic coral]

no, im saying separate classes dont need to be subscribing to events or the input system actions either. Lets say you have movement logic for your player, its pretty likely an enemy would move in the exact same way. The only difference is the input: you might use keyboard + space key to jump, the AI might use an if statement to figure out it needs to jump. The result is the same
If your movement logic is subscribing to the space key being pressed, well suddenly an AI doesn't work with this. The movement should just rely on being told to jump, then deciding to jump if its valid to do so

[uncut frost]

I'm now getting the error "Object reference not set to an instance of an object"

is that because my scripts are trying to get a reference to the event before the InputManager object has been instantiated?

[patent pollen]

yes, that could be

[uncut frost]

should I put my references to the events in Start instead of Awake

[patent pollen]

Does your inputsystem need to be a monobehaviour? I dont see any references you would put on there in the inspector

Instead you can make it a lazy singleton, the first one to request the "Instance" also triggers the init process of that class

(override the get function of instance to trigger the init process)

[uncut frost]

How would I go about making it a lazy singleton?

[patent pollen]

public class InputSystem
{
    public InputSystem_Actions InputSystemActions;
    public static InputSystem Instance
    {
        get
        {
            _instance ??= new InputSystem();
            return _instance;
        }
    }
    
    private static InputSystem _instance;


    private InputSystem()
    {
        InputSystemActions = new InputSystem_Actions();
    }
}

now any other class can just call InputSystem.Instance.InputSystemActions

The first one to do so (or call the .Instance at all) triggers the constructor of InputSystem

no need to attach it to a gameobject

[scenic coral]

Theres still absolutely no reason that this should be a singleton, or that other classes will be relying on the input being specifically human input

[uncut frost]

I won't be using any AI/enemies

[patent pollen]

if you dont want to use the inputsystem for anything else you can also just lazy load the actions like

public class InputSystem
{
    public static InputSystem_Actions InputSystemActions
    {
        get
        {
            _inputSystemActions ??= new InputSystem_Actions();
            return _inputSystemActions;
        }
    }
    private static InputSystem_Actions _inputSystemActions;
}

and then you can call InputSystem.InputSystemActions from anywhere, does not create an InputSystem instance then

[patent pollen]

well he needs a shared instance of the InputSystemActions somehwere, might aswell use a singleton

[uncut frost]

thanks for all your help

[scenic coral]

A shared instance is infact still not needed. Input can be read from one central place and passed down to systems that need it

[patent pollen]

The InputSystemActions IS the central place where inputs are read 😄

If you mean a system that distributes inputs to other systems, yes, that is possible but a design decision, its just a different solution

[scenic coral]

Yes my message saying "passed down to systems that need it" refers to distributing input to other systems.
Regardless it doesnt need to be a singleton here, there's literally no purpose

It could very well just stay as a component which allows for drag/drop functionality on what kind of input is used
I know they said they wont have AI/enemies but regardless, my suggestions are clearly about design