Fix exception thrown during Lycanthropy transition

[KABoissonneault]

A user on Discord sent me this error during gameplay.

This error is easy to reproduce.

1. Load any save
2. Use console command diseaseplayer werewolf to get Lycanthropy Infection
3. Wait 1 day (ex: travel anywhere), get the dream sequence
4. Wait 2 another day (ex: travel anywhere 2+ days away)
5. See error in logs / console

So, during EntityEffectManager.DoMagicRound(), we have this loop

foreach (LiveEffectBundle bundle in instancedBundles)
{
    // Run effects for this bundle 
    bool hasRemainingEffectRounds = false;
    foreach (IEntityEffect effect in bundle.liveEffects)
    {
        // Update effects with remaining rounds, item effects are always ticked
        // All other effects with a duration will be ended at zero rounds remaining
        hasRemainingEffectRounds = hasRemainingEffectRounds || effect.RoundsRemaining > 0;
        if (effect.RoundsRemaining > 0 || bundle.fromEquippedItem != null)
            effect.MagicRound();
        else if (effect.Properties.SupportDuration)
            effect.End();
    }
   ...

For the LycanthropyInfection effect, MagicRound updates the disease progress, until 3 days have passed and the dream sequence has been watched. This will call DeployFullBlownLycanthropy(), which is implemented by WereboarInfection and WerewolfInfection as this

protected override void DeployFullBlownLycanthropy()
{
    // Start permanent wereboar lycanthropy effect stage two
    EntityEffectBundle bundle = GameManager.Instance.PlayerEffectManager.CreateLycanthropyCurse();
    GameManager.Instance.PlayerEffectManager.AssignBundle(bundle, AssignBundleFlags.BypassSavingThrows);
    LycanthropyEffect lycanthropyEffect = (LycanthropyEffect)GameManager.Instance.PlayerEffectManager.FindIncumbentEffect<LycanthropyEffect>();
    if (lycanthropyEffect != null)
        lycanthropyEffect.InfectionType = LycanthropyTypes.Wereboar;
}

This is all happening still in the middle of the foreach loop above. As you can see, we're assigning a new bundle to the player's effect manager, which modifies instancedBundles if the bundle has any "live" effects (which Lycanthropy curse does). Modifying collections during iteration is not supported, so the error occurs.

While the error does not break anything permanently as far as we know, it still prevents other effects from being processed for the rest of this magic round. This could happen more often with mods.

[Interkarma]

I've approved this one, it fixes the problem for any effects that could trigger this (including vampirism, I assume, which works similarly).
I'd also like to offer an alternative approach. EntityEffectManager.DoMagicRound() uses bundlesToRemove collection to determine bundles scheduled for removal post magic round update process.
Of course, the problem here is that a bundle is being added to the collection mid-iteration, for which there's no current handling. So another approach would be to do something similar to bundlesToRemove except with adding bundles.
But cloning out the collection like you do here is perfectly fine. It's simple and easy to understand. There's only a small number of elements in instancedBundles at any one time, and they only tick every 5 seconds. So I have no problems if you folks run with this fix as-is.

[KABoissonneault]

I'd also like to offer an alternative approach. EntityEffectManager.DoMagicRound() uses bundlesToRemove collection to determine bundles scheduled for removal post magic round update process.

I have considered it after your post. However, there is one issue that bugs me.
To implement this, we would have to add to a new bundlesToAdd list in AssignBundle, sure. However, the code currently looks like

            // Add bundles with at least one effect
            if (instancedBundle.liveEffects.Count > 0)
            {
                instancedBundles.Add(instancedBundle);
                RaiseOnAssignBundle(instancedBundle);
                Debug.LogFormat("Adding bundle [{0}] {1}", instancedBundle.name, instancedBundle.GetHashCode());
            }

Each time we add a bundle, there is an event. In the current code, it means that when OnAssignBundle is raised, the new bundle is already part of instancedBundles. If we change that line to bundlesToAdd.Add(instancedBundle);, I fear some mod may rely on the new bundle already being part of instancedBundles somehow, while the change would make it missing from that list until the next call to AddPendingBundles().
This includes most functions in EntityEffectManager, CureAllDiseases, GetDiseaseCount,. GetDiseasebundles(), etc...
All these functions would have to be updated to include both instancedBundles and bundlesToAdd, so that mods responding to OnAssignBundle never miss any.

I feel this would be more complicated than the bundlesToRemove case, which never calls into external code, thus is never observable by mods.

In short, nah, not doing that :)