Better way to get the closest object of type T on a specific team

I have a lot of monobehaviors that are attached to a gameobject that has a TeamObejct component. Each TeamObejct belongs to a certain Team class. Many of these monobehaviors have functions on them that take a position and a team and returns the closest member of that class that is on the same team that was specified in the input.

For example, I have a ConstructionSite script, a StorageCenter script, and a WeaponController script, which all implement the interface ITeamBasedObject so I can get the Team that each componet is a part of. Right now, each component has identical static list that stores all active instances of each of the class, and a static function that returns a list of the classes that are on a paticular team.

Instead of rewriting the same filtering code over and over and over for each class, how could I just make a single script (wether its a class, interface, or something else I don’t know about yet) that would find the closest component of type T (where T is a ITeamBasedObject) on a paticular team?

Turn it upside down.

Rather than each type of object containing a list of teams it belongs to, make a Teams script that has lists with what kind of objects the team owns.

You needn’t even specify what kind of object that is so you could use a single list and then check if the object is of a given type, for instance by calling TryGetComponent<Building>(). If that ever were to become a bottleneck, it would be easy to refactor so that you keep a cached list of objects owned by the team for certain types.

Doesn’t adding a list of all buildings, unitMovmement, fabricator, constructionSites, builders, ect so forth to a Team script every time we add a new component break the Open–closed principle? I am a little afraid of calling GetComponet<> too because (at least in my experience) it gets expensive fast (I know premature optimization is bad too, but GetComponet seems to be pretty much universally agreed upon as being slow, and I may be calling this functions hundreds of times a secound)

Is each team it’s own class? I feel like you’d only need the one type with which you can compare equality, and then different instances of said type. A scriptable object is perfect here.

I mean this just sounds like an instance where you’d need a static utility class, and some methods that use generics:

public static class TeamUtils
{
    public static T GetClosestTeamObjectOfType<T>(Vector3 position, Team team) where T : ITeamBasedObject
    {
        // do stuff
    }
}

If you write terrible code it can be a factor. Otherwise it’s not.

The GetComponent<> is pretty darn fast but GetComponent(s)InParent/Children<> execution time highly depends on your scene hierarchy … imagine a root object trying to find the pinky bone’s Rigidbody of a specific boss enemy in a deep hierarchy where enemy instances are sorted by their types into subfolders but the bosses are last to be enumerated.

Anyhow, you know we shouldn’t write code like this, right?

Update()
{
   foreach (var go in manyObjects)
   {
         this.gameObject.transform.GetComponent<Rigidbody>.Something();
         this.gameObject.transform.GetComponent<Rigidbody>.SomethingElse();
         this.gameObject.transform.GetComponent<Rigidbody>.velocity = speedAndDir;
         this.gameObject.transform.GetComponent<Rigidbody>.isKinematic = false;
         this.gameObject.transform.GetComponent<Rigidbody>.YetAnotherMethod();
   }
}

That’ll be needlessly compounding the marginal cost of GetComponent multiple times and that can add up! This also operates on the component of the script, not each enumerated game object, and in addition (I couldn’t help it) this also adds the unnecessary overhead and convolutedness of writing wholly needless statements that only badly influencing youtube tutorial idiots spew out to their teenage audiences who would completely misunderstand if I said they ought to KISS! :laughing:

But doing something like this is absolutely a-okay:

AddBuildingToTeam(GameObject building, Team aTeam)
{
    aTeam.AddBuilding(building.GetComponent<BuildingScript>());
}

This happens once per new building and then you can work with the building’s components.

1 Like

First of all, thank you for taking time to help me understand everything! Sorry for making a mega-post for you to read, I just want to explain my thoughts so that way we can hopefully figure out whether or not my percived issue is actually real or not

My (probably flawed) reasoning against using the Team class to store my objects:

Why I don’t think I want to implement seperate lists for each componet

To be short, I don't want to add code to my Team script every time I make a new Team-Based Componet , since that links everything together in a rather messy way

Reasoning

This implementation feels bad just because every time I add a new component that is Team Based, (which I already have about 5-10 of those components and plan on adding many more provided my MVP is actually fun to play), I need to add a new function and list to the Team script.

I think that violates the Single-Responsibility Programming Principle, since my Team class would have many reasons to change.

Why I don’t think I want to implement a sigle list of GameObjects

If I made a single list of Gameobjects that are part of a certain team, I think I would be calling GetComponet a similar number of times to your "bad example"

Reasoning

If I just had a list of Gameobjects, I would need to call GetComponet<> on hundreds of gameobjects to build a new list of the componet I am looking for every time I try to find the Closest Building, Closest Storage Center, Closest Fabricator, etc so forth. If I had to call TryGetComponet<WhateverComponetIWantToGet>() for 50-100 objects on a specific team every time I wanted to find the closest object on a paticular team (which due to the way my game’s AI is set up, should happen once every 10ish frames), that may lead to hundreds of GetComponet<> calls per frame, which is not want we want.

My Own (Also Possibly Flawed) Solution

What I ended up doing was creating an interface with Generics

