patterncsharpMinor
Object pooling for a top down shooting game
Viewed 0 times
toppoolinggamedownforobjectshooting
Problem
I have an Asteroids clone I'm working on and I want to see if my object pooling is written correctly. The code works I just want to know if there is anything else I need to change to make it more efficient.
My first class is my ObjectPoolManager.cs. This holds the bullets and the asteroids.
Here is my ObjectPool.cs. This class holds the logic for objects that need to be called on by my MachineGun.cs to call bullets and my AsteroidsManager.cs to call asteroids.
```
public class ObjectPool : BaseClass {
public GameObject objectToPool;
[Range (1,45)]//This is done for an easy view in the level editor
public int pooledAmount = 25;
public bool willPoolGrow = false;
public bool gravity = false;
public List _pooledObjects;
public void LateStart(GameObject ParentToGameObject)
{
initPool(ParentToGameObject);
}
private void initPool(GameObject ParentToGameObject)
{
_pooledObjects = new List();
for (int i = 0; i ().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];
My first class is my ObjectPoolManager.cs. This holds the bullets and the asteroids.
public class ObjectPoolManager : MonoBehaviour
{
//Setup some objects to be pooled.
public GameObject BulletObj;
public ObjectPool Bullets;
public GameObject AstroidObj;
public ObjectPool Astroids;
void Start()
{
BulletObj = new GameObject("Bullets");
Bullets = gameObject.AddComponent();
Bullets.objectToPool = (GameObject)Resources.Load("Prefabs/Bullet");
Bullets.LateStart(BulletObj);
AstroidObj = new GameObject("Astroids");
Astroids = gameObject.AddComponent();
Astroids.objectToPool = (GameObject)Resources.Load("Prefabs/Astroid");
Astroids.LateStart(AstroidObj);
}
}Here is my ObjectPool.cs. This class holds the logic for objects that need to be called on by my MachineGun.cs to call bullets and my AsteroidsManager.cs to call asteroids.
```
public class ObjectPool : BaseClass {
public GameObject objectToPool;
[Range (1,45)]//This is done for an easy view in the level editor
public int pooledAmount = 25;
public bool willPoolGrow = false;
public bool gravity = false;
public List _pooledObjects;
public void LateStart(GameObject ParentToGameObject)
{
initPool(ParentToGameObject);
}
private void initPool(GameObject ParentToGameObject)
{
_pooledObjects = new List();
for (int i = 0; i ().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];
Solution
Ok, Few small things.
Hard coding is a bad idea.
Sure, it may not be that relevant in this case but it is good practice to take anything that can change...out, such as your prefabs.
Next up.
magic strings are scary.
again, knowing unity and what you are doing means its not a big deal but again...it is better practice to avoid doing it all together.
finally
Repetition leads to more places to change and is messy
in general remove the little bits of code repetition where you can, the component decoration and composition unity does can look a little bit hard on the eyes when stacked.
so move some of that into helper methods where appropriate
as for unity SPECIFIC things, the way to avoid the distasteful pattern of publicly allowing access to fields is:
this will still show up in the inspector, you can also use [HideInInspector] where needed
So with that in mind a few small changes:
Is that not a little easier to understand? Hopefully as an added bonus seeing how little the differences are in creating multiple pools, you might want to even go so far as to make a pool manager that takes in a custom struct of the data and manage tonnes of pools
e.g:
then by being a serializable, unity will make all the fields available in the inspector, now you can set up a more modular system.
so now, in the actual editor you just set "Bullets,False" - "Asteroids,True" and you can add new pool as easy as that. Nothing hardcoded.
...so yeah that's a small bit of homework if you want to make it more modular.
onto the ObjectPool class:
So, similar story with the private fields and so forth.
aside form that, I have always hated the line:
equally as bad as the resource load line above, take both. make yourself some extension methods ro a helper class or something. For the non unity-ers you may be wondering why so extreme for one line,
well, because once you create a gameObject in unity there is a 90% chance you will add or get a component. making an easier method to create a configured gameObject in a well phrased single function is a good idea.
as for error checking...... you are able to call "
so basically, if the pooledObjects doesn't exist, create it, then...return it. If it already exists...just return it.
this is just me being a bit picky, (as i am a linq fan) but you can simplify the GetPooledObject to:
I would also argue you could separate out the pool item from the look a little.
and then use:
easier on the eyes...
MachineGun is next....
privatize fields as before.
variablize the magic strings, either make them passable parameters or constants.
(will you ALWAYS have "FirePoint" in your scene and will it always be called that?)
as for unity specific,
Hard coding is a bad idea.
Sure, it may not be that relevant in this case but it is good practice to take anything that can change...out, such as your prefabs.
Next up.
magic strings are scary.
again, knowing unity and what you are doing means its not a big deal but again...it is better practice to avoid doing it all together.
finally
Repetition leads to more places to change and is messy
in general remove the little bits of code repetition where you can, the component decoration and composition unity does can look a little bit hard on the eyes when stacked.
so move some of that into helper methods where appropriate
as for unity SPECIFIC things, the way to avoid the distasteful pattern of publicly allowing access to fields is:
[SerializeField]private float someFloat;this will still show up in the inspector, you can also use [HideInInspector] where needed
So with that in mind a few small changes:
public class ObjectPoolManager : MonoBehaviour
{
//Setup some objects to be pooled.
[SerializeField]private GameObject BulletObj;
[SerializeField]private ObjectPool Bullets;
[SerializeField]private GameObject AstroidObj;
[SerializeField]private ObjectPool Astroids;
[SerializeField]private string PrefabsDirectory = "Prefabs";
[SerializeField]private string BulletPrefabName = "Bullet";
[SerializeField]private string AsteroidPrefabName = "Asteroid";
const string ASTEROIDS = "Asteroids";
const string BULLETS = "Bullets";
void Start()
{
BulletObj = LoadGameObjectResource(BulletPrefabName);
Bullets = CreateObjectPool(BULLETS,BulletObj);
AsteroidObj = LoadGameObjectResource(BulletPrefabName);
Asteroids = CreateObjectPool(ASTEROIDS,AsteroidObj);
Astroids.LateStart(AstroidObj);
}
ObjectPool CreateObjectPool(string name,GameObject objectToPool)
{
var pool = new GameObject(name).AddComponent();
pool.objectToPool = objectToPool;
return pool;
}
GameObject LoadGameObjectResource(string resource)
{
return (GameObject)Resources.Load(String.Format("{0}/{1}",PrefabsDirectory,resource));
}
}Is that not a little easier to understand? Hopefully as an added bonus seeing how little the differences are in creating multiple pools, you might want to even go so far as to make a pool manager that takes in a custom struct of the data and manage tonnes of pools
e.g:
[Serializable]
public class ObjectPoolConfig
{
[SerializableField]public string PrefabName;
[SerializableField]public bool LateStart = false;
}then by being a serializable, unity will make all the fields available in the inspector, now you can set up a more modular system.
foreach(var config in ObjectPoolConfigurations) //this is the list of the configs...
{
var objectToPool = LoadGameObjectResource(config.PrefabName);
var pool = CreateObjectPool("PoolNameHere",BulletObj);
pool.objectToPool = objectToPool;
if(config.LateStart)
pool.LateStart(objectToPool)
ObjectPools.Add(pool);
}so now, in the actual editor you just set "Bullets,False" - "Asteroids,True" and you can add new pool as easy as that. Nothing hardcoded.
...so yeah that's a small bit of homework if you want to make it more modular.
onto the ObjectPool class:
So, similar story with the private fields and so forth.
aside form that, I have always hated the line:
GameObject obj = (GameObject)Instantiate(objectToPool);equally as bad as the resource load line above, take both. make yourself some extension methods ro a helper class or something. For the non unity-ers you may be wondering why so extreme for one line,
well, because once you create a gameObject in unity there is a 90% chance you will add or get a component. making an easier method to create a configured gameObject in a well phrased single function is a good idea.
as for error checking...... you are able to call "
GetPooledObject" and if you didn't call LateStart, the list would be null and throw an exception, lazy load your list. List PooledObjects { get { return _pooledObjects ?? (_pooledObjects = new List(); ) } }so basically, if the pooledObjects doesn't exist, create it, then...return it. If it already exists...just return it.
this is just me being a bit picky, (as i am a linq fan) but you can simplify the GetPooledObject to:
PooledObjects.Where(x=>!x.activeInHierarchy).First();I would also argue you could separate out the pool item from the look a little.
void AddPoolItem(){}and then use:
for(int i = 0; i < pooledAmount; i++) AddPoolItem();easier on the eyes...
MachineGun is next....
privatize fields as before.
variablize the magic strings, either make them passable parameters or constants.
(will you ALWAYS have "FirePoint" in your scene and will it always be called that?)
as for unity specific,
GameObject.Find is SLOWWWWWWWWW. and aCode Snippets
[SerializeField]private float someFloat;public class ObjectPoolManager : MonoBehaviour
{
//Setup some objects to be pooled.
[SerializeField]private GameObject BulletObj;
[SerializeField]private ObjectPool Bullets;
[SerializeField]private GameObject AstroidObj;
[SerializeField]private ObjectPool Astroids;
[SerializeField]private string PrefabsDirectory = "Prefabs";
[SerializeField]private string BulletPrefabName = "Bullet";
[SerializeField]private string AsteroidPrefabName = "Asteroid";
const string ASTEROIDS = "Asteroids";
const string BULLETS = "Bullets";
void Start()
{
BulletObj = LoadGameObjectResource(BulletPrefabName);
Bullets = CreateObjectPool(BULLETS,BulletObj);
AsteroidObj = LoadGameObjectResource(BulletPrefabName);
Asteroids = CreateObjectPool(ASTEROIDS,AsteroidObj);
Astroids.LateStart(AstroidObj);
}
ObjectPool CreateObjectPool(string name,GameObject objectToPool)
{
var pool = new GameObject(name).AddComponent<ObjectPool>();
pool.objectToPool = objectToPool;
return pool;
}
GameObject LoadGameObjectResource(string resource)
{
return (GameObject)Resources.Load(String.Format("{0}/{1}",PrefabsDirectory,resource));
}
}[Serializable]
public class ObjectPoolConfig
{
[SerializableField]public string PrefabName;
[SerializableField]public bool LateStart = false;
}foreach(var config in ObjectPoolConfigurations) //this is the list of the configs...
{
var objectToPool = LoadGameObjectResource(config.PrefabName);
var pool = CreateObjectPool("PoolNameHere",BulletObj);
pool.objectToPool = objectToPool;
if(config.LateStart)
pool.LateStart(objectToPool)
ObjectPools.Add(pool);
}GameObject obj = (GameObject)Instantiate(objectToPool);Context
StackExchange Code Review Q#95468, answer score: 5
Revisions (0)
No revisions yet.