I am apparently misunderstanding how else works.

Unity Engine
,
1 
if(Input.GetKeyUp(KeyCode.Mouse1))
        {
            //Debug.Log("Mouseclick");
            Ray ray = cam.ScreenPointToRay(Input.mousePosition);
            RaycastHit hit;

            if(Physics.Raycast(ray, out hit, ClickRange, interactableObject))
            {

                Interactable interactable = hit.collider.GetComponent<Interactable>();
                NPCtest NPCtest = hit.collider.GetComponent<NPCtest>();
                enemyscript enemyscript = hit.collider.GetComponent<enemyscript>();

                selectedTarget = hit.transform;

                if(hit.collider.gameObject.tag == "Enemy")
                {
                    if(selectedTarget != null)
                    {
                        DeselectTarget();
                        if(enemyscript != null)
                        {
                            Enscript = hit.collider.gameObject.GetComponent<enemyscript>();
                            SelectTarget();
                        }
                    }
                }
                else
                {
                    DeselectTarget();
                    Debug.Log("DEEE");
                }
}

Debug.Log(ā€œDEEEā€); under else is not being called.

I understand this to mean ā€œIf raycast does not hit an interactable object within the float defined as clickRangeā€ then DeselectTarget and say DEEE." Clearly Iā€™m wrong. Would someone like to correct my thinking so I can get on to the other 5000 problems Iā€™m having?

The if statement works as intended.

2

That else is connected to this if statement on line 16:
if(hit.collider.gameObject.tag == "Enemy")

So hereā€™s how your logic works:

If your raycast hits any object:

  • if that object has the ā€œEnemyā€ tag, do the stuff between lines 18-26
  • otherwise (it has some other tag), Deselect the target and print ā€œDEEEā€

You currently donā€™t have any code handling the case if the raycast doesnā€™t hit anything at all. if you wanted to do that, youā€™d make an else statement connected to the outer if statement (the one on line 7)

3 
            if(Input.GetKeyUp(KeyCode.Mouse1))
            {
                //Debug.Log("Mouseclick");
                Ray ray = cam.ScreenPointToRay(Input.mousePosition);
                RaycastHit hit;
    
                if(Physics.Raycast(ray, out hit, ClickRange, interactableObject))
                {
                    Interactable interactable = hit.collider.GetComponent<Interactable>();
                    NPCtest NPCtest = hit.collider.GetComponent<NPCtest>();
                    enemyscript enemyscript = hit.collider.GetComponent<enemyscript>();
    
                    selectedTarget = hit.transform;
    
                    if(hit.collider.gameObject.tag == "Enemy")
                    {
                        if(selectedTarget != null)
                        {
                            DeselectTarget();
                            if(enemyscript != null)
                            {
                                Enscript = hit.collider.gameObject.GetComponent<enemyscript>();
                                SelectTarget();
                            }
                        }
                    }
                    else
                    {
                        DeselectTarget();
                        Debug.Log("DEEE");
                    }
                }
                else
                {
                    Debug.Log("Nothing clicked on");
                }
            }
4

Ah yes of courseā€¦ Itā€™s always the most obvious mistakes that confound me for hours.

5

You have some potentially confusing coding conventions as well.

You are naming NPCTest and enemyscript exactly the name of the class and the enemyscript class is lowercase.

Also notice that you are assigning interactable, NPCtest, enemyscript and selectedTarget before you know if it is an ā€œEnemyā€. You donā€™t use those values unless it is an enemy so you can move them inside the if(hit) test. And it doesnā€™t appear that NPCtest is used here.

6

Itā€™s not the full script but if you were to look at the entire thing youā€™d see a lot of unused and redundant lines (hence why I only post relevant parts lol). I usually keep clutter code until I know I definitely wonā€™t be using it, for reference or so I donā€™t accidentally do the same thing twice. It might not be the best way of doing thing but it kinda works for me.

In this case it was just the concept of the ā€˜if and elseā€™ which I needed advice with so I didnā€™t want to people waste their time digging through a few hundred lines of code written by a self learned noob.

7

You have a great deal of redundant indentation too. Since all of those if statements just skip the rest of the code if you swap the condition you end up with a smaller block and one less indentation which reduces the likelihood of errors. Line 16 is a little more complex thanks to combining two if statements but itā€™s the same idea as the others.

if (Input.GetKeyUp(KeyCode.Mouse1))
{
    var ray = cam.ScreenPointToRay(Input.mousePosition);
    RaycastHit hit;
    if (!Physics.Raycast(ray, out hit, ClickRange, interactableObject))
    {
        Debug.Log("Nothing clicked on.");
        return;
    }

    var interactable = hit.collider.GetComponent<Interactable>();
    var npcTest = hit.collider.GetComponent<NPCTest>();
    var enemyScript = hit.collider.GetComponent<enemyscript>();

    selectedTarget = hit.transform;

    if (hit.collider.gameObject.tag != "Enemy" || selectedTarget == null)
    {
        DeselectTarget();
        Debug.Log("DEEE");
        return;
    }

    DeselectTarget();
    if (enemyScript == null) return;

    Enscript = hit.collider.gameObject.GetComponent<enemyscript>();
    SelectTarget();
}
8

If it helps, this will never really go away. Iā€™ve been coding for 35-ish years. There are always ups and downs and itā€™s super fun except when itā€™s not.

9

So true, so true. Anyone who says they have mastered realtime interactive entertainment software engineering is simply mistaken.

It seems the main benefit to diligent study and application of self is that you minimize the amount of time you flop around like a dead fish wondering what on earth is wrong.

This is why I am such a HUGE proponent of keeping it simpleā€¦ simple simple simple simple, and then even simpler than that. One thing per line, ideally never indent deeper than you must, etc.

It has to be so simple that you can explain it to a dog. See what Iā€™m doing above in my avatar picture?

Note the code rewrite that @Ryiah posted above. It is the CLASSIC linear simple example of top-down flow, with early-outs at line 8, line 21 and line 25ā€¦ so clean!

As you read it you can immediately see where ā€œWeā€™re done here!ā€ happens, and all you do is move to the next line down the page if youā€™re not done.

Itā€™s like passing highway exitsā€¦ once youā€™re past it you know you have accomplished something and are now ready for the next step of the data cascade.

Also of note is that attaching the debugger is SUPER easy here: just set a breakpoint right after an early out that you care about and your code will only break when the condition you actually care about happens.