casting a spell casts the wrong spell

[ThePotatomancer]

currently selecting a spell in a long sorted spell list will cast the wrong spell due to select spell returning a sorted spell index while handle_action.cpp looks only at the unsorted spells

Summary

Bugfixes "Fix issues with the spell menu where choosing a spell to cast chooses the wrong one"

Purpose of change

fixes #76042 (and duplicate #76192)
makes is so the spell a player selected is cast.
reproduction:

1. make a world with magicalysm or mind over matter installed
2. make a character with some stating spells (I used dimensional mage with ascended teleporter proficiency)
3. open a spell list, and try to select a spell to cast (any method)
4. look at log and confirm what spell was actually cast

Describe the solution

select_spell will now returns the selected spell instead of an index, making the internal sorting of the function irrelevant for handle_action.cpp

Describe alternatives you've considered

returning spell_id directly from the uilist instead of returning an index and converting to a spell. but I am not sure if spell_id is always int based, and there is no easy way I saw to convert an int to a string_id even if there is an easy way to convert it back to int from one, and returning a spell is more convenient.

Testing

The steps detailed in purpose of change, casting multiple spells, using hotkeys and mouse clicks. Also checking that casting spells works as expected when changing spell hotkeys.
enter spell menu and exit it without casting a spell (by pressing esc for example).

Additional context

[CLIDragon]

get_spell_index can be replaced by std::find, e.g.

original_spell_index = std::find(known_spells_sorted.begin(), known_spells_sorted.end(), selected_spell->id() )

Might be neater than creating a new function.

[ThePotatomancer]

@CLIDragon std::find return the element in question instead of the index from what I understood? In this case we need the index since the access to the element happens elsewhere (in handle action). I guess another possibility I haven't considered is changing the return type of select_spell to the actual spell/spell_id instead of the index. I might actually do that instead since it would be cleaner and more robust, just need to make sure no one besides handle_action uses select_spell

[CLIDragon]

@CLIDragon std::find return the element in question instead of the index from what I understood? In this case we need the index since the access to the element happens elsewhere (in handle action).

It returns an iterator which points to the location of the spell in the list. You can convert that to an index with std::distance if necessary, but I assumed (bad, I know) that the index was just being used to find an element, in which case an iterator would work equally as well.

[ThePotatomancer]

the reason I put it on draft is that I found a bug (when exiting spell menu without casting a spell, aka when selected id is negative). currently fixing along with the improvement of changing the return type of select spell instead of using the get_spell_index function

[ThePotatomancer]

took some effort due to my rusty cpp, but got it to where I wanted. Works with the new test and all the previous ones, and is much much cleaner and more error proof (since we now return the exact spell instead of a disconnected index).