Script efficiency between two methods

Unity Engine
, , ,
1

I have a function that throws a spell (gameObject) from a origin point to a target point. When the spell reaches the ground, it’s checked if the spell’s position is equal to ground’s position. I would like to know which of methods below is most efficient and faster.
Thanks in advance.

private void OnTriggerEnter(Collider collider)
    {
        if(collider.gameObject.layer == TARGET_LAYER){
            spellActionCollider.radius = 6f;
            OnCollided?.Invoke(this, EventArgs.Empty);
            Invoke(nameof(SpellActionEffect), 2f);
        }
    }

    private void CheckHit()
    {
        if(transform.position == target){
            spellActionCollider.radius = 6f;
            OnCollided?.Invoke(this, EventArgs.Empty);
            Invoke(nameof(SpellActionEffect), 2f);
        }
    }
2

Well, since we don’t know the conditions under which the second method would get called we have no way of answering that. The profiler however would answer that for you with excrutiating detail. My gut instinct is to just stick with the simplest solution until you know it’s causing performance issues. At that point you’d be armed with the knowledge of what needs to change to fix it anyway.

Regardless, I want to point out that comparing an exact position with another is a recipie for bugs. It’s tantamount to comparing floats for equality and comparing floats for equality is a bad idea except for some very specific conditions.

A safer way to do it would be to get the difference between the target position and the actual position and then make sure the magnitude of that difference is under some reasonably small threshold of error.

EDIT: I just doubled checked since it seemed odd they’d even supply an equality comparison operator (its never once occurred to me that it even existed). It does appear that Unity actually has implemented an approximate check for comparing Vector3s so i’s actually is fine. The only reason to avoid it would be if you wanted a different threshold of error.

3 Likes
3

Your CheckHit method is potentially more performant assuming you’re moving the spell with Vector3.MoveTowards or similar as it means it doesn’t need a collider and doesn’t need the physics engine to constantly check for a collision every frame.

4

Efficiency and speed are two different things. Efficiency can be about, memory allocations, precision, easier to test, easier to debug and other things, among them performance too.

We cannot judge which method is more efficient, for many reasons:

We don’t know when the CheckHit method is called and how often, every each update? every 10 updates? every time some other condition is true?

We also don’t know if you use the physics system in your game. If you don’t, then adding it just for the sake of the first method, won’t make the method slower but will make your game as a whole slower.

We also don’t know the performance hit of the things you do inside each method. If the first method call takes 5ns and the second 1ns the first seems to be five times slower but if the things inside the methods take 5us, then the difference is negligible 5.005us vs 5.001us, less than 0.1%

We also don’t know, how the rest of the code in your game is. Something else maybe the bottleneck, for example both methods may wait for something to finish so which is faster doesn’t matter as both will take the same amount of time waiting for this thing to finish.

In the end, only you can answer these questions, not only which method is faster, but also if this speed actually makes a difference in the game those methods are implemented. Use the profiler: https://docs.unity3d.com/Manual/Profiler.html and the performance testing package for the test framework: https://docs.unity3d.com/Packages/com.unity.test-framework.performance@3.0/manual/index.html to find out, but do these only if you feel there are actually performance problems with you game.

3 Likes
5

I had to double-check this because I couldn’t believe Unity would make such an error, but it’s true and it’s poor programming practice.

Overloading the == operator for floats or Vector3s in this way is mathematically unsound and can lead to serious bugs.

We know that checking equality with floats isn’t reliable due to precision issues, so instead, we check if values are “close enough.” However, this approximation IS NOT true equality.

Equality has a transitive property: if a == b and b == c, then a == c. By overloading the == operator, Unity has created a concept of equality that lacks this transitive property. Anyone who hasn’t read this specific implementation in the Unity manual, unaware that “equals” here doesn’t really mean equals might end up confused when a != c in his code.

This is precisely why floating-point numbers don’t override the == operator to check for “near enough” but instead leave this decision to the programmers.

To anyone reading this, please explicitly check whether the magnitude of the difference between the target position and the actual position is less than a set tolerance, rather than using the == operator. Doing so can save you, and any other developer reading your code, a significant amount of debugging time.

2 Likes
6

I dont think it matters that much but on trigger enter seems like the way to go.

7

Here is the whole code:

public class SpellAction : MonoBehaviour
{
    public event EventHandler OnCollided;

    private const int TARGET_LAYER = 8;

    [SerializeField] private MagicItemSO magicItemSO;
    private SphereCollider spellActionCollider;   
    private Vector3 origin;
    private Vector3 target;
    private Transform handlePoint;
    private float interpolateAmount;

    private void Start()
    {
        spellActionCollider = GetComponent<SphereCollider>();
        origin = transform.position;
        target = GetComponentInParent<MagicCrafter>().GetTarget().transform.position;
        handlePoint = GetComponentInParent<MagicCrafter>().GetHandlePoint();
    }

    private void Update()
    {
        interpolateAmount += Time.deltaTime;

        SpellActionMove(origin, handlePoint.position, target, interpolateAmount);
        CheckHit();
    }

    private void SpellActionMove(Vector3 a, Vector3 b, Vector3 c, float t)
    {
        Vector3 ab = Vector3.Lerp(a, b, t);
        Vector3 bc = Vector3.Lerp(b, c, t);

        transform.position = Vector3.Lerp(ab, bc, interpolateAmount);
        transform.rotation = Quaternion.LookRotation(target - transform.position, Vector3.up);
    }

    private void SpellActionEffect()
    {
        GetComponentInParent<MagicCrafter>().DestroySpell();
    }

    /*private void OnTriggerEnter(Collider collider)
    {
        if(collider.gameObject.layer == TARGET_LAYER){
            spellActionCollider.radius = 6f;
            OnCollided?.Invoke(this, EventArgs.Empty);
            Invoke(nameof(SpellActionEffect), 2f);
        }
    }*/

    private void CheckHit()
    {
        if(transform.position == target){
            spellActionCollider.radius = 6f;
            OnCollided?.Invoke(this, EventArgs.Empty);
            Invoke(nameof(SpellActionEffect), 2f);
        }
    }
}
8

Because you’re lerping the spell to the target you can simplify the CheckHit like this:

private void CheckHit()
{
	if (interpolateAmount>1)
	{
		spellActionCollider.radius = 6f;
		OnCollided?.Invoke(this, EventArgs.Empty);
		Invoke(nameof(SpellActionEffect), 2f);
	}
}

Or you can even get rid of the CheckHit and Invoke the SpellActionEffect etc to happen a few seconds after casting.

2 Likes
9

In this case I’d say the Check hit version is likely faster since it avoids the need to query broad phase or perform any specific collision calculations in order to see if the object has collided with anything other than the target.

The real question to ask here is: Does it give you the result you want? If it’s working the way you want the effect to behave and isn’t causing issues in gameplay logic or framerate, then you’re all done!

1 Like
10

Yes, it’s working great. Thank you very much to everyone for the support :smiley: