2D Unity game using 3D objects, gather points and avoid obstacles

Question

I've been messing around with some stuff in Unity and are now working with 5 classes in my project. As I am very new to the engine, and even C#, I believe I made a rather messy code, even though it works as desired.

I would love your guys personal opinions on how to clean up and organize my code, which I hope is okay to post about here.

I am currently for the experience of it, creating a 2D game using 3D objects:

CollisionScript.cs:

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

public class CollisionScript : MonoBehaviour {
    public int scorePoints;
    public int healthAmount;
    public Text scoreUI;
    public Text health;
    public GameObject points;
    public GameObject player;
    public Camera mainCamera;
    public GameObject obstacle;
    public GameObject obstacle1;
    Vector3 randomPos;
    Vector3 randomPos2;
    Vector3 randomPos3;
    float speed = 1.0f;
    float obstacleSpeedX = 1.5f;
    float obstacleSpeedY = 1.5f;

    public void RandomFunction() {
        float screenX = Random.Range(2.0f, mainCamera.pixelWidth - 2.0f);
        float screenY = Random.Range(2.0f, mainCamera.pixelHeight - 2.0f);
        float screenZ = 10.0f;
        Vector3 position = mainCamera.ScreenToWorldPoint(new Vector3(screenX, screenY, screenZ));

        float screenX2 = Random.Range(4.0f, mainCamera.pixelWidth - 4.0f);
        float screenY2 = Random.Range(4.0f, mainCamera.pixelHeight - 4.0f);
        float screenZ2 = 10.0f;
        Vector3 position2 = mainCamera.ScreenToWorldPoint(new Vector3(screenX2, screenY2, screenZ2));

        float screenX3 = Random.Range(4.0f, mainCamera.pixelWidth - 4.0f);
        float screenY3 = Random.Range(4.0f, mainCamera.pixelHeight - 4.0f);
        float screenZ3 = 10.0f;
        Vector3 position3 = mainCamera.ScreenToWorldPoint(new Vector3(screenX3, screenY3, screenZ3));

        randomPos2 = position2;

        if (scorePoints == 1) {
            randomPos = position;
            randomPos3 = position3;
        }
    }

    void Start() {
        obstacle.SetActive (false);
        obstacle1.SetActive (false);
    }

    void Update() {
        if (scorePoints == 5) {
            obstacle.SetActive (true);
            obstacle.transform.position = randomPos;
        } else if (scorePoints == 10) {
            obstacle1.SetActive (true);
            obstacle1.transform.position = randomPos3;
        } else if (scorePoints >= 15) {
            obstacle.transform.position += Vector3.right * obstacleSpeedX * Time.deltaTime;
            obstacle1.transform.position += Vector3.up * obstacleSpeedY * Time.deltaTime;
        }

        if (obstacle.transform.position.x <= -8.9f) {
            obstacleSpeedX = 1.5f;
        }

        if (obstacle.transform.position.x >= 8.9f) {
            obstacleSpeedX = -1.5f;
        }

        if (obstacle1.transform.position.y <= -5.0f) {
            obstacleSpeedY = 1.5f;
        } 

        if (obstacle1.transform.position.y >= 5.0f) {
            obstacleSpeedY = -1.5f;
        }

    }

    public void pointPosition() {
        points.transform.position = randomPos2;
    }
}

LethalCollisionScript.cs:

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

public class LethalCollisionScript : CollisionScript {

    void OnTriggerEnter2D(Collider2D coll) {
        healthAmount -= 1;
        player.transform.position = new Vector3(0,0,0);

        if (healthAmount >= 0) {
            health.text = "HEALTH: " + healthAmount;
        }

        if (healthAmount <= -1) {
            Application.LoadLevel (0);
        }
    }
}

ObstacleScript.cs:

using UnityEngine;
using System.Collections;

public class ObstacleScript : MonoBehaviour {

    // Update is called once per frame
    void Update () {
        transform.Rotate (new Vector3 (0, 0, 90) * Time.deltaTime * 2);
    }
}

PlayerScript.cs:

using UnityEngine;
using System.Collections;

public class PlayerScript : MonoBehaviour {

    public float speed = 1.0f;
    public Camera mainCameraPS;

    // Use this for initialization
    void Start () {

    }

