Why do I get this error?

Started by
6 comments, last by Cestarian 1 year, 1 month ago

I'm trying to make an object in-game follow the mouse, using the mouse sort of like an analogue stick, although I wouldn't say I'm very far along with it I've run into a problem I can't seem to figure out already.

Here's the code:
using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class FemaleActor : MonoBehaviour

{
private bool inMotion;

void Start()
{
float mousePosX = 0f;
float mousePosY = 0f;
}

void Update()
{
var x = Input.GetAxis ("Mouse X");
var y = Input.GetAxis ("Mouse Y");

float mousePosX = x + mousePosX;
float mousePosY = y + mousePosY;

transform.position += new Vector3(0f, mousePosY, mousePosX) * Time.deltaTime;
}
}

And here's the error:

The variable 'mousePosX' is assigned but its value is never used
The variable 'mousePosY' is assigned but its value is never used

Use of unassigned local variable 'mousePosX'
Use of unassigned local variable 'mousePosY'

The particular problem is the ‘use of unassigned local variable’ compiler error. As you can see the warnings are contradictory, mousePosX and mousePosY are assigned the value of 0f in Start() but when called upon in Update() it's like the assignment has been forgotten even when it, as far as I know, shouldn't be.

Advertisement
void Start()
{
float mousePosX = 0f;
float mousePosY = 0f;
}

Those are “local variables”. They are only valid inside the Start-function. You need to add them to the class-scope, like “private bool inMotion”. Then you can access them without the “float” type declaration inside the functions:

mousePosX = x + mousePosX;
mousePosY = y + mousePosY;

You don't even need to assign = 0 in start, this is done automatically upon class construction.

public class FemaleActor : MonoBehaviour
{
private bool inMotion;
private float mousePosX;
private float mousePosY;
private float axisY;
private float axisX;

void Update()
{

axisX = Input.GetAxis ("Mouse X");
axisY = Input.GetAxis ("Mouse Y");
mousePosX += axisX;
mousePosY += axisY;

transform.position += new Vector3(0f, mousePosY, mousePosX) * Time.deltaTime;
}

}


@Juliean
I see, so I just have to set it up like this, not that it actually works like I wanted it to, the movement speed is way too fast :')

Thanks for your help

Cestarian said:
I see, so I just have to set it up like this

Not entirely.

The disadvantage of variables in the class scope is that any function can use and change them. So if you write a lot of code in a lot of functions in this class, you'll end up with many many variables in the class scope and it becomes a nightmare to understand what variable is used where (and for what purpose).

So as a general rule, variables that are used in one function only AND where you don't need the previous value, should be made local to that function.

