What to do with a coroutine having too much parameters? Help to refactor

I need a shooting hit effect to happen some time after you shot the target via Raycasting, to imitate a time it takes for a bullet to hit a target. (Works okay for slow targets, fast bullets, and short distances, I think. Just saying in case you want to say me I shoudn’t use that approach).

Anyway, I was applying different effects via different coroutines, like one for harming, one for SFX, one for sounds. But I figured out that creating multiple coroutines instead of just one, considering I just need the same delay for everything, is a waste of resources (as far as I know each coroutine is creating a timer and watch it every frame, and make a bit of garbadge, for some reason, idk well how it works but I got the idea), so yeah, why don’t pack all in one coroutine.

So, in the end I got something like this abomination here:

StartCoroutine(MakeHit(effectManager.getDelay(cachedHits[k].point), bonuspart, DistanceReducedInt(damage, cachedHits[k].distance), DistanceReducedInt(impulse, cachedHits[k].distance), cachedHits[k], forwardVector, type, effectManager.PrepareSoundSource(cachedHits[k].point, currentshootable.HitLoudness)));

Yeah… Any advices how to achieve this a better way?

To give some context, for something like late game OP rifle, it can penetrate up to 5 targets every 0.08 seconds, making up to 62.5 corotunes per second, which is considerable.

One-liners like this maybe minimizes LOC but will maximize debugging time later on. I suggest you decompress it a bit, for readability

var cachedHit = cachedHits[k];
var delayValue = effectManager.getDelay( cachedHit.point );
var damageValue = DistanceReducedInt( damage , cachedHit.distance );
var impulseValue = DistanceReducedInt( impulse , cachedHit.distance );
var soundSource = effectManager.PrepareSoundSource( cachedHit.point, currentshootable.HitLoudness );
var makeHitRoutine = MakeHit( delayValue, bonuspart, damageValue, impulseValue, cachedHit, forwardVector, type, soundSource );
StartCoroutine( makeHitRoutine );

High-frequency coroutines could throttle the main thread indeed, cause unpredictable spikes. I would consider consolidating all these coroutines into a system that looks like something like this:

// NOTE: this is sketch and won't compile. See the other posts for something that works.
using UnityEngine;

[AddComponentMenu("")]
public class HitEffectSystem : MonoBehaviour
{
    FxPrefabPool _fxPool = new ();
    MinHeap<ScheduledHit> _schedule = new ();// sorted by hitTime

    void FixedUpdate ()
    {
        if( _schedule.Length!=0 )
        if( _schedule.Peek().hitTime<=Time.time )// @TODO: handle >1 hit per tick
        {
            var evt = _schedule.Pop();
            Transform instance = _fxPool.GetInstance( evt.fx );
            instance.position = evt.hitPosition;
            // etc...
        }
    }

    /// <summary> Weapon calls this </summary>
    public void OnShotFired ( /* shot input arguments */ )
    {
        // calculates shot hit position, time and everything else
        float tminus = 1;
        _schedule.Push( new ScheduledHit{
            hitTime = Time.time + tminus,
            // ..
        } );
    }

    public struct ScheduledHit
    {
        public float hitTime;
        public Vector3 hitPosition;
        public Vector3 hitNormal;
        public int fx;// fx ID in the system
        /* other data */
    }

    [RuntimeInitializeOnLoadMethod( RuntimeInitializeLoadType.BeforeSceneLoad )]
    static void InitializeType () => DontDestroyOnLoad( new GameObject($"#{nameof(HitEffectSystem)}").AddComponent<HitEffectSystem>().gameObject );

}

@andrew-lukasik

I am kinda stuck here, made it this far, but it of course don’t work due me misinterpreting how compare thingy works, or how your MinHeap works
I may probably get it eventually, but asking for help is much faster here I guess

UPD: Well I have trimmed your implementation out of “COMPARABLE” stuff and this bit

ITEM : System.IEquatable<ITEM>

(what they were for, anyway?)
And I guess it’s working now. Well, at least compiles, I will try to use it later. So my ask for help is deprecated.

    public struct HitsComp : IMinHeapComparer<ScheduledHit>
    {
        public int Compare(ScheduledHit lhs, ScheduledHit rhs)
        {
            return lhs.hitTime.CompareTo(rhs.hitTime);
        }
    }
    public struct ScheduledHit
    {
        public float hitTime;
        //more stuff coming soon
    }
    private MinHeap<ScheduledHit, HitsComp> schedulethits = new(100);

UPD2:why making everything unmanaged in MinHeap cs? My custom structs would also contain references of mobs they are going to hurt, or somthing like that.
Well, after more trimming, this all finally work, yay. Still don’t understand alot in your code. Left a comment there.

Hi @BlackyPootis ! Here is an example implementation that compiles.

Next step would be to add a GameObject pooling implementation because Instantiate() causes cpu spikes and we don’t want that.

HitEffectSystem.cs

using System.Collections.Generic;
using UnityEngine;

[AddComponentMenu("")]
public class HitEffectSystem : MonoBehaviour
{
    FxPrefabPool _fxPool = new ();
    MinHeap<ScheduledHit> _schedule = new ( 64 );// sorted by hitTime
    RaycastHit[] _hits = new RaycastHit[8];