    // Update is called once per frame
    void Update () {
        if (Input.GetKey (KeyCode.LeftArrow)) {
            transform.position += Vector3.left * speed * Time.deltaTime;
        }
        if (Input.GetKey (KeyCode.RightArrow)) {
            transform.position += Vector3.right * speed * Time.deltaTime;
        }
        if (Input.GetKey (KeyCode.UpArrow)) {
            transform.position += Vector3.up * speed * Time.deltaTime;
        }
        if (Input.GetKey (KeyCode.DownArrow)) {
            transform.position += Vector3.down * speed * Time.deltaTime;
        }

        if (transform.position.x <= -8.4f)
            transform.position = new Vector3 (-8.4f, transform.position.y, transform.position.z);
        else if (transform.position.x >= 8.4f)
            transform.position = new Vector3(8.4f, transform.position.y, transform.position.z);

        if (transform.position.y <= -4.6f)
            transform.position = new Vector3 (transform.position.x, -4.6f, transform.position.z);
        else if (transform.position.y >= 4.6f)
            transform.position = new Vector3 (transform.position.x, 4.6f, transform.position.z);
    }
}

PointCollisionScript.cs:

using UnityEngine;
using System.Collections;

public class PointCollisionScript : CollisionScript {

    void OnTriggerEnter2D(Collider2D coll) {

        RandomFunction ();
        pointPosition ();

        scorePoints += 1;
        scoreUI.text = "SCORE: " + scorePoints.ToString ();
    }
}

My main concern is regarding when extending classes, it's required I have to pull down objects into the script as public variables on EVERY objects I used a sub-class or main class on. (There must be a better way).

I'm also almost certain I should divide my code into even more classes for organizing and maybe create some sort of manager for my "global" variables?

Answer 1

CollisionScript.cs

• Remove speed and other unused fields.
• Place empty line between public and private fields, place empty lines between groups of fields. Programming is 80% reading code, so squeeze maximum readability from your code by taking seriously formatting, code style...
• ... and naming:
  • Field names could be clearer. For example, scoreUI→scoreText and health→healthText.
  • RandomFunction() says nothing about it (except that it uses random). It should be GenerateRandomPositions() or something like that.
  • The class itself doesn't seem to have a depictive name. It should be something like CollistionHandlerBase.
  • pointPosition() → PutPointAtPosition().
• No need to maintain public Camera mainCamera field, use Camera.main instead.
• Methods are rather big, but I don't think it's worth it refactor them, because game logic will change many times in near future. Instead, make them at least 2x times for understandable.
• Use var keywords whenever possible.

LethalCollisionScript.cs, ObstacleScript.cs, PointCollisionScript.cs

• Rename the class to LethalCollisionHandler, etc.

PlayerScript.cs

• Managable enough messinness level for now.
• Rename to something like PlayerCharacter.

Overall

1. After you're done with things above, you can start more global refactoring. It's quite brain-heavy job, don't do it until unused variables and bad names are stopped from obscuring the picture.
2. Take each class and analyze what it does overall. Separate most obvious/big/stable responsibilities to new classes. Move UI-related logic to separate class, etc.

The main concern

This problem is usually called “dependency management”. And it's a probably biggest pain of Unity game programmers. Different approaches are equally possible, but you should use not more than couple of top simplest among them for now.

• Expose fields to editor only when nesseccary. Obtain as much as possible in code logic. (Use GetComponent<Desired_type_here>(), etc.)
• Tag the player GameObject with built-in “Player” tag. Get player in Start() methods using GameObject.FindWithTag("Player");
• Create GameController MonoBehaviour. Keep common dependencies there. Other scripts can get needed data from it. Tag it with built-in “GameController” tag.
• Game Conroller acts as director above all other scripts, but keep its logic minimalistic. Don't add any logic to it without neccessity, even “for 5 minutes”, or you'll lose control over it, and it'll bloat.

More or less like this:

public class GameController : MonoBehaviour
{
    public GameObject Player;
    public UIManager UIManager;

    void Awake()
    {
        Player = GameObject.FindWithTag("Player");
        UIManager = GameObject.FindObjectOfType<UIManager>();
    }
}

Then:

public class Some_of_Game_Scripts : MonoBehaviour
{
    GameController gameController;
    UIManager uiManager;

    void Start()
    {
        gameController = GameObject.FindWithTag("GameController");
        uiManager = gameController.UIManager;
    }

    void Update()
    {
        uiManager.SetStatusText_Or_Something("Some text");
        if (something)
            gameController.ResetGame();
    }
}