It is terrible that the bullet class has a reference for the bullet pool

In this example, the launcher class has the bullet pool and does things to the bullet.
The return method should be in the launcher class, not in the bullet class.
The bullet class should just do the bullet related behavior, it shouldn’t know everything about the bullet pool.
From this example, I think it should be the bullet class provides an event that will be called when the bullet disappears.

1 Like

I saw your comment here and thought it would be fun to try and implement a version of this where the bullet does not contain any reference to the launcher or the bullet pool.

Here is my solution for the Bullet class

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

public class Bullet : MonoBehaviour
{
    [SerializeField] Vector3 speed;

    public delegate void InvisibleAction(Bullet bullet);
    public static event InvisibleAction OnInvisible;

    void Update()
    {
        transform.position += speed * Time.deltaTime;
    }

    private void OnBecameInvisible()
    {
        if(OnInvisible != null)
        {
            OnInvisible(this);
        }
    }
}

And for the Launcher Class:

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

public class Launcher : MonoBehaviour
{
    [SerializeField] Bullet bulletPrefab;
    private IObjectPool<Bullet> bulletPool;

    private void Awake()
    {
        bulletPool = new ObjectPool<Bullet>(
            CreateBullet,
            OnGet,
            OnRelease,
            OnDestroyBullet,
            maxSize:3
           ); 
    }

    private void Start()
    {
        Bullet.OnInvisible += ReturnInvisibleBulletToPool;
    }

    private void OnDestroyBullet(Bullet bullet)
    {
        Destroy(bullet.gameObject);
    }

    private Bullet CreateBullet()
    {
        Bullet bullet = Instantiate(bulletPrefab);
        bullet.transform.position = transform.position;
        return bullet;
    }

    private void OnGet(Bullet bullet)
    {
        bullet.transform.position = transform.position;
        bullet.gameObject.SetActive(true);
    }

    private void OnRelease(Bullet bullet)
    {
        bullet.gameObject.SetActive(false);
    }

    void Update()
    {
        FireBullet();
    }

    private void ReturnInvisibleBulletToPool(Bullet bullet)
    {
        bulletPool.Release(bullet);
    }


    private void FireBullet()
    {
        if (Input.GetKeyDown(KeyCode.Space))
        {
            bulletPool.Get();
        }
    }
}

I don’t love that the bullet needed to implement an event to tell the world that it was no longer visible and would prefer a solution where the launcher keeps track of all the bullets in the pool.

I tried creating a List of Bullets in the Launcher and adding each Bullet to that list on creation. But there was a mismatch between what was being added/removed from the list and what was being added/removed from the pool that was throwing errors. If there was an easy way to access all the objects in the pool I reckon it should be possible to loop through them and check isVisible on Update.

@Brian_Trotter - do you know if Unity has a ready way to loop through all objects in a pool?

Tracking the bullets in the Launcher is needlessly complex, and could be prone to mismatch… Once the launcher lets go of the bullet, it shouldn’t be wasting Update cycles checking to see if bullets are visible.

A better solution is to use a Lambda expression, passed to the bullet when it is instantiated.

public class Bullet : MonoBehaviour
{
    [SerializeField] Vector3 speed;

    public System.Action OnBulletFinished;
   

    void Update()
    {
        transform.position += speed * Time.deltaTime;
    }

    private void OnBecameInvisible()
    {
        OnBulletFinished?.Invoke();
    }
}
using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.Pool;

public class Launcher : MonoBehaviour
{
    [SerializeField] Bullet bulletPrefab;
    private IObjectPool<Bullet> bulletPool;

    private void Awake()
    {
        bulletPool = new ObjectPool<Bullet>(
            CreateBullet,
            OnGet,
            OnRelease,
            OnDestroyBullet,
            maxSize:3
           ); 
    }


    private void OnDestroyBullet(Bullet bullet)
    {
        Destroy(bullet.gameObject);
    }

    private Bullet CreateBullet()
    {
        Bullet bullet = Instantiate(bulletPrefab);
        bullet.transform.position = transform.position;
        return bullet;
    }

    private void OnGet(Bullet bullet)
    {
        bullet.transform.position = transform.position;
        bullet.gameObject.SetActive(true);
        bullet.OnBulletFinished = ()=>
        {
             bulletPool.Release(bullet);
        };
    }

    private void OnRelease(Bullet bullet)
    {
        bullet.gameObject.SetActive(false);
    }

    void Update()
    {
        FireBullet();
    }




    private void FireBullet()
    {
        if (Input.GetKeyDown(KeyCode.Space))
        {
            bulletPool.Get();
        }
    }
}

With this, the delegate is given an anonymous function to call when it is no longer visible, and the function contains all the information it needs to return to the pool without the bullet actually knowing anything about the transaction. It’s just calling a delegate. There is no need to pass parameters, because the parameter is wrappedup in the Lambda expression in what’s called a closure.

So for Launcher, it’s Fire and Forget, and for the bullet, it’s just “Let’s do this thing, whatever it is, when we’re done”

Ooh! I have never seen that particular form before and it looks cool.

Thank you for the input - totally agree that having two sets of the same data and trying to make sure they are always matching is playing with fire!

This is an anonymous function. It has no name, and exists solely for this purpose. A special object is created that -=is=- the function. What makes this magical is that the values within the method are encapsulated with the function itself, so in thsi case, the reference to bullet is captured within the method to be used as the parameter in the call to bulletPool.Release(bullet);

When we use lambda expressions in this way, they are commonly called a closure.

We use these closures within the Shop and Abilities course to bind functionality at runtime to buttons within the shop window.

More reading: A Simple Explanation of C# Closures | Closures, lambdas, and first-class functions don't have to be so hard. - Simple Thread

1 Like

I do love me some delegates. I often use them with coroutines to get results back - or at the least, do something when it completes.

private IEnumerator DoBigLoop(int manyIterations, Action<float> onProgress, Action onComplete)
{
    for (var i = 0; i < manyIterations; i++)
    {
        DoSomething();
        onProgress?.Invoke(i / (float)manyIterations);
        yield return null;
    }
    onComplete?.Invoke();
}

// usage
_progressBar.value = 0f;
_progressBar.gameObject.SetActive(true);
StartCoroutine(DoBigLoop(10_000_000, (f) => _progressBar.value = f, () => _progressBar.gameObject.SetActive(false));
1 Like

Privacy & Terms