Issue

In the following line in SpellResolver::resolveEffects() a new integer is used to iterate through all spells:

While the method SpellContext::nextSpell() uses an internal integer to resolve what the next spell is:

This can lead to overflow behavior if a spell effect / aug tries to use the SpellContext to access following spells.
I know this "should" not happen since it was probably not intended to be used like that, but it can be!

Who does this affect and how?

Mostly only add-on authors that want to touch the SpellContext to read following spells.
After the following spells have been consumed the spell has to be canceled completely or it will throw "Invalid spell cast found! This is a bug and should be reported!" in SpellContext::nextSpell().

Proposed Solution

I suggest making it so that SpellContext has a method SpellContext::hasNextSpell() (hasNextPart) which checks that:

• Spell::isValid()
• SpellContext::isCanceled()
• SpellContext::currentIndex won't be out of bounds (meaning more spells exist)

Meaning the code can then be rewritten as follows:

while( spellContext.hasNextPart() ) {
    // Remove spellContext.isCanceled() check. Not needed anymore
    AbstractSpellPart part = spellContext.nextSpell();
    // [...]
        .setAugments(spell.getAugments( spellContext.getCurrentIndex() , shooter))
    // [...]
}

Variation of Solution

Alternatively it could be decided to still have a split hasNextSpell and isCanceled check, in case something might want to proceed differently if it is canceled or if it is done.

This has been added to 1.18, let me know if anything looks unusual.

Thanks!