    void FixedUpdate ()
    {
        if( _schedule.Length!=0 )
        if( _schedule.Peek().time<=Time.time )// @TODO: handle >1 hit per tick
        {
            var hitEvent = _schedule.Pop();
            Transform instance = _fxPool.GetInstance( hitEvent.prefab );
            instance.position = hitEvent.position;
            instance.rotation = hitEvent.rotation;
        }
    }

    /// <summary> Weapon calls this </summary>
    public void OnShotFired ( Vector3 origin , Vector3 velocity , GameObject prefab )
    {
        // calculates shot hit position, time and everything else
        int numRaycastHits = Physics.RaycastNonAlloc( origin:origin , direction:velocity , _hits );
        float speed = velocity.magnitude;
        for( int i=0 ; i<numRaycastHits ; i++ )
        {
            var hit = _hits[i];
            float tminus = hit.distance / speed;

            var entry = new ScheduledHit{
                time = Time.time + tminus,
                position = hit.point ,
                rotation = Quaternion.LookRotation( velocity , hit.normal ) ,
                prefab = prefab ,
            };
            _schedule.Push( entry );
        }
    }

    public struct ScheduledHit : System.IComparable<ScheduledHit>
    {
        public float time;
        public Vector3 position;
        public Quaternion rotation;
        public GameObject prefab;
        int System.IComparable<ScheduledHit>.CompareTo ( ScheduledHit other )
        {
            return this.time.CompareTo(other.time);
        }
    }

    [RuntimeInitializeOnLoadMethod( RuntimeInitializeLoadType.BeforeSceneLoad )]
    static void InitializeType () => DontDestroyOnLoad( new GameObject($"{nameof(HitEffectSystem)}").AddComponent<HitEffectSystem>().gameObject );

}

[System.Obsolete("replace me with a real gameobject pooling solution")]
public class FxPrefabPool
{
    public Transform GetInstance ( GameObject prefab ) => GameObject.Instantiate(prefab).transform;
}

HitEffectTester.cs

using UnityEngine;

public class HitEffectTester : MonoBehaviour
{

    [SerializeField] GameObject _prefab;
    [SerializeField][Min(.001f)] float _bulletSpeed = 20f;
    HitEffectSystem _hitEffectSystem;
    Camera _camera;
    
    void Start ()
    {
        _hitEffectSystem = FindAnyObjectByType<HitEffectSystem>();
        _camera = Camera.main;
    }

    void Update ()
    {
        if( Input.GetMouseButton(0) )
        {
            Ray ray = _camera.ScreenPointToRay( Input.mousePosition );
            _hitEffectSystem.OnShotFired( origin:ray.origin , velocity:ray.direction*_bulletSpeed , prefab:_prefab );
        }
    }

}

MinHeap.cs

// src* https://gist.github.com/andrew-raphael-lukasik/b00a59509cbe8fa2b15b1949fa4f4ad2
using System.Collections.Generic;

public struct MinHeap <T>
    where T : System.IComparable<T>
{
    List<T> _stack;

    public int Length => _stack.Count;
    public int Count => _stack.Count;

    public MinHeap ( int capacity )
    {
        this._stack = new List<T>( capacity );
    }

    public void Push ( T item )
    {
        _stack.Add( item );
        MinHeapifyUp( _stack.Count-1 );
    }
    public T Pop ()
    {
        T removedItem = _stack[0];
        RemoveAtSwapBack( _stack , 0 );
        MinHeapifyDown( 0 );
        return removedItem;
    }

    public T Peek () => _stack[0];
    public void Clear () => _stack.Clear();

    void MinHeapifyUp ( int childIndex )
    {
        if( childIndex==0 ) return;
        int parentIndex = (childIndex-1)/2;
        T childVal = _stack[childIndex];
        T parentVal = _stack[parentIndex];
        if( childVal.CompareTo(parentVal)<0 )
        {
            // swap the parent and the child
            _stack[childIndex] = parentVal;
            _stack[parentIndex] = childVal;
            MinHeapifyUp( parentIndex );
        }
    }

    void MinHeapifyDown ( int index )
    {
        int leftChildIndex = index * 2 + 1;
        int rightChildIndex = index * 2 + 2;
        int smallestItemIndex = index;// The index of the parent
        if(
            leftChildIndex<=this._stack.Count-1
            && _stack[leftChildIndex].CompareTo(_stack[smallestItemIndex])<0 )
        {
            smallestItemIndex = leftChildIndex;
        }
        if(
            rightChildIndex<=this._stack.Count-1
            && _stack[rightChildIndex].CompareTo(_stack[smallestItemIndex])<0 )
        {
            smallestItemIndex = rightChildIndex;
        }
        if( smallestItemIndex!=index )
        {
            // swap the parent with the smallest of the child items
            T temp = _stack[index];
            _stack[index] = _stack[smallestItemIndex];
            _stack[smallestItemIndex] = temp;
            MinHeapifyDown( smallestItemIndex );
        }
    }

    public int Parent ( int key ) => (key-1)/2;
    public int Left ( int key ) => 2*key + 1;
    public int Right ( int key ) => 2*key + 2;

    void RemoveAtSwapBack ( List<T> list, int index )
    {
        int lastIndex = list.Count - 1;
        list[index] = list[lastIndex];
        list.RemoveAt(lastIndex);
    }

}