public interface ITeamBasedObject<Generic>
    where Generic : MonoBehaviour, ITeamBasedObject<Generic>
{
    public TeamObject TeamObject { get; }

    public Team OwnerTeam { get { return TeamObject.owenerTeam; } }

    public static List<Generic> GetAllObjectOnTeam (List<Generic> objects, Team team) 
    {
        List<Generic> returnList = new(objects);

        returnList.RemoveAll(generic => generic.OwnerTeam != team);

        return returnList;
    }

    public static Generic GetClosestObjectOnTeam(List<Generic> objects, Team team, Vector3 position)
    {
        Generic closestGeneric = null;
        float closestDistance = float.PositiveInfinity;

        foreach (Generic generic in GetAllObjectOnTeam(objects, team))
        {
            float distance = Vector3.Distance(generic.transform.position, position);
            if (closestDistance > distance)
            {
                closestGeneric = generic;
                closestDistance = distance;
            }
        };

        return closestGeneric;
    }
}

When I make a new Team-Based Component that I need to compare distances of, I make a static list of all instances of said Componet:

public class ConstructionSite : NetworkBehaviour, ITeamBasedObject<ConstructionSite>
{
    public static List<ConstructionSite> All = new();

    private void OnEnable()
    {
        All.Add(this);
    }

    private void OnDisable()
    {
        All.Remove(this);
    }

//Insert the rest of the code I want to do here

Then, when I need to find the closest instance of a componet, I use the GetClosestObjectOnTeam function to get the nearest componet on my team:

namespace MyGOAP.Unit
{
    public class BuildBuildingSensor : MultiSensorBase
    {
        public BuildBuildingSensor()
        {
            this.AddLocalTargetSensor<BestConstructionSite>((agent, reference, target) =>
            {
                //This only calls GetComponet if I don't already have a reference
                   //to the compponet saved in the reference class.
                ConstructorUnit unit = reference.GetCachedComponent<ConstructorUnit>();
                
                ConstructionSite constructionSite = ITeamBasedObject<ConstructionSite>.GetClosestObjectOnTeam(
                    ConstructionSite.All,
                    unit.TeamObject.owenerTeam,
                    unit.transform.position);
                
                return new IConstructionSiteTarget(constructionSite);
            });
        }
   }
}

Why I Think This Is Good

  1. I only call GetComponet<> once when an Object is enabled.
  2. I don’t violate the Single Responsibility Principle as each script only has one reason to be changed (If I add a new Team-Based Componet, I only modify that particular script)

I feel like you’re forgetting about the component based nature of Unity. To key a game object to a specific team, you only need one component to do this. If needed, other components can reference said component to express a team themselves. Otherwise this is just a lot repeated code.

3 Likes

Exactly what spiney says. I often use Components merely to store object information. Like so:

public class Team : MonoBehaviour
{
    public int TeamIndex {get;private set;}
}

For any object I can then simply use GetComponent<Team>.TeamIndex to get that index. For instance this is useful if you need that information in a time and place where you have the object handed to you, typically physics collision callbacks. It’s really not a performance issue to get the component here in every collision as there’ll hardly be more than a couple dozen per frame at most.

Rather than have a generic ITeamBasedObject<T> you really only need a Team component, and if that exists, it is a team object. And another component ObjectType that contains the type, or even an empty BuildingObject component that simply tags the object of type “Building”. Then you can do without the generics which, if used where unnecessary, typically add more coding overhead(aches).

If at all possible I prefer to keep my components dumb and simple and move any processing code to central classes which enumerator over each instance, rather than individual scripts doing their thing (ie per component callback overhead like Update vs updating many objects from a central class).

Try not to use NetworkBehaviour unless you absolutely HAVE to use it. Because NGO adds callbacks and maintains lists of every NetworkBehaviour in a scene. The more unnecessary NBs you have the more overhead there’ll be in NGO processing.

Secondly, the static list shouldn’t be public or anyone from the outside could modify the list, even new or null it. That’s part of Open-Closed principle but generally encapsulation best practice, hiding internal details.

And in general I avoid static fields as much as possible because it requires extra coding to make it work with disabled domain reload, which allows you to enter playmode instantly. Also static fields break encapsulation because, like a singleton, these become accessible from anywhere by anyone and thus these entangled dependencies have a tendency to become spaghetti code faster than you can spell “static”. :wink:

1 Like

Once again, thanks to you and spiney for the help! Hopefully you can tell that I am (probably very slowly) learning, but thank you both regardless for you continued patience along the way!

So, my solution to the original problem should be to have a list of TeamObject monobehaviors on each team script, and when I need to find the closet object of type X, I would loop through all TeamObjects on my Team’s OwnedObjects list, call .TryGetComponet() on each OwnedObject, and if that function returns true, I run my sorting code on that object, and return the result.

If performance were to become a problem, I would give in and create separate lists for the most commonly compared components (for example, if I needed to find the nearest Building 1,000 times a second, I would make a varible List<Building> OwnedBuildings, add each building to the list when a Team is assigned to the TeamObject componet on the same gameobject, then have a function on the Team script called GetClosestBuilding(Vector position)), as the “violation” of the Single Responsibly Principle is still less than the “violations” my current mess makes.