Best coding practices: A manager listener scripts in C#

Unity Engine
1

I’ve written a flocking behavior script, and I feel that I’m writing some clunky code, and I’d like some advice on how better to organize this code.
I’m what you’d call an “advanced beginner” at coding, and am trying to be more effective, efficient, and organized.

So, in my manager script is the following code particle, where I’m creating the manager script and creating an instance as a static variable to be referred to.

[System.NonSerializedAttribute]
	public List<GameObject> boids; 
	
	void Awake(){
		Instance = this;
		boids = new List<GameObject>();
		
		
	}

then I have the manager doing a FindAllObjectsWithTag() to build the List<>

void Start () {
		populateList ();
		for (int i = 0; i < boids.Count; i ++){
			GameObject boid;
			boid = boids[i];
			boid.BroadcastMessage("setLocalArray");
		}
	}
	
	
	void Update () {
		compareVars();
	}
	
	void populateList(){
		GameObject[] allFollowers;
		allFollowers = GameObject.FindGameObjectsWithTag("follower");
		foreach(GameObject follower in allFollowers){
			boids.Add (follower);
		}	
		print ("length of boids is " + boids.Count);
	}

And finally, I have a compareVars() function in the update loop that checks the manager’s variables and passes them on to the individual boids if they have changed.
This is the part that I feel is really clunky, and it seems like there must be a better way to broadcast variables to individual flock members. There is a function on each boid, setLocalArray(), that references this manager’s variables and updates themselves.

	void compareVars(){
		if(separateWeight != separateCompare || seekWeight != seekCompare || wanderWeight != wanderCompare || cohesionWeight != cohesionCompare){
			for(int i = 0; i < boids.Count; i ++){
				boids[i].BroadcastMessage("setLocalArray");
			}
			separateCompare = separateWeight;
			seekCompare = seekWeight;
			wanderCompare = wanderWeight;
			cohesionCompare = cohesionWeight;
		}
	}

So, my question is, are there better ways of doing this? I stumbled on this solution just seeking around and doing tutorials, and I’ve crunched it together in the only way I know how to do. What could I do better?

Thanks in advance for the advice

2 
using UnityEngine;

public class FlockManager : MonoBehaviour
{
	private void Start()
	{
		ResetAllBoids();
	}

	private void Update()
	{
		if (_condition_)
		{
			ResetAllBoids();
		}
	}

	private void ResetAllBoids()
	{
		foreach (var boid in Boid.boids)
		{
			boid.DoSomethingUseful();
		}
	}
}

using System.Collections.Generic;
using UnityEngine;

public class Boid : MonoBehaviour
{
	public static readonly List<Boid> boids = new List<Boid>();

	private void Awake()
	{
		boids.Add(this);
	}

	public void DoSomethingUseful()
	{
		// ...
	}
}
3

The only tip i have for you is to consider your data structure for storing the boid data. AI is heavily dependent on the data structure you choose for the data your storing.
It looks like you didn’t even think about it (hence your usage of a List<GameObject>).
broadcast IS a terrible way to send data through objects, look into a messaging system or delegates, not only are they much faster, but they clearly define what they do.

Now you made hand-tuned flocking… MUAHAHAH. That’s the stuff i learned in school, our TEST was to write flocking without setting up the flocks. The reason is because flocking is an AI pattern. The boids would just start flying and then get together, a leader is formed and followers begin following. When two flocks of boids intersected, they might decide to switch flocks. Why am i pointing this out to you? Because it will enlighten you as to how to message objects back and forth better when you don’t know who’s managing them when you solve that problem.

Now as an experienced tip, try to avoid singleton patterns (Instance = this). They are dangerous when it comes to initialization and shutdown but are useful for defined resources. For some reason this year, I’ve ended up using them alot (accelerometer stuff i guess…).

4

I wouldn’t use messages. Direct calls on stored references or delegates are better.

Why are you using tags? Is it because you don’t know what types might be managed so you are abstracting through tags? I would avoid this if possible. Perhaps using a base class that can be used with List<>.

IMO the delegate event pattern is the best because the code is totally decoupled, but this requires that the “managed” instances know enough to register their delegates in Awake()

5

Singleton with static access is fine. Just make sure you don’t store static references directly (just reference within function scope, for example, and if you must, be sure to set == null for clean-up!). Managers are a great example for singleton static access in Unity. It lets you have instance access for inspector settings and access to components without complex “find” code. Just add a check in the manager’s Awake() to ensure that the static “Instance” field is null, otherwise throw an error. In other words, protect yourself properly and it is a great pattern.

I’m a fan of keeping the instance field “private” and using property get and set to access it. I think it makes more a more explicit interface. I tend to use the name “_Inst”. The underscore denotes that it is designed for internal use, even if it isn’t be officially private. I guess this is stolen from Python.

