Average position of a set of GameObjects

Unity Engine
1

Hi I am trying to familiarize myself with Unity, I want to get the average position of a list of objects.

Currently I have two 2D objects and I am trying to find the average position, I am using DrawRay to view visually. I expect the two lines to meet between the two objects - but they dont…

Update Code:

           //list of positions to average
            List<Vector3> positions = new List<Vector3>();
 
            //get some objects
            GameObject[] arr = GameObject.FindGameObjectsWithTag("test");
 
            //add objects positions to list
            for (var i = 0; i < arr.Length; i++) {
                positions.Add (arr [i].transform.position);
            }
 
            //get the average of the objects in positions
            Vector3 avg = GetMeanVector(positions);
 
            //draw line from center of current gameObject to average position.
            Debug.DrawRay(transform.position, avg, Color.red);
 
            Debug.Log ("AV" + avg + "RL" + transform.position);

The GetMeanVector function found here, which looks ok:Find the average of 10 Vectors - Questions & Answers - Unity Discussions

       private Vector3 GetMeanVector(List<Vector3> positions){
            if (positions.Count == 0)
                return Vector3.zero;
         
            float x = 0f;
            float y = 0f;
            float z = 0f;
 
            foreach (Vector3 pos in positions)
            {
                x += pos.x;
                y += pos.y;
                z += pos.z;
            }
            return new Vector3(x / positions.Count, y / positions.Count, z / positions.Count);
        }

I have some screenshots of the result.
This screenshot shows the rays in the scene view.

This screenshot prints the positions.

Any help appreciated thanks. Also, I should mention I asked this in Unity answers, but it is taking all day for moderation…

2

Everything seems good except for the ray draw. The second argument isn’t an end point, it’s a direction; that’s why both drawn rays go up by ~1.3 and left by ~0.2 from each object.

When drawing the ray, use

Debug.DrawRay(transform.position, avg - transform.position, Color.red);

instead to convert the position avg into a direction from the reference point transform.position, or just use Debug.DrawLine().

1 Like
3

Also fyi, you can shorten that averaging function a bit by doing the operations directly on a Vector3.

private Vector3 GetMeanVector(List<Vector3> positions)
{
    if(positions.Count == 0)
    {
        return Vector3.zero;
    }

    Vector3 meanVector = Vector3.zero;

    foreach(Vector3 pos in positions)
    {
        meanVector += pos;
    }

    return (meanVector / positions.Count);
}
2 Likes
4

Also 1 meaningless (and cpu/ram consuming) task is done by having both of these and doing conversion

List<Vector3> positions = new List<Vector3>();
GameObject[] arr = GameObject.FindGameObjectsWithTag("test");

Instead remove the List entirely and calculate average from the “arr” directly.

5 
private Vector3 GetMeanVector(List<Vector3> positions)
{
    Vector3 meanVector = Vector3.zero;
    foreach(Vector3 pos in positions)
    {
        meanVector += pos;
    }
    return (positions.Count == 0 ? meanVector : meanVector / positions.Count);
}

Slightly smaller.

6

This is really ugly, hard to read and just over complicates things. You should use a guard clause / early exit as LiterallyJeff did.

    private Vector3 GetMeanVector(List<Vector3> positions)
    {
        if (positions == null || positions.Count == 0)
            return Vector3.zero;
        Vector3 meanVector = Vector3.zero;
        foreach(Vector3 pos in positions)
            meanVector += pos;
        return meanVector / positions.Count;
    }

This is much more readable. Code being short is not necessarily any better. You can make your code even “shorter” by removing all that unnecessary whitespace:

private Vector3 GetMeanVector(List<Vector3> positions){Vector3 meanVector=Vector3.zero;foreach(Vector3 pos in positions)meanVector+=pos;return(positions.Count==0?meanVector:meanVector/positions.Count);}

Is that better? No, it’s not ^^.

If you’re just looking for short code, you could just use Linq Aggregate and Select to do the final division.

2 Likes
7

As Bunny points out, don’t do this to your code. It’s a well-understood noob reflexive mistake to combine code into one line. Don’t fall for it.

If you have more than one or two dots (.) in a single statement, you’re just being mean to yourself.

How to break down hairy lines of code:

http://plbm.com/?p=248

Break it up, practice social distancing in your code, one thing per line please.

“Programming is hard enough without making it harder for ourselves.” - angrypenguin on Unity3D forums

“Combining a bunch of stuff into one line always feels satisfying, but it’s always a PITA to debug.” - Star Manta on the Unity3D forums

2 Likes
8

It’s a well-understood noob reflexive mistake to combine code into one line. Don’t fall for it.

Jeez mate, theres people on the end of the telephone. Go easy…
I was bundling it in a utility class and figured maybe I could help someone - theres more than one use case.

Good points, but sometimes honey over vingear eh…

2 Likes
9

I think people are more annoyed that you necroed a 6 year old thread without really adding anything to it. Since it was already solved, it may have been better to create your own thread with the prefix Showcase instead, as it doesn’t unnecessarily ping everyone in the thread who hasn’t looked at it in 6 years.

2 Likes
10

The problem of packing everything into a few lines is so common, so pernicious, so much of a colossal waste of everybody’s time, that my entire post above is from a text file I keep of common “oh here we go again” responses to bad practices.

It’s just copy/pasted advice.

And it works. Break it up. Social distancing please.

Don’t read emotion into technical advice. (That’s another noob mistake, while we’re breaking eggs… :slight_smile: )

And don’t necro-post. That’s straight from forum rules.

But welcome to the forum nonetheless!

1 Like
11

Ah ok - I didn’t realise everyone got pinged. My mistake there - thanks for pointing that out. Will keep a bit quieter on these things…!

12

Yeah, good point - and I should draw a line between your points and how you package them - not getting emotional is a better way to be better.

Some of this stuff doesn’t always work - like DOTween has a ton of fullstops packed into a single line - it’s common to find a caveat in a ruleset.
Being able to read a slightly convoluted ternary statement quickly can have some merits - its common - imperfect code is common, being able to work within that space can have some merits.

If you’re solo coding and trying to make a small class - to explain myself away… I had seen that someone else might be trying to do the same thing and were looking for a smaller snippet… maybe i could of been of some help. Earnestly it was me just thinking maybe someone could be in the same spot - I didn’t realise everyone else had gotten notified, didn’t realise the rules about necromancy…
But i don’t know how well i can defend myself - you both have far better points than me… It was more the framing than the content.

Considering we joined this forum on the same year - welcome to you as well!
Thanks for the introduction - I’ll see myself out.

2 Likes