Implement quick spell cycling #6861
Conversation
| newIndex = *currentIndex == 0 ? (validHotKeyIndexes.size() - 1) : (*currentIndex - 1); | ||
| } | ||
| ToggleSpell(validHotKeyIndexes[newIndex]); | ||
| } |
There was a problem hiding this comment.
This looks.... more complicated than it needs to be, at least at first glance.
Have you considered just creating 2 separate functions, "NextSkill" and "PreviousSkill", instead of combining both here?
Also, isn't there something in C++ like a "circular iterator" that we could tap into? Then, it would be a matter of walking forward or backward on it, while checking for the first valid spell occurrence using those predicate lambdas.
I don't know..... this might be me just misremembering how awful C++ is, but this looks incredibly convoluted for such a simple operation conceptually. Maybe we are just lacking some abstraction here.
There was a problem hiding this comment.
I haven't given it much thought.
But I don't know anything about a circular iterator.
If you want you can give it a try to improve it. 🙂
|
What happens if the player is on a spell that isn't a hotkey spell and tries to do next/previous? Does it fail? I think that what I was seeing, but wasn't sure. If that's the case, should it just start from the beginning or end of the spell cycle, instead? Edit: or maybe it's undefined behavior, because |
| // This is a hack, mousewheel events should be handled directly by their consumers instead. | ||
| event->type = SDL_KEYDOWN; | ||
| if (e.wheel.y > 0) { | ||
| event->key.keysym.sym = (SDL_GetModState() & KMOD_CTRL) != 0 ? SDLK_KP_PLUS : SDLK_UP; |
There was a problem hiding this comment.
I'm super happy to see this gone, thank you! 🎉
Did we lose the Control + Wheel functionality to zoom the map with this change though?
There was a problem hiding this comment.
Yes this logic is also removed. You can still zoom with the +/- keys.
But if we want this back as special case, we can implement them the same way as scrolling in shops etc. is done. 🙂
The following behavior is implemented:
Thank you very much and have a wonderful Christmas. ❤️ |
Fixes #6858
Notes