6

Hmm! Cool, I’m not familiar with using static readonly vars, i assume they take less memory are faster access?
Aside from that, I’m somewhat aiming at the if(condition){ resetAllBoids} style by comparing all the current variables with their stored reference.
But it still feels clunky, like I should have them in an array or something.
Thanks for the response

7

Hmm, well, I wasn’t sure how else in unity to store a dynamic list of GameObjects where I could iterate over them easily. I ran into problems with Arraylist and also just using GameObject[ ] boids. I forget exactly what the issue was, but I believe it was directly accessing the datatypes and iterating through the list.
What would be a better data storage model?
Also, how in Unity could I more effectively communicate between scripts? I used the singleton pattern because that’s essentially the best way I know how to have scripts reference each other. So in this case the FlockManager script manages the flocks, no matter how many of them there are (I have yet to set it up for multiple flocks, but that’s what I’m working on now).

Yeah! Flocking ends up looking really cool. I haven’t put in any leader or flock switching behaviors yet. So far I just have seek, cohesion, separation, and wander behaviors. I adapted my flocking script from the example in processing and also from ideas here: http://www.red3d.com/cwr/steer/
Thanks for the advice!

8

You might consider using a co-routine. Then you have the option to reduce the frequency of your checks. E.g. every 0.3 seconds. You can even start launching more than one and offsetting them if you want to update large groups of objects by spreading them out over time. The player shouldn’t notice.

I wouldn’t expose the list. Keep it private and expose access to the list. There isn’t much need for explicitly marking as read-only. A property with only get{} will handle that.

Why not have boids add themselves in OnEnable()? They could call a static method “Boid.Add(this);”… oh wait… is the Manager class and the “Instance” class the same class?

A little note, I always recommend prefixing static fields with the class name. For example, in your example, “Boid.boids(this);”. It makes it obvious how the field is being accessed.

9

yes, I’m using messages/tags because I don’t know how better to get references to objects. I’m aware it’s a slower way to do things, but it’s what I know how to do currently. How can I set up stored references or delegates in this case?

I’ve read about the get{} and set{} messages, usually in references to interfaces, but I’m not savvy on how to use them. Looks like I got some learnin’ ahead of me

Thanks!

10

Use a List of the component type you want to access. If you store GameObjects you will always have to GetComponent() before you can access anything, which is a big waste. For example, List

11

Hmm, well, currently it only searches for tags once. I instantiate a flock, and the manager script adds all of the followers in the scene to the array, and then hands that array to each boid. From there on out, all flock logic is handled by the script on each individual boid. But, I see that having the boids add themselves is a better way, and therefore cutting out the tag searching method.

12

OHHHHH!!!
It hadn’t occurred to me that the type of data in a List could be my custom class! I had been functioning under the assumption that it had to be one of Unity’s pre-defined classes. THIS CHANGES EVERYTHING.
I did at least have it set up with:
boids = List;

13

Just typing, not testing… but roughly…

class Manager : MonoBehavior
{
    public static delegate OnUpdate();
    public static OnUpdate onUpdate;

    private void Awake()
    {
        // Clear static delegates in case of level change
        //    It is a good idea to set this script to run first in the project's order
        Manager.onUpdate = null;
    }

    private void Update()
    {
        Manager.onUpdate();   // Runs them all
    }
}

class Boid : MonoBehavior
{
    private void Awake()
     {
        // Subscribe to event delegate
        Manager.onUpdate += this.OnUpdate; // Can be named anything. Just match delegate signature
     }

    private void OnUpdate()
    {
        Debug.Log("The manager triggered me!");
    }
}
14

Hey Rafes,
Thank you so much for illustrating how it works. I got this set up and it’s changed the way I think about organizing my scripts now. Took me a good portion of the day yesterday to wrap my head around how to get it to work properly and how to effectively use it, but I’m on it. Thanks so much! This is exactly the kind of info I was looking for.

15

Might put something in the Boids class to unsubscribe if/when the gameobject gets nuked, maybe:

private void OnDestroy()
{
    Manager.onUpdate -= this.OnUpdate;
}
16

Yup! I did that very thing, actually. After some reading I realized this was going to be necessary. Thanks for the heads up

17

I love efforts like this…it’s very helpful and it’s great to see the community contribute, to make an excellent solution.

I encourage you to put this in the wiki as a code snippet or something titled “Simple Manager and Listeners”. I think forums are a lousy home for things like this :slight_smile:

18

Yeah, this worked out really well and has helped me enormously on my current project. Great advice, as soon as I’m done with my current run I’ll pare down the code and put it on the wiki.

Thanks for everyone who contributed to this