Correct way to use if statements in C#?

I am an extreme noob who is in my first week of Unity & C# and in between reading and watching videos I have been trying to make myself a simple archer.

I have some code that I have been working on:

public class ArrowShooter : MonoBehaviour {
   
private GameObject arrowPrefab;


void Start () {

arrowPrefab = Resources.Load ("arrow") as GameObject;
            }

void Update ()
    {

        if (Input.GetMouseButtonDown (1)) {

            GameObject newArrow = Instantiate (arrowPrefab) as GameObject;

            newArrow.transform.position = transform.position;

            Rigidbody rb = newArrow.GetComponent<Rigidbody> ();

            rb.isKinematic = true;

            if (Input.GetMouseButtonDown (0)) {

                rb.isKinematic = false;

                rb.velocity = Camera.main.transform.forward * 30;

            }

        }
    }

I can get the arrow to fire if I leave out the isKinematic = true and the nested if statement and just set the velocity.

Otherwise the arrow is instantiated in the right place but is not parented to the object the script is on and will not fire.

I can see myself that the logic looks all wrong. I initially made two functions ArrowSpawner and ArrowFire like so:

public class ArrowShooter : MonoBehaviour {
   
private GameObject arrowPrefab;


void Start () {

arrowPrefab = Resources.Load ("arrow") as GameObject;
            }

void ArrowSpawner ()
    {

        if (Input.GetMouseButtonDown (1)) {

            GameObject newArrow = Instantiate (arrowPrefab) as GameObject;

            newArrow.transform.position = transform.position;

            Rigidbody rb = newArrow.GetComponent<Rigidbody> ();

            rb.isKinematic = true;

        }
    }
           
void ArrowFire () {

        if (Input.GetMouseButtonDown (0)) {

            rb.isKinematic = false;

            rb.velocity = Camera.main.transform.forward * 30;

            }

        }
    }

But ArrowFire couldn’t find the rigidbody from ArrowSpawner. So I tried to define the rigidbody in void Start (), which allowed me to compile but did nothing in the game. I thought this was because I was missing void Update () which led me back to the first piece of code.

I know I need to do a lot more reading before I can make any meaningful progress scripting. But can someone tell me the correct way to say:

if (Input.GetMouseButtonDown (1)) the rigidbody is kinematic (& parented to the game object the script is attached to)

then when: (Input.GetMouseButtonDown (0)) the rigidbody in not kinematic and finally apply the velocity Camera.main.transform.forward * 30.

Thanks for taking the time to read this. I hope I was clear in my question.

Stick a Debug.Break() statement after your
GameObject newArrow = Instantiate (arrowPrefab) as GameObject;

Run your program in Unity. It will pause soon after you instantiate your arrow.

Go into the Unity editor and find the Arrow object that you just instantiated. Check if there is a RigidBody component on it.
There probably won’t be.

It sounds like your prefab is bad. The above will let you spot that.

1 Like

Hi Peter,

Thanks for your reply, there is definitely a rigidbody on the instantiated clone of the prefab, have confirmed by selecting a clone in the scene.

With the script I am using at the moment (the top one in my OP) the clones are instantiated in the correct place (transform of the object the script is on) and have the isKinematic = true check box checked on the rigidbody.

Its just that then when I left click the velocity + isKinematic = false is not being applied, and also they just float in mid air as if they are not parented to the game object the script is on.

Is the issue not the way in which I am trying to call the 2nd ifstatement which includes the instruction to fire? This is the bit unity is ignoring and I thought it was down to me structuring the ifstatement incorrectly due to not knowing the correct order in which to put the code.

Is this how you would handle 2 different mouse inputs with actions that follow on from one another?

No, that’s not the way I would do it.

Having this
if (Input.GetMouseButtonDown (1)) {

in Update() looks pretty dangerous. Update gets called on each frame - so 30-60 times a second. And you instantiate a new arrow if the mouse button is held down. It looks like you would be instantiating a ton of arrows for a single mouse click.

I would probably look at using an EventTrigger for this. Add an EventTrigger object to your archer. Use the PointerClick event and provide a callback function. That function can check if there’s an arrow present or not. If not, it creates an arrow. If yes, it fires the arrow that’s there.

=====================

I was just trying to help you with your statement that…

But ArrowFire couldn’t find the rigidbody from ArrowSpawner.

If that’s true, then this line

Rigidbody rb = newArrow.GetComponent ();

Should be giving you Component Not Found exceptions (I think that’s what they’re called) that show up in the log as serious errors and pause your execution. Not sure if that’s happening.

===========================

Not sure if you’re clear on what isKinematic does. isKinematic = true means that you can reposition and move the object in code. isKinematic = false means the object will be subject to physics and will move by itself. Changing the isKinematic setting is going to have NO effect on parenting.

Hi again Peter,

Again thanks for taking the time to read over and respond, I don’t seem to be explaining very well so I apologise.

Also thanks for the tip about EventTrigger, I have done some reading last night and this morning on your suggestion and will definitely look into using it when I am a bit more au fait with the basic logic in C#.

The GetMouseButtonDown thing only happens when an individual click is registered, I tried GetMouseButton first which gave me the aforementioned million arrows per click!

The error I got when trying the 2 separate functions was that the ArrowFire - Rigidbody rb was not defined, but when I tried to define it within the function I got a different error saying it couldn’t define it twice. When I defined it in void Start the script compiled but it didn’t do anything in game. I don’t remember the exact wording of the error so again apologies.

The reason I am using isKinematic = true when I instantiate is so that the rigidbody of the arrow is not affected by gravity, if I don’t do this, it simply falls away the moment it is instantiated.

I have set up two separate scripts now which sort of achieves the desired effects but I am still having some issues. New scripts are one to spawn the arrow and one to shoot the arrow ArrowSpawner and ArrowShot:

ArrowSpawner:

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

public class ArrowSpawner : MonoBehaviour {

    private GameObject arrowPrefab;

    // Use this for initialization
    void Start () {

        arrowPrefab = Resources.Load ("arrow") as GameObject;
    }
  
    // Update is called once per frame
    void Update () {

        if (Input.GetMouseButtonDown (1)) {

            GameObject newArrow = Instantiate (arrowPrefab) as GameObject;

            newArrow.transform.position = transform.position;

            newArrow.transform.rotation = transform.rotation;

            newArrow.transform.parent = transform.parent;

            Destroy (newArrow, 5);

            Rigidbody rb = newArrow.GetComponent<Rigidbody> ();

            rb.isKinematic = true;
      
    }
}

}

Then ArrowShot:

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

public class ArrowShot : MonoBehaviour {

    private GameObject arrowClone;

    // Use this for initialization
    void Start () {

    }
  
    // Update is called once per frame
    void Update () {

        if (Input.GetMouseButtonDown (0)) {

            arrowClone = GameObject.Find ("arrow(Clone)");

            arrowClone.transform.parent = null;

arrowClone.name = "inflight";

            Rigidbody rc = arrowClone.GetComponent<Rigidbody> ();

            rc.isKinematic = false;

            rc.velocity = Camera.main.transform.forward * 30;
      
    }
}
}

The issues I am now having are:

  1. Had to set a destroy timer on the instantiated arrow, because the script to fire the arrow doesn’t work if there is more than one “arrow(Clone)” in the scene.

I need a way of ArrowShot getting the rigidbody of newArrow rather than arrow(Clone) so I don’t have to destroy the previous arrow before I can fire again.

  1. Even though I set the rotation of the instantiated arrow, it stays in one orientation. So I can drag it around attached to my bow and fire it in the right direction, but it is always facing the way it was spawned.

Really confused at this. I have also tried transform.rotation = transform.parent.rotation, but it still does the same thing.

  1. Some clarification as to whether this is the right way to approach the issue of multiple if statements that follow on from each other. I have googled and there is no such thing as an if/then statement in unity. I am pretty sure I am missing something blatantly obvious. Any issue in having 2 scripts rather than one to complete this single task?

EDIT
Fixed issue no.1 by calling arrow(Clone).rename on the ArrowShot script and renaming it to arrowflight. Added this to the script.

Now I just need to fix the rotation issue of the instantiated arrow so it turns with the direction of the camera.

I don’t know what you mean by “there is no such thing as an if/then statement in unity”. Of course there is; you’re using it in the scripts above.

I’m not clear on what your confusion about the if statement is. I do note that your original script had the second “if” statement inside the first, which doesn’t seem to be what you want. Move it down after the end of the first if block, and you should be all set.

Indeed, the likelihood of of getting MouseButtonDown for both buttons in 1 frame is very tiny, I think.

Also, @peterk1968 : MouseButtonDown is only the first frame the button is pushed down.