[melody] Internal logic improvements / fixes

[cord-b]

Internal Logic Improvements and Cleanup for Melody

Summary

• Eliminated several edge cases where Melody could skip songs forward/backward incorrectly.

Internal Changes

These changes are aimed as fixing some current bugs and possible future bugs by future-proofing a lot of the logic in here to work even if non-songs are thrown into the mix of events happening.

• Store casting_melody_spell_id state variable to improve Melody rewind detection by always knowing what the current in-progress Melody song is, rather than relying on the current_index, which isn't always an accurate depiction of the casting state.
• Simplified stop_current_cast() to interrupt whatever it is currently casting.
• Improved handle_stop_cast_callback() to correctly differentiate between the Melody song being interrupted versus some other spell ID that may trigger a stopcast event.
• The checks for auto-skipping targeted songs when missing a target now also supports "Targetted AE"-type spells (for Denon`s Desperate Dirge).

Notes

• These core changes were split out and refined based on this earlier PR.
  • (On Hold) [melody] non-casting state between songs #81

[cord-b]

Ran through the test cases in melody.cpp and all pass for me.
This PR does not address the 'timing window vulnerability' known issue.

[iamclint]

Will check this out this evening seems decent enough but I'm only looking on my phone atm.

[CoastalRedwood]

I think the changes are worthwhile overall and remove some assumptions in the previous code.

I left some nitpicky comments. The only one I particularly would prefer changed is the extra rewind logic, especially w/out any retry limit catch.

[iamclint]

They are shorts only because it felt better checking for -1 vs word max 0xffff
If you want we can change it all to word and just create a macro in structs header for invalidspellid

[CoastalRedwood]

I'd just vote to be consistent, leaning towards whatever we set in EqStructures.h being the source of truth. I skimmed it and there are a fair number of SpellId entries all labeled as WORD, so I'd lean towards standardizing on WORD and defining that invalid (or none?) spell_id constant.

[cord-b]

I'm fine with either one. For now I went with WORD consistency within Melody, and it shouldn't have had any impact on the rest of the code. Broad consistency changes would maybe make sense in a different PR (e.g. standardizing ActorInfo->CastingSpellId to a short like the rest of spells, if that's the intended direction).

[cord-b]

The previous commit should have cleaned up all the feedback items.

[CoastalRedwood]

Look good to me if, as long as Salty is happy with the EqStructures updates.

[cord-b]

Yeah as far as the cast() and stop_cast() function changes in EqStructures:

• EQCHARINFO::cast() is only used here and by /useitem, and /useitem always passes 0 for spell_id, so should be harmless
• EQCHARINFO::stop_cast() is not used by anything.