Is it fine for scripts to be dependent on other scripts?
For example, I have a Player script and a UIManager script shown below. When the player collects matter, it calls a function from the UIManager to update the UI, that gets the value from the Player script then updates the UI with that value.
Is that fine? Is this bad design? Should I be designing my ‘manager’ scripts differently?
Thanks.
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Player : MonoBehaviour
{
private static Player _instance;
public static Player Instance { get { return _instance; } }
private UIManager uIManager;
public int matterAmount = 0;
private void Awake()
{
if (_instance != null && _instance != this)
{
Destroy(this.gameObject);
} else {
_instance = this;
}
}
void Start(){
uIManager = UIManager.Instance;
}
public void SetMatter(int amount){
matterAmount += amount;
uIManager.UpdateMatterText();
}
}
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using TMPro;
public class UIManager : MonoBehaviour
{
public TextMeshProUGUI matterUI;
private Player player;
private static UIManager _instance;
public static UIManager Instance { get { return _instance; } }
private void Awake()
{
if (_instance != null && _instance != this)
{
Destroy(this.gameObject);
} else {
_instance = this;
}
}
void Start(){
player = Player.Instance;
}
public void UpdateMatterText(){
matterUI.text = player.matterAmount.ToString();
}
}
It’s fine for very small games, but it’s not a good design.
UI Manager is very abstract name which tells you nothing. As project grow this class can get very big because it has no strict responsibility.
Player is responsible for updating UI. People usually use events for that.
How “fine” it is depends on your game. Your Player now depends on a UIManager existing, so if you put the Player into an empty scene it will break, or even if you rename UIManager. If that’s not a problem, then it can work.
The first issue can be “solved” with null checks. Renaming/deletion issues can be solved via an depending on an Interface (IUIManager interface) instead of a concrete type (UIManager class).
One thing you can do immediately is remove the “uIManager” field/instance in your Player. You have already made a static instance (UIManager.Instance), so you can just use that, unless you want a shorter name.
The Player referencing UIManager doesn’t make much sense. Objects should generally not know about UI. If you have to have them know about each other, it would be better for the UIManager to know about the Player, so everything is in one place.
Example of quick solution (UIManager knows about Player, Player doesn’t know about UIManager)
using System;
public class Player : MonoBehaviour
{
public int matterAmount;
public static event Action<int> UpdatedMatter;
public void SetMatter(int amount)
{
matterAmount += amount;
UpdatedMatter?.Invoke(matterAmount); // UIManager inserts a function (OnUpdatedMatter) to be called here
}
}
public class UIManager : MonoBehaviour
{
public TextMeshProUGUI matterUI;
void OnEnable()
{
Player.UpdatedMatter += OnUpdatedMatter;
}
void OnDisable()
{
Player.UpdatedMatter -= OnUpdatedMatter;
}
void OnUpdatedMatter(int amount)
{
matterUI.text = amount.ToString();
}
}
UIManager knows about Player, Player doesn’t know about UIManager
UIManager code has to be modified when Player is renamed/deleted/changed.
UIManager has to add new lines of code when something more than just the Player can update matter text.
Example of more decoupled that supports more than just a Player:
public interface IUpdateMatter
{
event Action<int> UpdatedMatter;
}
public class Player : MonoBehaviour, IUpdateMatter
{
// Same code as previous example, except event isn't "static".
}
public class UIManager : MonoBehaviour
{
public TextMeshProUGUI matterUI;
private List<IUpdateMatter> _allUpdateMatters = new List<IUpdateMatter>();
void Awake()
{
// Find everything that implements IUpdateMatter
_allUpdateMatters = FindObjectsOfType<IUpdateMatter>().ToList();
}
void OnEnable()
{
foreach (var i in _allUpdateMatters)
{
i.UpdatedMatter += OnUpdatedMatter;
}
}
void OnDisable()
{
foreach (var i in _allUpdateMatters)
{
i.UpdatedMatter -= OnUpdatedMatter;
}
}
void OnUpdatedMatter(int amount)
{
matterUI.text = amount.ToString();
}
}
UIManager doesn’t know about Player, Player doesn’t know about UIManager. Both know about IUpdateMatter interface.
Supports adding new objects that can update matter without modifying UIManager code.
UIManager might need to refresh its list/subscriptions if new things are added after Awake.
Another way would be using ScriptableObjects to allow use of inspector, but I don’t really do that, so maybe someone else can explain.
What you’re mentioning is singletons with tight coupling.
Robot needs an instance of Player in order to call SetMatter.
Then Player needs an instance of PlayerUI to update UI.
If you’d have multiple players, you’d be having a player manager and even more tight coupling. Robot needs PlayerManager to get the Player to call SetMatter and update the UI…
When you want to test a little part of your logic you’ll need all the managers. All the things that are coupled.
Even if the only thing you want to test is in E, it needs B, C, D, F, G, H and I to function normally without errors because they all depend on each other.
Here’s the talk on scriptable objects which ryan mentions singletons.
Singletons are easy to work with when you’re having a small game. It gets the job done.
When your code base becomes larger, it becomes annoying.
Scriptable Objects isn’t necessarily the holy grail that solves everything. Even scriptable objects has its own annoyances.
If for example combined with Addressable Assets… you could have copies inside the Scene that is built in with the game as well as copies from addressables from either streaming assets or a content network server. You’d need to pull everything to the addressable side in order to fix that.