-
-
Notifications
You must be signed in to change notification settings - Fork 566
Avoid context specific songs playing in a row #6319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Avoid context specific songs playing in a row #6319
Conversation
hhyyrylainen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is slightly different than what I had in mind. Or at least I think so.
To me it looks like from the code that this suppresses context specific tracks after playing them once. However the issue calls for preventing a track from playing twice in a row. Meaning that after any other track has been played all the context specific tracks should again be eligible for playing.
|
That is correct, I originally went down that route, but ran into an issue trying to figure out the prior played song for comparison. The TrackList sets LastPlayedIndex and in other places uses that index on the tracks array to identify if the new song != old song, because the assumption is the list will always be in the same order. However, when the context random is hit a new array is created with new indexes, so I had different songs in the same index. My change kept growing as I tried to work around that, so I was thinking of other ways to approach the issue and in my opinion this approach is simpler and more consistent with how I typically think of a playlist. If I were adding a new music set to music_tracks.json the only option I would think wouldn't respect PlayedOnce would be EntirelyRandom and the other options would go 1-10 or a random order, but still hit all songs. However, I do recognize that since the contextual music is supposed to bring a consistent feel to an area of the game that playing Song A, Context Music, Song B, Context Music, Song C, Context Music does make sense. I can take another stab at the original changes. Would it be too expensive to store LastPlayedTrack instead of LastPlayedIndex for comparison, or I could compare against something unique per track like resourcePath or a generated id of some kind? |
|
I think it would be a fine enough change for the jukebox class to remember what track it last played as I can foresee quite many situations where it is desirable to skip the immediately previously played track no matter what. |
|
We are currently in feature freeze until the next release. |
Brief Description of What This PR Does
Add a check for context specific songs to ensure they are only played once in the category rotation. Once all songs have been played in a category and played once is reset then the context specific song would be played again.
NOTE: It seems like the randomness of this should be removed and always have the context specific song play first and then allow other songs to be played.
Related Issues
closes #5615
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.