Ability to change the music sort order of the Options - Music and Edit Music Selection windows#2695
Ability to change the music sort order of the Options - Music and Edit Music Selection windows#2695LeeSpork wants to merge 6 commits intoOpenLoco:masterfrom
Conversation
Moves logic for getting the list of available tracks from Options.cpp to Audio.cpp, and has it sort the music either alphabetically or by era.
Changes that should have no effect on functionality following the "don't repeat yourself" principle.
| 24.10+ (???) | ||
| ------------------------------------------------------------------------ | ||
| - Fix: [#2676] Cargo labels are misaligned in vehicle window. | ||
| - Feature: [#2695] Ability to change the music sort order of the Options - Music and Edit Music Selection windows. |
There was a problem hiding this comment.
In the changelog we list Features before Fixes. I know this isn't obvious, sorry! (but you can see the ordering if you look at the previous entries)
| 2357: "{COLOUR WINDOW_2}Sort music by:" | ||
| 2358: "Original order" | ||
| 2359: "Alphabetical order" | ||
| 2360: "Reverse alphabetical order" |
There was a problem hiding this comment.
I think Alphabetical (reversed) would be more consistent with the Era (reversed) below. I also think you don't need to write order on the first 3 items. Also I think Year sounds more natural than Era. I would do
- Original
- Alphabetical
- Alphabetical (reversed)
- Year
- Year (reversed)
| switch (Config::get().sortMusicBy) | ||
| { | ||
| case Config::MusicSortType::original: | ||
| // Assume it is already in original order |
There was a problem hiding this comment.
regardless of what the initial state is, if the player sets another sort (eg alpabetical) and then switches back to original, you're gonna need to sort it
|
|
||
| static constexpr uint8_t kRowHeight = 12; // CJK: 15 | ||
|
|
||
| static std::vector<uint8_t> _musicTracks; |
There was a problem hiding this comment.
I'm not a fan of making this static, but I can see you've done it because the mouse scroll handler is static. Having a more OOP-style approach to these windows is what we're trying to do, so is it possible to make the mouse scroll handler not static? Or do you think this is the best way to do this here?
There was a problem hiding this comment.
I think I wanted it to be an attribute of a music selection window object, so making it static was my quick solution considering it doesn't appear to be an object/class currently (and I'm not sure I'm confident enough to personally rewrite an entire file to be object oriented yet lol)
| bool usePreferredOwnerFace; | ||
| ObjectHeader preferredOwnerFace; | ||
|
|
||
| MusicSortType sortMusicBy = MusicSortType::original; |
There was a problem hiding this comment.
just adding this here does nothing. you need to write the code to load and save it from config as well. this happens in readNew() and writeNewConfig() in Config.cpp, but its fairly straightforward
| auto comparison = strcoll(aTitle, bTitle) < 0; | ||
|
|
||
| if (reversed) | ||
| { | ||
| return !comparison; | ||
| } | ||
| return comparison; |
There was a problem hiding this comment.
this entire thing amounts to an XOR, so it is just
return !(strcoll(aTitle, bTitle) < 0) != !reversed;
though admittedly that is a little harder to read...
|
I don't think this makes sense. Why would you go into options to adjust the layout of a singular window. And why would we want to persist the order of the window between instances? The better approach would be to redesign the window to have multiple columns and allow sorting on those columns. Then you can add the information to the window like the era. Its how the vehicle list window works: |
|
That is a very good point for the "Edit Music Selection" window! Although there is also the "Currently Playing" dropdown in "Options - Music", which is currently always in order of the music's ID number, which I find weird (and mildly annoying if you want to switch the currently playing track in "Play all music" mode). I admit that this might not be the right solution tho. |
If it's about ordering a dropdown then i would say only do alphabetical there is no need to do anything too complex. It's not like there are that many tracks anyway. |
Adds a new drop-down option to the music options that changes the sorting of the game's music to either "Original order", "Alphabetical order", "Reverse alphabetical order", "Era", or "Era (reversed)". Changing your selection changes the order that the tracks appear in in the "Currently playing" drop-down, and in the customized selection of music's "Edit Music Selection" window.
Does not currently influence the order that the game automatically chooses to play the music in (it is still random).