3
\$\begingroup\$

After the advise from this code review I have come up with the following changes. I'll go in the same order as my previous post. This post is mainly for an updated version so that others can see the previous posts suggestions. If I have made any mistakes please let me know.

ObjectPoolManager.cs:

public class ObjectPoolManager : MonoBehaviour {
#region Objects to Pool
[SerializeField] private GameObject bulletObj;
[SerializeField] private GameObject bulletPool;
[SerializeField] private ObjectPool bullets;
[SerializeField] private GameObject asteroidObj;
[SerializeField] private ObjectPool asteroids;
[SerializeField] private GameObject asteroidPool;

public ObjectPool Bullets { get { return bullets; } }
public ObjectPool Asteroids { get { return asteroids; } }
#endregion

[SerializeField] private string PrefabsDirectory = "Prefabs";

[SerializeField] private string BulletPrefabName = "Bullet";
[SerializeField] private string AsteroidPrefabName = "Asteroid";

#region Const Names
const string ASTEROIDS = "Asteroids";
const string BULLETS = "Bullets";
const string POOL = "Pool";
#endregion

    void Start() {
        initBulletPool();
        initAsteroidPool();
    }

    private void initAsteroidPool() {
        asteroidPool = new GameObject(ASTEROIDS + POOL);
        asteroidObj = LoadGameObjectResource(AsteroidPrefabName);
        asteroids = CreateObjectPool(ASTEROIDS, asteroidObj);
        asteroids.LateStart(asteroidPool);
    }

    private void initBulletPool() {
        bulletPool = new GameObject(BULLETS + POOL);
        bulletObj = LoadGameObjectResource(BulletPrefabName);//Loads the prefab into the GameObject
        bullets = CreateObjectPool(BULLETS, bulletObj);
        bullets.LateStart(bulletPool);
    }

    ObjectPool CreateObjectPool(string name, GameObject objectToPool) {
        var pool = new GameObject(name).AddComponent<ObjectPool>();
        pool.ObjectToPool = objectToPool;
        return pool;
    }

    GameObject LoadGameObjectResource(string resource) {
        //Why do we use string format here? Isn't it easier to put "PrefabsDirectory + resource"
        return (GameObject)Resources.Load(System.String.Format("{0}/{1}", PrefabsDirectory, resource));
    }
}

It's a little bit longer but it privatizes the variables while still allowing them to be viewed in the inspector. I also added some helper functions that were recommended.

ObjectPool.cs:

public class ObjectPool : BaseClass {
[SerializeField] private GameObject objectToPool;
public GameObject ObjectToPool { get { return objectToPool; } set { objectToPool = value; } }

[Range(1, 45)]//This is done for an easy view in the level editor
[SerializeField] private int pooledAmount = 25;
[SerializeField] private bool willPoolGrow = true;
[SerializeField] private bool gravity = false;

[SerializeField] private List<GameObject> _pooledObjects;

    public void LateStart(GameObject ParentToGameObject) {
        initPool(ParentToGameObject);
    }

    private void initPool(GameObject ParentToGameObject) {
        _pooledObjects = new List<GameObject>();
        for (int i = 0; i < pooledAmount; i++) AddPoolItem(ParentToGameObject);
    }

    private void AddPoolItem(GameObject ParentToGameObject) {
        GameObject obj = objectToPool.InstantiateGameObject();
        obj.GetComponent<Rigidbody>().useGravity = false;
        _pooledObjects.Add(obj);
        obj.SetActive(false);
        obj.transform.parent = ParentToGameObject.transform;
    }

    public GameObject GetPooledObject() {
        for (int i = 0; i < _pooledObjects.Count; i++) {
            if (!_pooledObjects[i].activeInHierarchy) {
                return _pooledObjects[i];
            }
        }
        if (willPoolGrow) {
            GameObject obj = objectToPool.InstantiateGameObject();
            _pooledObjects.Add(obj);
            return obj;
        }
        return null;
    }
}

I just added a helper function to add the objects to the pool for me. Which allowed me that change the for loop in initPool().

MachineGun.cs:

public class MachineGun : MonoBehaviour {
    public ObjectPoolManager _objectPoolManager;
    public Transform gunPoint; 

    GameObject projectile;
    [SerializeField] private float rapidFireTimeCap;
    [SerializeField] private float rapidFireTimeCurrent;
    [SerializeField] private float rapidfireTimeMultiplier;
    [SerializeField] private bool fireBullet = true;

    void Start() {
        _objectPoolManager = GameObject.Find("ObjectPoolHolder").GetComponent<ObjectPoolManager>();
        gunPoint = transform.Find("FirePoint");//Get the point of the gun
    }

    void Update() {
        RapidFire();
        rapidFireTimeCurrent += Time.deltaTime;
    }

    void RapidFire() {
        if (rapidFireTimeCurrent > rapidFireTimeCap && fireBullet) {
            rapidFireTimeCurrent = 0.0f;
            FireBullet();
        }
    }

    void FireBullet() {
        projectile = _objectPoolManager.Bullets.GetPooledObject();
        if (projectile != null) {
            projectile.transform.position = gunPoint.transform.position;
            projectile.transform.rotation = gunPoint.transform.rotation;
            projectile.GetComponent<Projectile>().FireMe();
        }
    }
}

I didn't change a lot. I just serialized fields and took out some comments.

I'll be changing the ObjectPoolManager into a singleton as advised. I just need to learn about those so that I can get started.

\$\endgroup\$
4
\$\begingroup\$

A couple of very minor points.

I noticed a bit of an inconsistency with your convention for naming variables. Sometimes you are using _ prefix, sometimes not, sometimes PascalCase sometimes not. For example some of your variables are declared as:

 private GameObject objectToPool;
 private List<GameObject> _pooledObjects;
 public ObjectPoolManager _objectPoolManager;
 public Transform gunPoint; 
 public GameObject ObjectToPool { get { return objectToPool; } set { objectToPool = value; }

I think it would be best to stick to a convention and ensure you are consistent across that as that helps with code readability as well as maintainability for other developers.

The method GetPooledObject could be potentially re-written using Linq. I'm not sure if you chose a loop iterator as that is your preference, however the method could be simplified (IMO) to be:

public GameObject GetPooledObject() {

  var pooledObject = _pooledObjects.FirstOrDefault(p => !p.activeInHierarchy);
  if(pooledObject != null) {
    return pooledObject;
  }

  if (willPoolGrow) {
      GameObject obj = objectToPool.InstantiateGameObject();
      _pooledObjects.Add(obj);
      return obj;
  }
  return null;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.