Ienumerator is not working as expected

Question

So I'm trying to get my miner drone to mine from the resource by using an Ienumerator but the miner either leaves with more than the capacity or it leaves and it has taken more than the amount of resources available.

So in short, the miner is to head to the resource, when its within the trigger, it needs to take the minerAmount over the miningSpeed amount of time.

Please help.

private void OnTriggerStay(Collider other)
{
    MinableResource mr;
    if (other.CompareTag("Resource"))
    {
        Debug.Log("Within Range of Resource");
        mr = other.GetComponent<MinableResource>();

        if (minerCurrentCapacity < minerMaxCapacity)
        {
            StartCoroutine(MineResource(other));
        }
        else if (minerCurrentCapacity >= minerMaxCapacity)
        {
            minerFull = true;
            Move(storagePos);
        }
    }
}

IEnumerator MineResource(Collider _other)
{
    if (minerFull)
        yield break;

    yield return new WaitForSeconds(miningSpeed);
    Mining(_other);
}


private void Mining(Collider _other)
{
    Debug.Log("Mining");

    MinableResource mr;
    mr = _other.GetComponent<MinableResource>();

    minerCurrentCapacity += miningAmount;
    mr.resources -= miningAmount;

}

Answer 1

This is the kind of problem we can debug by stepping through the code to see what it does at each moment. Let's look at the first frame your object stays inside range of a mineble resource:

Frame 1

OnTriggerStay: minerCurrentCapacity < minerMaxCapacity, so we start a new MineResource coroutine. (#1)

MineResource (#1): minerFull is false, so we wait for the defined miningSpeed delay. For simplicity let's say that's 3 frames.

Frame 2

OnTriggerStay: minerCurrentCapacity < minerMaxCapacity, so we start another MineResource coroutine (#2).

MineResource (#1): still waiting.

MineResource (#2): minerFull is false, so we wait for the defined miningSpeed delay.

Frame 3

OnTriggerStay: minerCurrentCapacity < minerMaxCapacity, so we start another MineResource coroutine (#3).

MineResource (#1): still waiting.

MineResource (#2): still waiting.

MineResource (#3): minerFull is false, so we wait for the defined miningSpeed delay.

Frame 4

OnTriggerStay: minerCurrentCapacity < minerMaxCapacity, so we start another MineResource coroutine (#4).

MineResource (#1): finishes waiting, and calls Mining to harvest resources from other. To speed up this example, let's say that's the last of the resources we need to hit our max capacity, and to empty the resources available at this site.

MineResource (#2): still waiting.

MineResource (#3): still waiting.

MineResource (#4): minerFull is false (even though we're full now - it's only set in OnTriggerStay which runs earlier in the frame, so we won't know we're full until next frame!), so we wait for the defined miningSpeed delay.

Frame 5

OnTriggerStay: minerCurrentCapacity >= minerMaxCapacity, so we set minerFull = true and start moving back to the storage hopper.

MineResource (#2): finishes waiting, and calls Mining to harvest resources from other. Even though the miner is full and the resource source is empty, there's no logic here to check for that, so we harvest resources that aren't there and put them in capacity we don't have.

MineResource (#3): still waiting.

MineResource (#4): still waiting.

Frame 6

MineResource (#3): finishes waiting, and calls Mining to harvest even more excess resources from other.

MineResource (#4): still waiting.

Frame 7

MineResource (#4): finishes waiting, and calls Mining to harvest even more excess resources from other.

---

So, there are a whole pile of problems here: OnTriggerStay runs every physics step that we remain in range. We start a new coroutine every time, without cancelling or waiting on the previous one. We check if we're full only when we start waiting, not when we actually do the mining. We never check whether the resource patch is empty.

I'd recommend something a bit more like this:

// OnTriggerEnter fires just once, when a new trigger is touched, not every frame.
void OnTriggerEnter(Collider other) {

    // I like to use a state enum to keep track of what we're doing.
    if(state == MinerState.Mining || state == MinerState.Return)
        return; // Ignore new triggers if we're busy or returning with stuff.

    var resource = other.GetComponent<MinableResource>();

    // If there are resources there to grab, start mining.
    if(resource != null && resource.resources > 0) {
        StartCoroutine(MineResource(resource));
    }
}

IEnumerator MineResource(MinableResource resource) {
    // Mark that we're busy mining, so we don't try to start another coroutine.
    state = MinerState.Mining;

    // Rather than fire a new coroutine each frame, we'll just loop till we're done.
    while(resource.resources > 0 && resourcesHeld < capacity) {
        yield return new WaitForSeconds(miningDelay);

        // Never take more than what's there, or what we can hold.
        var increment = Mathf.Min(
            miningAmount, 
            resource.resources,
            capacity - resourcesHeld
        );       
        resourcesHeld += increment;
        resource.resources -= increment;
    }

    if(resourcesHeld == capacity)
        DropOff(); // Transition to our delivery state.
    else
        SearchForNearbyResources(); // Look for a new target.
}