Too many cross script references?

Hello,

I have my Player GameObject, and I currently have about 10 scripts on the object to control various things. I’m trying to follow single responsibility, which causes so many scripts. I have scripts for

  • movement
  • casting spells
  • tracking buffs/debuffs
  • tracking/consuming/regen mana
  • tracking health
  • tracking ultimate power
  • Animation
  • Is the player grounded?
  • Is the player locked because the game is stopped?
  • handling ragdoll

so far, so good. But many of my scripts need to know about the state of other scripts.
I cannot move, if the DebuffHandler has a “rooted” debuf, or if the player is not grounded.
I cannot cast spells if there is not enough mana. Some spells block mana regeneration for a while.
I cannot cast certain spells if I am silenced, or slowed.
I cannot cast spells if the player is not grounded.
I cannot cast the ultimate, if there is not enough ult power, or the player is not grounded, or is silenced.
I cannot sprint or teleport if I’m carrying an item.

And so on…

So the behaviour of many of my scripts depends on all kinds of information from many of the other scripts, and I feel like I’ve ended up with a lot of references back and forth.

Does anyone else encounter something similar?

You may be interested in interfaces. You could make an interface, like:

interface IStatusReceiver {
void ReceiveStatusEffect(StatusEffectEnum status, float duration);
}

And then any of your scripts that need to know you’ve received a status effect can implement the interface:

public class Movement : MonoBehaviour, IStatusReceiver {
public void ReceiveStatusEffect(StatusEffectEnum status, float duration) {
if (status == StatusEffectEnum.Slow) movementSpeed = 5f;
}

(A script can implement as many interfaces as it needs to)
And here’s the cool thing, GetComponent & co work on interfaces:

var allStatusReceivers = GetComponentsInChildren<IStatusreceiver>();
foreach (var sr in allStatusReceivers) {
sr.ReceiveStatusEffect(StatusEffectEnum.Slow, 5f);
}

So that may help reduce these codependencies a lot.

2 Likes

Thank you very much for your input.
I’m familiar with interfaces, though don’t use them that often in Unity. Most of my scripts implements Resetable, for when the player dies and needs to respawn.

But I think I need a bit more to see your point. Do you mean to e.g. track the status in each script? I have currently two scripts interested in the slow status. Should they each track the status?
Currently, they will, as needed, ask the DebuffHandler, IsSlowed? Or CanTeleport? IsRooted? IsSilenced? But that means both my MovementHandler and my SpellCaster needs a reference to the DebuffHandler script (I seem to have a lot of *Handler scripts)

I previously had a PlayerStats, which grew large and handled many things: health, mana, ultimate power, turning the player on/off upon death, activating ragdoll/deactivating animator, keeping track of buffs, tracking is-touching-ground status, updating UI health/mana bars and more. The idea was to have one “gateway” script to a player.

Now, I’ve split the various responsibilities out into separate scripts.
So, when a spell hits a player, that spell needs to look up e.g. DebuffHandler to apply silence. UltimatePowerHandler to reduce ultimate power/mana. HealthHandler to deal damage. ForceHandler to apply a push to the player hit.

But those are separate things. I don’t see that I can hide it all behind one interface. The arguments of the various method calls are different. Floats, vectors, enums, strings… The interface implies some kind of similarity between the scripts, which I can’t identify.

1 Like

For one prefab with about 20 scripts attached which need information from the other scripts, I resolved the reference madness by having one additional script which has references to all the object’s scripts, and each script has a reference to that script. So whenever I need to access something on another script I just go through the single reference.

3 Likes

Joe, that sounds like the service locator pattern? My friend just mentioned it to me, it sounds like it’s commonly used.

I’m not sure it will change much for me, though. It will be a difference in my start() methods:
healthHandler = ServiceLocator.GetHealthManager();

versus

healthHandler = GetComponent();

There might be a small performance boost in your approach, because the GetComponent has to do a search (I believe). But almost all mine is done in the start method, when a scene loads. I doubt I’ll gain a noticable performance increase. I might change later. It makes sense if you have to look up scripts at run-time, no doubt.

You could mimic what MonoBehaviour used to do many moons ago. Make all your scripts of this kind derive from a common class (ServiceLocatingBehaviour maybe?). In that class, have a bunch of member variables (healthHandler, etc) which are all set in initialization with GetComponent<>. Now, in all of these classes, you can just check healthHandler.health < 5 or whatever.

(You’ll have to be careful of Unity’s weird behavior with the builtin Start() etc functions - if your derived class has Start(), it will silently hide the parent class’s Start() and the initialization won’t be run)

1 Like

The biggest gain is just saving yourself some headache having to add a new reference cache variable and GetComponent call each time you need to reference another script on the same object for the first time. When I was doing all those GetComponent calls in Start I was seeing a stutter on instantiation, which went away when I removed all of them. (probably had at least 40 GetComponent calls at the time, but I instantiate multiple of this object at once so that’s probably a lot for a single frame).

1 Like

That’s a good point, Joe. So, in your Awake() methods, a script will add itself to the servicelocator, and in Start() it will fetch its dependencies?
I just have about < 5 dependencies to set up, in most of my scripts, so I’m not seeing problems yet. But you’re right, it probably doesn’t scale all that well.

Games like League of Legends, Heroes of the Storm, Battlerite, DotA, they must all have similar challenges. I wonder how they attack this problem.

Another question, how is your servicelocator implemented? Is it a map, so you just have one get method:
HealthHandler hh = (HealthHandler)servicelocator.get(“HealthHandler”);

or do you have a specific method for each script in your service locator?
Or are you using generics somehow?

Personally I do it a little similar to what @Joe-Censored suggested.

Components often only know nothing about how they’re used. They are sort of passive, except for a few Unity messages that are used to (de-)initialize everything that belongs to themselves.

Then, I usually tend to use an orchestrating behaviour, which knows all these components that make up a certain kind of entity. This behaviour is very application / context specific. It is not necessarily meant to be re-used across multiple projects, but specific to one application in the first place, whereas the other components can be very self-contained.

In practice, this orchestrating scripts query information, and instruct components to do a certain thing.

In practice:
Instead of having the movement behaviour asking the player behaviour whether it is grounded, and then do movement, it’ll be the orchestrating script that queries the player behaviour, checks whether the player is currently grounded, and if it is, instructs the movement behaviour to do its thing.

Player and movement are decoupled. The same works similarly with interfaces, unless these get too specific and expose too much details.

Another major difference is that the components do not run completely independently based on outside conditions, but provide the required API to to their thing.

In this case, movement can also be reused in disregard of having any player component at all.

1 Like

That sounds like a more elegant solution, but what I was referring to was just a straight brute force approach. This is just a copy/paste of the script in my project I’m referring to, even the script comments :stuck_out_tongue: .

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using JCGNetwork;
public class ShipComponents : MonoBehaviour {
    //Maintains references to all other scripts on the ship object
    //Created this because getting and tracking references to the other components on the ship was getting out of control
    public Rigidbody ShipRigidbodyComponent;
    public JCGIdentity JCGIdentityComponent;
    public JCGTransformSync JCGTransformSyncComponent;
    public ShipMovement ShipMovementComponent;
    public RockTheBoat RockTheBoatComponent;
    public WeaponControl WeaponControlComponent;
    public ShipOutfitting ShipOutfittingComponent;
    public AI AIComponent;
    public ShipStats ShipStatsComponent;
    public SailModelControl SailModelControlComponent;
    public Armament ArmamentComponent;
    public MiscShipMaintenance MiscShipMaintenanceComponent;
    public ShipOwner ShipOwnerComponent;
    public CityShipInterface CityShipInterfaceComponent;
    public ShipCrew ShipCrewComponent;
    public ShipStorage ShipStorageComponent;
    public ShipDamageStatus ShipDamageStatusComponent;
    public ShipHeel ShipHeelComponent;
    public ShipBonusAndAbilities ShipBonusAndAbilitiesComponent;
    public ShipFaction ShipFactionComponent;
    public ShipGathering ShipGatheringComponent;
 
