Code review for a beginner Arknanoid project

Unity Engine
1

Hello people!

I’m doing a BlockBreaker/Arknoid clone. I’m trying to refactor my code to create the habit of doing the projects in the best ways. I have a hard time using Managers and Controllers. I do not know how to avoid using GetComponent and FindObject, and I’m not sure how to set the specific function for each of them.

I was told to use a EventSystem, but I do not understand anything about it and I prefer to learn it calmly later. I want to optimize it in a way that it remains clear for me.

Could you guys please help me?

Here is my complete project:
https://github.com/thisfn/Breakout/

There is a bunch of scripts here. But few ones show Exactly what I have difficulty on is the BlockController.cs, EffectsController.cs and GameManager.cs.

public class BlockController : MonoBehaviour
    {
        [SerializeField] private AudioClip _hitAudioClip;

        private CameraController _cameraController;
        private EffectsController _effectsController;
        private LevelManager _levelManager;
        private SoundManager _soundManager;

        public static int BlockCount;

        private void Awake()
        {
            _cameraController = FindObjectOfType<CameraController>();
            _soundManager = FindObjectOfType<SoundManager>();
            _levelManager = FindObjectOfType<LevelManager>();
            _effectsController = FindObjectOfType<EffectsController>();
        }

        public void BlockHit()
        {
            _soundManager.PlaySingle(_hitAudioClip);
            _cameraController.ShakeScreen();
        }

        public void BlockDestroy(Vector3 position, Material material)
        {
            BlockCount--;
            if (BlockCount <= 0)
                _levelManager.LoadNextLevel();

            _effectsController.BlockExplosion(position, material);
        }
    }

public class EffectsController : MonoBehaviour
    {
        [SerializeField] private List<ParticleSystem> _blockParticles;
        [SerializeField] private List<ParticleSystem> _explosionParticles;

        private int _tempExploCounter;

        private static EffectsController _instancEffectsController;

        private void Awake()
        {
            if (_instancEffectsController == null)
                _instancEffectsController = this;
            else if (_instancEffectsController != this)
                Destroy(gameObject);
        }

        public void Explosion(Vector2 position)
        {
            //TODO How to optimize?
            //TODO reactivate after counting
            //_explosionParticles[0].transform.position = position;
            //_explosionParticles[0].gameObject.SetActive(true);
            //_blockParticles[0].Play();
            //_explosionParticles.RemoveAt(0);
            _tempExploCounter++;
            Debug.Log(_tempExploCounter);
        }

        public void BlockExplosion(Vector2 position, Material material)
        {
            //TODO How to optimize?
            _blockParticles[0].transform.position = position;
            _blockParticles[0].GetComponent<Renderer>().material = material;
            _blockParticles[0].gameObject.SetActive(true);
            _blockParticles[0].Play();
            _blockParticles.RemoveAt(0);
        }
    }

public class GameManager : MonoBehaviour
    {
        [SerializeField] private float _ballOffset;
        [SerializeField] private GameObject _ballGameObject;
        [SerializeField] private GameObject _boardGameObject;
        [SerializeField] private GameObject _effectsController;

        private bool _isPaused;
        private SoundManager _soundManager;

        private static GameManager _instanceGameManager;

        private void Awake()
        {
            if (_instanceGameManager == null)
                _instanceGameManager = this;
            else if (_instanceGameManager != this)
                Destroy(gameObject);

            DontDestroyOnLoad(gameObject);

            _soundManager = FindObjectOfType<SoundManager>();
        }

        public void InitilizeLevel()
        {
            BlockController.BlockCount = 0;

            var effectsController = Instantiate(_effectsController).GetComponent<EffectsController>();
            var board = Instantiate(_boardGameObject).GetComponent<Board>();
            var ballPos = board.transform.position + Vector3.up * _ballOffset;
            var ball = Instantiate(_ballGameObject, ballPos, Quaternion.identity).GetComponent<Ball>();

            ball.Board = board.GetComponent<Board>();
            ball.SoundManager = _soundManager;
            ball.EffectsController = effectsController;

            StateManager.SetWaiting();
        }

        public void PauseGame()
        {
            _isPaused = !_isPaused;
            if (_isPaused)
            {
                Time.timeScale = 0f;
                StateManager.SetPaused();
            }
            else
            {
                Time.timeScale = 1f;
                StateManager.ChangeToPreviousState();
            }
        }
    }

Many thanks in every way possible, I really want to improve, so sorry if I’m asking much or stupid questions.

2

Hey there. What I recommend that you do, for some suggestions/feedback, is to post a script (or 2) that are concerning you the most, in the forums. Please take a moment to look at this page for how to do it, if you don’t know how: Using code tags properly
Then, you’ve already mostly described what it is you’re looking for, so that’s good.
People will be able to come to this page, and take a look at your question and offer their comments more easily, without having to change pages :slight_smile:

I took a quick look at 1 file you linked BlockController. You said one of your problems was not being able to avoid FindObject. If you want to keep your variable private but avoid that find, you can add the attribute [SerializeField] above any variable, and then drag & drop it in the inspector.

1 Like
3

Hey thank you, I will fix the post!

So i’m trying to find a way to do that through the code, because it will work in most of my scenes. Since my Managers are Singleton’s (the ones that I used on BlockController with Find).

4

I see, and this is a little helpful now that I know that.

If they persist through levels and are all available at the same time, you could use the method I mentioned before. Drag n drop should still work if they are all DontDestroyOnLoad.

However, another option is to get the reference in Start, for any singleton you’ve made (in its Awake) :slight_smile: Useful for other situations. *quick scan of your code shows you have a private singleton, but if you added a public property to return that private instance, that could work…

1 Like
5

Oh I got that!
So if I had for example the GameManager as a static public Instance (returning the private _instance), I can use it without problems and without the need to find reference for the script again! Is that right?

6

I believe we’re saying the same thing… to be 100% certain, let’s say you have:

private static SomeClass _instance;
public static SomeClass Instance { get { return _instance; }};
void Awake() {
 // do singleton check/creation here.
 }

Then, in some other class that’s looking for SomeClass

SomeClass SomeClassManager;
void Start() {
   SomeClassManager = SomeClass.Instance;
 }

err, and now of course you can choose if you even need that, because… lol… you could just use 'SomeClass.Instance" in lieu of SomeClassManager :slight_smile:

1 Like
7

Thank you! you were so helpful

8

No problem :slight_smile: You’re welcome.

1 Like