I’ve created an extension method for Transform, to make it easier to get a 2D Vector of the transform’s position.
However, it’s quite slow and I can’t figure out why.
Here’s the script:
using UnityEngine;
using System.Collections;
public static class TransformExtension
{
public static Vector2 GetPosition2D (this Transform trans)
{
return new Vector2(trans.position.x, trans.position.y);
}
}
How could I make it faster?
Thanks in advance!
I don’t see how this could become any faster. How do you define “slow”? And could it be that the problem lies somewhere else? Maybe you are accessing a large number of transforms and do so in an inefficient manner?
This method is called ~200 / FixedUpdate.
I have tons of small units running around, which use this method.
It has to be called every frame, seeing as they use every frame.
This is the code that the profiler complains about:
public void UpdatePresenter ()
{
Vector2 dir = new Vector2(0,0);
List<Unit> units = UnitController.GetInstance().UnitsInGame;
foreach (Unit unit in units)
{
dir += (transform.GetPosition2D() - unit.transform.GetPosition2D()).normalized *
Mathf.Clamp(1 - Vector2.Distance(transform.GetPosition2D(), unit.transform.GetPosition2D()), 0f, 1f) * 2f;
}
dir *= .1f;
presenter.transform.position = Vector2.Lerp(
presenter.transform.GetPosition2D(),
transform.GetPosition2D() + dir,
Time.deltaTime * 10);
}
Well maybe someone with more experience than me should comment on this, but it looks normal to me. I mean you are really calling the method from a loop quite often and do little else.
Why don’t you cache transform.GetPosition2D()? That would save 3 calls per iteration.
EDIT: Sorry, I misread and did not see the “unit” in front of 2 of them. Still might be worth a shot to avoid fetching a value you already have
Maybe some more time could be saved if you’d only return a value type and save the Vector2 instance, but since you need a vector2 to calculate the distance, it won’t help here at all.
Managed to get it down from ~70 ms to ~45 ms.
This is my new, optimized code.
Tell me if you have any more tips!
public void UpdatePresenter ()
{
// Store 2D positions
Vector2 presenterPos = presenter.transform.GetPosition2D();
Vector2 myPos = transform.GetPosition2D();
Vector2 dir = new Vector2(0,0);
List<Unit> units = UnitController.GetInstance().UnitsInGame;
foreach (Unit unit in units)
{
Vector2 unitPos = unit.transform.GetPosition2D();
float distance = Vector2.Distance(myPos, unitPos);
if (distance < 1)
{
dir += (myPos - unitPos).normalized *
Mathf.Max(1 - distance, 0f) * 2f;
}
}
dir *= .1f;
presenter.transform.position = Vector2.Lerp(
presenterPos,
myPos + dir,
Time.deltaTime * 10);
}
accessing the transform property is quite slow. npfs3000 has conducted some tests some time ago and could significantly (factor 20) speed it up with a manually cached transform. maybe thats also something to consider for you.
edit:
some things jumping my eye but i’m not sure if they affect:
distance < 1 → possibly interpreted as integer and converted to float each iteration. don’t know if compiler optimzes this away but why risk it instead of using 1.0f?
calculating the distance requires a squareroot which is quite slow. better to square the other value and compare it with squared magnitude of vector3.
foreach can potentially be slower. if you have a list use a for loop. set it up properly
Yeah I’d definitely get ride of the foreach. For loops aren’t drastically faster but it’ll save you from the garbage collector.
More to exiguous’ point - you’re actually doing 2 square roots in a redundant calculation. I’d probably store (myPos - unitPos) in a Vector2 and use that instead of calculating Distance() and also doing the vector subtraction.
This also sort of looks like a steering behavior - what is it that you’re trying to do?
You may try that as well:
using UnityEngine;
using System.Collections;
public static class TransformExtension
{
public static Vector2 GetPosition2D (this Transform trans)
{
Vector3 position = trans.position;
return new Vector2(position.x, position.y);
}
}
Like that you can be sure that the position is only computed once. Maybe Unity internally caches the position, but I am not sure about that. It is definitely worth a try.
Vector2 posDiff = (myPos - units[i].transform.GetPosition2D());
float magnitude = posDiff.magnitude;
Vector2 normalized = new Vector2(posDiff.x / magnitude, posDiff.y / magnitude);
if (magnitude < .5f)
{
dir += normalized *
Mathf.Max (.5f - magnitude, 0f) * 6f;
}
Is this what you meant?
Now I use a for loop, instead of foreach, and I only do the squareroot once.
Following all of your tips, this is my new code:
public void UpdatePresenter ()
{
// Store 2D positions
Vector2 presenterPos = presenterTransform.GetPosition2D();
Vector2 myPos = myTransform.GetPosition2D();
Vector2 dir = new Vector2(0,0);
// Get array of units
Unit[] units = UnitController.GetInstance().UnitsInGame.ToArray();
// Loop through all units
for (int i = 0; i < units.Length; i++)
{
// Store difference
Vector2 posDiff = (myPos - units[i].transform.GetPosition2D());
// Calculate mangnitude
float magnitude = posDiff.magnitude;
// Manually calculate normalized vector, to save a square root call
Vector2 normalized = new Vector2(posDiff.x / magnitude, posDiff.y / magnitude);
if (magnitude < .5f)
{
dir += normalized *
Mathf.Max (.5f - magnitude, 0f) * 6f;
}
}
dir *= .1f;
presenterTransform.position = Vector2.Lerp(
presenterPos,
myPos + dir,
Time.deltaTime * 8);
presenterTransform.DepthBasedOnY(.1f);
}
Any further optimizations that can be done?
Thanks for your help! 
Oh, and for you who wondered what it was for, I made a small .mp4:
http://gyazo.com/cb67099dccf27daeb94e51844093246f.mp4
It plays straight in the browser.
It’s the subtle “push” effect you see.
how about the logic itself?
does every unit call this method, and check against all other units in the for loop? i.e. n * (n-1)
if so, then consider the middle for loop for 3 Units ABC
A against A (pointless)
A against B
A against C
B against A (redudant)
B against B (pointless)
B against C
C against A (redudant)
C against B (redudant)
C against C (pointless)
instead only calculate those values once per frame
SomeValue[][] someValueMatrix;
GenerateValueMatrix() {
Unit[] units = // list at UnitController.GetInstance().UnitsInGame
for(int cursor = 0; cursor < units.Length; cursor++) {
Unit unitA = units[cursor];
someValueMatrix[cursor, cursor] = -1; // ignore unit to itself
for(int unitBIndex = cursor + 1; unitBIndex < Units.Length; unitBIndex++) {
Unit unitB = units[unitBIndex];
var d = SomethingSomething(unitA, unitB) // the inner part of the for loop calculation, so calculating the distance or dir or w/e
someValueMatrix[cursor, unitBIndex] = someValueMatrix[unitBIndex, cursor] = d; // relationship from a to b is same as b to a
}
}
}
then in the UpdatePresenter method you can do the for loop like
Unit[] units = UnitController.GetInstance().UnitsInGame.ToArray();
int myIndex = // units own index in the array
// Loop through all units
for (int i = 0; i < units.Length; i++)
{
if(myIndex == i) continue;
dir += someValueMatrix[myIndex, i]; // add the vector or w/e you have stored in the matrix at this point
}
dir *= .1f;
note1, before a moved in response to b, and b moved in response to the updated position of a – now the response is static based on initial a-b position difference per frame
note2, optionally you can check if a unit has moved at all, so if neither a nor b has moved, there is no need to update their relationship value
note3, if you add/remove a unit you have to recreate the matrix accordingly, otherwise just re-initialize it per frame
Thank you for you tips!
About the B → A and A → B, that part is in this case not redundant, as both units should be affected by this. However, A → A is indeed pointless.
A question about note 2, checking if the two units have moved will take a small amount calculations. Will it be worth the cost of it?
Anywho, thank you for all your tips. Very useful indeed!
Since this is a spatial calculation, you should consider implementing a space-partitioning scheme to optimize these types of methods. A quad-tree would be the ideal solution, I think. Your current algorithm is O(n^2), and wont scale well. Partitioning the space will be the next best optimization that you can make. Additionally, you’ll be able to apply your oct/quad-tree implementation to other spatial methods.
+1 this
you can use triggers as well , here is a proof of concept I did some time ago while having a discussion about such things in another thread here on the forums
There are 200 “towers” and 10 objects moving around( one object is controlled with the arrow keys by you )
So by using triggers as a means of a space-partitioning scheme the calculations can be reduced drastically
There are many expressions here that I’m not quire familiar with.
This sounds very good indeed, but I don’t quite understand.
Mind sharing some example code so I could better understand?
Thank you!