    //Send an update of all scripts to the client now
    //Used such as when a player reconnects
    public void UpdateClientNow()
    {
        ShipStorageComponent.UpdateClientNow();
        ShipCrewComponent.UpdateClientNow();
        WeaponControlComponent.UpdateClientNow();
        CityShipInterfaceComponent.UpdateClientNow();
    }
}

Then in every one of those scripts, I have this line.

public ShipComponents ShipComponentsScript;

And I connect them all in the inspector. Then I use it as an example like this in my ShipMovement script, which relies on values from several other scripts.

    //run this to propel the ship
    private void propulsion()
    {
        if (ShipComponentsScript.ShipDamageStatusComponent.IsShipSunk() != true)  //We don't propel the ship if it is sunk
        {
            if ((ShipComponentsScript.ShipStatsComponent.HasSails) && (currentSailConfiguration != SailModelControl.SailConfiguration.Furled))
            {
                float crewEffectPercent = ShipComponentsScript.ShipCrewComponent.GetCrewEffectMultiplier(Crew.CrewType.sailing);  //The crew effect on sail propulsion, a value between 0.25 and 1
                float sailConfigEffect = 1f;
                if (currentSailConfiguration == SailModelControl.SailConfiguration.BattleSails)
                {
                    sailConfigEffect = 0.4f;
                }
                PointOfSailToWindDirection postwd = HowIsCurrentPointOfSail(ShipComponentsScript.SailModelControlComponent.CurrentPointOfSail);
                float maxSpeed = (ShipComponentsScript.ShipStatsComponent.GetMaxSpeed(ShipComponentsScript.SailModelControlComponent.CurrentPointOfSail, postwd) * crewEffectPercent * sailConfigEffect * ShipComponentsScript.ShipBonusAndAbilitiesComponent.SailingSpeedBonus);
                if ((maxSpeed > 0f) && (Speed < maxSpeed))
                {
                    ShipComponentsScript.ShipRigidbodyComponent.AddForce(transform.forward * ((float)SailForce * funFactorSpeedMultiplier * crewEffectPercent * sailConfigEffect * ShipComponentsScript.ShipBonusAndAbilitiesComponent.SailingSpeedBonus), ForceMode.Force);
                }

            }
            if (ShipComponentsScript.ShipStatsComponent.HasOars)
            {
                if (rowingEnabled)
                {
                    if ((ShipComponentsScript.ShipStatsComponent.GetMaxOarSpeed()) > 0f && (Speed < ShipComponentsScript.ShipStatsComponent.GetMaxOarSpeed()))
                    {
                        float crewEffectPercent = ShipComponentsScript.ShipCrewComponent.GetCrewEffectMultiplier(Crew.CrewType.rowing);  //The crew effect on rowing
                        ShipComponentsScript.ShipRigidbodyComponent.AddForce(transform.forward * ((float)OarForce * funFactorSpeedMultiplier * crewEffectPercent * ShipComponentsScript.ShipBonusAndAbilitiesComponent.RowingBonus), ForceMode.Force);
                    }
                }
            }
        }
    }

Dependency injection or service locator can work to get references, or use an EntityController class that manages the other components, like Suddoha said. It would use some FSM to turn on/off components when required. For example, if the game gets “Paused”, then put the player into the Locked state which would disable the appropriate components, and not respond to input.

But, I would use a Behaviour Tree. It sounds like your player can be treated like an AI and you just need to block off certain code based on conditions, such as HasTarget?, IsCastingSpell? IsAttacking? and IsJumping?, then a Selector node will just choose what is viable. Obviously it’s not an AI, so input would be controlled by the user.

Here, nothing on the left can occur unless an enemy is visible. Each node checks only its children left-to-right.

You would have functions/nodes that check your other components for their values. Your Jump action would be in a branch that can only happen if IsGrounded/IsCasting nodes return the correct value, and of course, if the player isn’t Locked. The IsLocked node would be on the left side, so it has higher priority and will block everything after it, so you won’t be able to jump/cast/anything.

5609053--580525--unnamed.png

In case anyone is interested, here’s a very interesting talk on how to use scriptable objects to make your game very modular and decoupled:

Man… I really need to look into these. It’s solid base programming to use interfaces, yet, I don’t even use it once. :sweat_smile:
I also stripped all my classes and removed functionality that were just too monotonically coded.
My code right now is like a Swiss cheese. xD
Might need to call out for help from Charles (Infallible code) as I’m sure he’d be willing to help me refactor my code for some money in exchange.

I totally agree with @Suddoha . As a small utility that makes referencing all these components less painful, I built an attribute that automatically finds references https://forum.unity.com/threads/automatically-reference-components-on-same-object-child-or-parent.862456/

1 Like

Also, I use events a lot for decoupling components. The orchestrating component can subscribe to events of one component and trigger another as a response. Or use UnityEvents so that these connections can be established in the editor and are not hard-coded in your scripts.

For example, If an enemy can be hit and takes damage if it is, you can construct it like this.

public class Hittable : MonoBehaviour
{
    public delegate void HitDelegate(int damage);
    public HitDelegate WasHit;

    public void GetHit(int damage)
    {
        WasHit?.Invoke(damage);
    }
}

public class Health : MonoBehaviour
{
    private int _health;

    public void TakeDamage(int damage)
    {
        _health -= damage;
    }
}

public class Enemy : MonoBehaviour
{
    [SerializeField, ComponentReference] private Health _health;
    [SerializeField, ComponentReference] private Hittable _hittable;

    private OnEnable()
    {
        _hittable.WasHit += _health.TakeDamage;
    }

    private OnDisable()
    {
        _hittable.WasHit -= _health.TakeDamage;
    }
}

Or, with Unity events, you can use the following approach and connect the event to TakeDamage in the inspector. This is viable since the new prefab workflow with variants was introduced, I think.

public class Hittable : MonoBehaviour
{
    public HitEvent WasHit;

    public void GetHit(int damage)
    {
        WasHit?.Invoke(damage);
    }

    // need this to make the event with parameter serializable
    [Serializable]
    public class HitEvent : UnityEvent<int> {}
}
1 Like

The ScriptableObject value approach is only good for global values. If you have more than one enemy/player having them share one health SO won’t work.