(Basically, it's about trying to reduce the amount of code that can access a variable as much as possible.)

For “mousePosX” (and “mousePosY” too), you only use it in Update(), but the “+=” means “add the value at the right to the previous value of the variable at the left, and store it again”. That is, you use the previous value of the “mousePosXY" variables, so they should NOT be local to the Update function. In other words, they are correctly placed currently.

For the “axisX” and “axisY” variables, they are also only used in Update, but the first thing you do is assign them a new value from the Input. So you don't need the previous value at all. These variables can be made local to Update.

I am not sure about the “inMotion” variable.

@Alberth The inmotion variable is not used yet in the above code, it is now.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class FemaleActor : MonoBehaviour
{
    private bool inMotion;
    [SerializeField] private float wait = 0.5f;
    private bool mouseIdle;
    private float mousePosX;
    private float mousePosY;
    private float axisY;
    private float axisX;
    private float timer;
    Vector3 originalPos;
    void Start()
    {
        originalPos=gameObject.transform.position;  
    }

    void Update()
    {
     if ( axisX == Input.GetAxis ("Mouse X") && axisY == Input.GetAxis ("Mouse Y"))
     {
        mouseIdle=true;
        timer += Time.deltaTime % 60;
        inMotion=false;
     }
     else
     {
        axisX = Input.GetAxis ("Mouse X");
        axisY = Input.GetAxis ("Mouse Y");
        mouseIdle=false;
        timer=0;
        inMotion=true;
     }

     mousePosX = axisX;
     mousePosY = axisY;

    transform.position += new Vector3(0f, mousePosY, mousePosX) * Time.deltaTime;

    if (mouseIdle && timer > wait)
    {
        transform.position = Vector3.MoveTowards(transform.position, originalPos, (wait/100*timer));
        inMotion = transform.position != originalPos;
    }

    }

    public bool InMotion()
    {
        return inMotion;
    }
}

So as a general rule, variables that are used in one function only AND where you don't need the previous value, should be made local to that function.

Why? I'd have thought that it's better to initialize the variable only once (e.g. in the class or in Start()) for performance reasons, especially for functions that get run very frequently such as Update(), in fact I'd think you'd never want to initialize a variable in Update().

Sure if it's something in a function that's not used frequently like Start() then of course i'd initialize it there, but that's not the case with any of the variables here, they're all bound for Update() one way or another.

The only things I could initialize in Update() are mouseposx and mouseposy; and that is only because they are actually superfluous as is (they won't remain so though, which is why they're still there), but let's' say i use them as is, and I could indeed initialize them in Update(), should I really?

Clean code is good and all but not if performance suffers for it (if you choose human readability over performance consistently it'll add up pretty fast after all). or am I wrong and is it cheaper to create a new variable than to set an existing one?

Cestarian said:
Why? I'd have thought that it's better to initialize the variable only once (e.g. in the class or in Start()) for performance reasons

Assignment of a variable is exactly the same thing as initialization for a computer, you give the variable a value. It's perfectly fine if you initialize a local variable by the value it should have anyway:

You changed the code, but in the version above my first reply, you could have written

float axisX = Input.GetAxis ("Mouse X");

Cestarian said:
Clean code is good and all but not if performance suffers for it (if you choose human readability over performance consistently it'll add up pretty fast after all).

Your ideas don't match with reality here.

Performance is important, but small fish like an assignment to a float is not going to have noticeable impact at all, not in the last place because the C# compiler removes such inefficiencies for you. (In other words, while you wrote the code, the computer never performed it because it deduced it wasn't needed for the result.)

Even if the C# compiler would not remove those, you're talking about a few nano-seconds gain at most for an eliminated assignment, good luck with that if you need to improve performance a few 1/10th of second. (To give an idea, assume 10 nano-seconds gain for 1 eliminated assignment, so for 1/10th of a second performance gain you need thus to remove 1.000.000.000 / 10 / 10 = 10.000.000 assignments. I have been programming for 50+ years and have never written a program with 10 million lines of code. Now imagine you need to improve by 30 minutes or an hour or more.

You win the war on performance by understanding how the code behaves while it is being executed. Together with benchmarking tools you can then find the troublesome spot, and fix things locally there. The key here is **understanding**. Without it you're just wandering aimlessly and you'll never hit the right spot and have an impact.

This is why human readability is the primary aim. If you can't easily read the code it becomes impossible to grasp how it works because your entire mind is bogged down into deciding what each statement is doing rather than seeing the big picture of how the code works. It's like trying to find a path on a map from town A to town B, except you're reading the map at 1 cm distance so you never see more than 1-2 streets at the same time. It becomes impossible to obtain a general direction of the path or even find which towns you generally pass through. All the high level information that you need for understanding drowns in the (almost) infinite number of street names that you see.

So aim for human readability, and don't worry much about performance until you find it is needed. Understanding the code is key so you can look at the map at a comfortable distance, intuitively understand the code, and find a good path.

@Alberth I see, thank you for your insights, I didn't know the compiler countered inefficiencies like that. And the human readability to make performance optimizations easier makes sense too.

I like to try for both at the same time though usually. Having a bunch of variables initialized at the start of code isn't out of the norm it's a fairly common practice so in that regard i wasn't worried about it from a readabiliity perspective. My original point of view which remains unchanged though is that if you make a habit of making inefficient code on purpose it'll quickly add up once you have some thousands of lines written out.

All it really takes to fuck your performance in my experience is one bad line. Now I don't know how unity is set up in regards to multithreading but if it is then maybe it'll take a bit more than that but I'm sure you get the idea, I'm sure you've experienced an unoptimized loop that takes a couple seconds when it was expected to take less than 1. And since the update function is just that, a loop, I always like to be extremely careful with what I put in there in particular.

But thakn you for opening my eyes to the fact that initializing variables is inexpensive enough not to worry about it, and that the compiler mitigates any performance impact it could have effectively ?

This topic is closed to new replies.

Advertisement