Basic Beginner Unity Project with a bouncing corona

Question

At first I wasn't really proud of this one, but then I decided that I need to know what's so bad about it? Why does it feel unsatisfying? How can I do it better?

So yeah here's a unity project that does the following:

Spawns a maximum of 3 wall bouncing corona balls where the player clicks, which when they collide with each other they heal some health, and if with a wall (screen edges) they loose health. I also tried to do so their alpha value changes in proportion with its health (the closer it is to the death, the more transparent it is). There is a text showing how many time the corona balls bounced of the walls. One last thing, it plays a cough sound on collision.

Here are the scripts I have and which gameObject are they attached to:

Basic script I found online, I remember reading and understanding it before writing on my own, can't recall if I modified it though. Its attached to the camera obviously:

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

// script to instantiate edge colliders at start
public class edges : MonoBehaviour
{
    public PhysicsMaterial2D bouncy;
    void Awake()
    {
        AddCollider();
    }

    void AddCollider()
    {
        if (Camera.main == null) { Debug.LogError("Camera.main not found, failed to create edge colliders"); return; }

        var cam = Camera.main;
        if (!cam.orthographic) { Debug.LogError("Camera.main is not Orthographic, failed to create edge colliders"); return; }

        var bottomLeft = (Vector2)cam.ScreenToWorldPoint(new Vector3(0, 0, cam.nearClipPlane));
        var topLeft = (Vector2)cam.ScreenToWorldPoint(new Vector3(0, cam.pixelHeight, cam.nearClipPlane));
        var topRight = (Vector2)cam.ScreenToWorldPoint(new Vector3(cam.pixelWidth, cam.pixelHeight, cam.nearClipPlane));
        var bottomRight = (Vector2)cam.ScreenToWorldPoint(new Vector3(cam.pixelWidth, 0, cam.nearClipPlane));

        // add or use existing EdgeCollider2D
        var edge = GetComponent<EdgeCollider2D>() == null ? gameObject.AddComponent<EdgeCollider2D>() : GetComponent<EdgeCollider2D>();

        var edgePoints = new[] { bottomLeft, topLeft, topRight, bottomRight, bottomLeft };
        edge.points = edgePoints;
        edge.sharedMaterial = bouncy;
    }
}

Script attached to HUD (canvas), it just has a AddBounce method to display bounce count, would be surprised if I screwed even this:

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

// Script for HUD management, and stuf...

public class HUD : MonoBehaviour
{
    // declaring them fields
    [SerializeField]
    Text bounceText;
    int bounces;

    // Start is called before the first frame update
    void Start()
    {
        // assign number of bounces to text
        bounceText.text = bounces.ToString();

    }

    // Method to add bounces
    public void AddBounce()
    {
        bounces += 1;
        bounceText.text = bounces.ToString();
    }
}

This is the spawner attached to the camera that detects players clicks so it can spawn coronas as long as there aren't 3 or more already:

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

public class coronaSpawner : MonoBehaviour
{
    // declaring fields
    [SerializeField]
    GameObject prefabCorona;

    // Start is called before the first frame update
    void Start()
    {

    }

    // Update is called once per frame
    void Update()
    {
        if (Input.GetButtonDown("Fire1"))
        {
            if (GameObject.FindGameObjectsWithTag("corona").Length < 3)
            {
                // gets mouse location and convert it to world position
                Vector3 mouseLocation = Input.mousePosition;
                Vector3 worldPosition = Camera.main.ScreenToWorldPoint(mouseLocation);
                worldPosition.z = 2f;

                // spawns a corona in mouse location
                Instantiate(prefabCorona, worldPosition, Quaternion.identity);
            }
        }
    }
}

And finally, here's what I hate (I think?), a blunder of horribly declared fields that feel that are too much already, here's the script attached to the coronaPrefab which traces its health, alphaValue, X and Y values for impulse force and everything else you'll have to figure out reading it:

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

public class bouncer : MonoBehaviour
{
    //declaring variables
    [SerializeField]
    int health = 100;
    [SerializeField]
    int minX = 6;
    [SerializeField]
    int maxX = 9;
    [SerializeField]
    int minY = 4;
    [SerializeField]
    int maxY = 20;
    [SerializeField]
    int fragility = 10;
    float fullHealth;
    float alphaValue;
    HUD hud;
    AudioSource audioSource;

    // Start is called before the first frame update
    void Start()
    {
        // assigning fields
        audioSource = GetComponent<AudioSource>();
        fullHealth = health;

        // calculates right alpha value for the sprite to dissapear at death
        alphaValue = (float)1 / (health / fragility);

        // taking RigidBody2D and adding random impulse force
        Rigidbody2D rb2d = GetComponent<Rigidbody2D>();
        rb2d.AddForce(new Vector2(Random.Range(minX,maxX), Random.Range(minY,maxY)), ForceMode2D.Impulse);

        // taking HUD component
        hud = GameObject.FindGameObjectWithTag("HUD").GetComponent<HUD>();

    }

    // called when the Object's colider enters a collision with another collider
    void OnCollisionEnter2D(Collision2D col)
    {
        // calls the Change method
        audioSource.Play();
        Change(col);
    }

    // gets alpha value, changes it and assigns it back. Also reduces or adds health
    private void Change(Collision2D col)
    {
        // gets color componenent
        Color color = GetComponent<SpriteRenderer>().color;

        // adds or reduce values depending on choice
        if (col.gameObject.tag == "MainCamera")
        {
            hud.AddBounce();
            health -= fragility;
            if (health <= 0)
            {
                Destroy(gameObject);
            }
        }
        else if (col.gameObject.tag == "corona")
        {
            health += fragility * 2;
            Range(health, 0, fullHealth * 1.5);
        }

        // assigns color component back with modified alpha value
        color.a = health / fullHealth;
        GetComponent<SpriteRenderer>().color = color;
    }

    // makes sure value is within a reasonable range
    public static double Range(int value, double minimum, double maximum)
    {
        if (value < minimum) { return minimum; }
        if (value > maximum) { return maximum; }
        return value;
    }
}

So yeah, this is one of my training Unity Projects, I'll be forever grateful to you if you give it a look and tell me what I could've done better, and thank you in advance!

Also I would like a smoll tip, when I get feedback on a script(s?), should I go back and change what I can? Should I try to test all the new features and tricks I learned in a different project? Or should I just read, understand and memorize? Again, thank you all so much that for your help and for taking the time and effort to share your experiences so us beginners can learn! ^^

Answer 1

Pity no one has tried to help out a beginner. While I know little of Unity, I can offer some C# comments.

Let me start with that overall your code looks nice. It has good indentation and variable names are meaningful, i.e. not cryptic or abbreviated. Use of braces is very good.

Actually, I don't see any properties in your class, but that might be a Unity requirement that objects are to be fields rather than properties.

I personally want to see access modifiers (public, private, or internal) where possible. Yes, there are defaults, but especially when dealing with beginners I want to see them explicitly specify the access modifier.

For naming conventions, classes, methods, and properties using Pascal casing. See:

.NET Design Guidelines

Naming Guidelines

C# Coding Standards

You have a lot of comments. Some are good, some not so good. There is no need for a comment that tells us what you are doing, because what the code is doing should be self evident. Comments are better if they tell us why you are doing something a certain way.

Finally the Range method could have a better name. On the surface, if I see Range, I am thinking it should be a property. For a method, I suggest a naming pattern of [Action Verb] + [Noun]. Something like GetRange follows that pattern. But in your case, I think ClampRange would be the more meaningful name, in that the name clearly denotes to the reader the intent of the method.