I have an IPawnInput interface that has an OnInput(object sender) method for input. Before, I just called this method for the other components (PlayerMovement, PlayerCamera) through the "main" class - Player. Soon I had problems linking all the necessary components to the main one. I decided to have an input manager that calls this method on all the required components. Because this manager itself should get the input command from above, I created a class PlayerInputManagerSender that has information about the original sender of the input request. Sending the original sender directly is not an option, because some components need to know that they were used through the manager. For some reason creating this class causes a Stackoverflow error. I understand that calling method and creating a class every frame isn't the best solution, but I have no other solution, because input in some components must be read every frame.

using System.Collections.Generic;
using UnityEngine;

namespace LOK1game
{
    public class PlayerInputManager : MonoBehaviour, IPawnInput
    {
        [HideInInspector] public List<IPawnInput> PawnInputs = new List<IPawnInput>();

        [SerializeField] private List<MonoBehaviour> _actors;

        private void Awake()
        {
            foreach (var actor in _actors)
            {
                if(actor is PlayerInputManager)
                {
                    throw new System.Exception("Input manager can not calls itself!");
                }

                PawnInputs.Add(actor.GetComponent<IPawnInput>());
            }
        }

        private void Update()
        {
            //Test temp solution
            OnInput(this);
        }

        public void OnInput(object sender)
        {
            var managerSender = new PlayerInputManagerSender(sender, this);

            foreach (var pawn in PawnInputs)
            {
                pawn.OnInput(managerSender);
            }
        }
    }

    public struct PlayerInputManagerSender
    {
        public object OriginSender;
        public object ActualSender;

        public PlayerInputManagerSender(object originSender, object actualSender)
        {
            OriginSender = originSender;
            ActualSender = actualSender;
        }
    }
}

I've just tried to change class to struct

CodePudding user response：

StackOverflow exceptions are often causes by infinite recursion. One of your functions is calling another function that calls back to the originally called function somehow. Use breakpoints/writelines in your OnInput() and Update() functions to make sure they're being called the appropriate number of times for what you expect.

CodePudding user response：

Your class PlayerInputManager is implementing IPawnInput itself so when you do

foreach (var actor in _actors)
{
    if(actor is PlayerInputManager)
    {
        throw new System.Exception("Input manager can not calls itself!");
    }

    PawnInputs.Add(actor.GetComponent<IPawnInput>());
}

it is very possible that actor itself is not the PlayerInputManager component but another one that is attached to the same GameObject.

=> It is not prevented that a PlayerInputManager makes it into the PawnInputs!

You should do the check rather after GetComponent<IPawnInput> like

foreach (var actor in _actors)
{
    if(actor.TryGetComponent<IPawnInput>(out var pawnInput))
    { 
        if(pawnInput is PlayerInputManager)
        {
            // personally I would just ignore it and not throw an exception
            // that seems a bit overkill ;)
            continue;
        }

        PawnInputs.Add(pawnInput);
    }
}

CodePudding user response：

My guess is that you have cyclic references. PlayerInputManager is a IPawnInput, and owns a list of IPawnInput. So there is the possibility that some other IPawnInput object contains a reference to the owning object. Essentially forming a graph. So when you call OnInput you end up with infinite recursion, and eventually run out of stack.

To check for this you may iterate over the graph and check if any object occurs twice:

public static bool HasCycle<T>(T self, Func<T, IEnumerable<T>> selector, IEqualityComparer<T> comparer = default)
{
    var stack = new Stack<T>();
    stack.Push(self);
    comparer ??= EqualityComparer<T>.Default;
    var visited = new HashSet<T>(comparer);
    while (stack.Count > 0)
    {
        var current = stack.Pop();
        if (!visited.Add(current))
        {
            return true;
        }
        foreach (var child in selector(current))
        {
            stack.Push(child);
        }
    }
    return false;
}

Called like

var result = HasCycle(this, p => p.PawnInputs);

You could modify the iteration code above to give you what object occurs twice, or call it every time you add a pawn to find where the cycle is created, and fix the issue.

You can also use a similar approach to just ignore any duplicated items in the graph in case you actually intend to have cycles, but want to process each item in it once.