Add melody refinements #59
Conversation
CoastalRedwood
commented
Jun 15, 2024
CoastalRedwood
commented
- Improved error messages (clarity, fewer silent failures)
- /stopsong no longer required to switch to a new melody
- /melody with an active melody transitions at end of current song
- Additional defensive programming to stop corner cases
- Paranoid checking of all pointers
- Additional constraints checking on start
- Tighter control that songs and current_index are always kept valid (or disabled)
- Retry (missed note) logic improvements:
- correct wraparound for 0 index retry
- retry_count tracking to constrain log spam in error cases
- auto-advances to next song if a song fails 8 times, terminates melody if 15 retries w/out success
- Melodies now end when sitting (and thus camping, eliminates some corner cases and surprises)
- Improved error messages (clarity, fewer silent failures) - /stopsong no longer required to switch to a new melody - /melody with an active melody transitions at end of current song - Additional defensive programming to stop corner cases - Paranoid checking of all pointers - Additional constraints checking on start - Tighter control that songs and current_index are always kept valid (or disabled) - Retry (missed note) logic improvements: - correct wraparound for 0 index retry - retry_count tracking to constrain log spam in error cases - auto-advances to next song if a song fails 8 times, terminates melody if 15 retries w/out success - Melodies now end when sitting (and thus camping, eliminates some corner cases and surprises)
|
Please take a look when you have time. I went a little OCD and touched a lot of it to make it as solid as possible (we bards use it a lot), there are just a few rare corner cases that I wanted to improve with some light refactoring of some patches. The primary intentional changes are above. I think the majority of bards will appreciate the retry limits (8 is sufficiently generous in my experience). The melody terminating when sitting could annoy someone, but I couldn't think of a use case where a bard would sit and need to auto-resume melody (versus looting which is common). The testing I did (on Icestorm's server) is described in the .cpp. If the changes are just too much, I could try again with smaller patches. |
|
Thanks for approving! Just let me know if you want any (now or future) changes, I'm happy to help out with maintaining melody when useful. I just pushed a trivial second commit to add /mel as an alias. I'm fine with reverting or modifying it. Thanks! Sherra (on DIscord) |
|
Np i'll merge it in when I bring the dev branch in to main |
