Spawn At Mouse Position

[ryanren]

Hi,

I recently just finished the lecture Spawn At Mouse Position.

In the video, Rick uses an “inception” kind of way that like the code below.

public class DefenderSpawner : MonoBehaviour
{

    [SerializeField] GameObject defender;

    private void OnMouseDown()
    {
        SpawnDefender(GetSquareClicked());
    }

    private Vector2 GetSquareClicked()
    {
        Vector2 clickPos = new Vector2(Input.mousePosition.x, Input.mousePosition.y);
        Vector2 worldPos = Camera.main.ScreenToWorldPoint(clickPos);
        return worldPos;
    }

    private void SpawnDefender(Vector2 worldPos)
    {
        GameObject newDefender = Instantiate(defender, worldPos, Quaternion.identity) as GameObject;
    }

}

I solved the problem by just plugging the GetSquaredClicked method like this

public class DefenderSpawner : MonoBehaviour
{

    [SerializeField] GameObject defender;

    private void OnMouseDown()
    {
        SpawnDefender();
    }

    private Vector2 GetSquareClicked()
    {
        Vector2 clickPos = new Vector2(Input.mousePosition.x, Input.mousePosition.y);
        Vector2 worldPos = Camera.main.ScreenToWorldPoint(clickPos);
        return worldPos;
    }

    private void SpawnDefender()
    {
        GameObject newDefender = Instantiate(defender, GetSquareClicked(), Quaternion.identity) as GameObject;
    }

}

Could you explain the advantage of using RIck’s way than the simple and easy just plugging the method into SpawnDefender?
Is there any long-term disadvantage using the second way?

[Yee]

There’s no real benefit or drawback on doing it either way, it all comes down to preference, but I must say, Rick’s method is better from a readable standpoint. Let me explain:

Instantiate asks for a position, yet your method is called GetSquareClicked, the name implies that is not a position what you are getting but a square. This might sound nitpicky, but if you were sharing your code with someone it might get confusing, it might also be confusing for you in the long run, especially if you stopped working on this project for a long time. Naming conventions are one of those things that truly sets apart a good programmer from a bad one.

So, the only thing you would need to do to make your code better, is to change the name of GetSquareClicked function to something like GetSquarePosition.

[Edit]
Almost forgot to mention, the position of the functions should be reversed.
Imagine you have never seen the code:

Rick’s way

• OnMouseDown leads to SpawnDefender which sends something from GetSquareClicked.
• Scroll down. You can see GetSquareClicked sends a Vector2.
• Now everything makes sense in Spawn Defender.

With your way, as it is

• OnMouseDown leads to SpawnDefender.
• You see GetSquareClicked, What is this for? Who knows.
• You read all of SpawnDefender and see that Instantiate is using the method you just read.

Again, nitpicky. But order matters when reading things. Your code should be as readable as a kid’s book. If you are a beginner don’t focus too much on that, just keep it in mind when you start writing more complex scripts.