update ScreenSelectMusic's SortMenu UIUX and data structure

[quietly-turning]

UIUX Changes

This changes the UI for ScreenSelectMusic's SortMenu's choices (like "sort by title", "sort by artist", "change style to double") visually distinct from folders-of-choices (like "Sorts", "Options").

Folders-of-choices now:

• have a background quad
• have left-aligned text
• include a folder icon

The recent change that allowed the SortMenu to use a folder/nested-choice structure was a good change(!), though I sometimes found myself getting visually lost in the menu. My primary goal with this PR was to make folder-rows more visually distinct from choice-rows.

The folder icon comes from https://github.com/tailwindlabs/heroicons, which is MIT licenced.

This also changes the UX of SSM's SortMenu folder to behave similar to the engine's MusicWheel: toggling a folder open reveals new rows below it, and opening one new folder auto-closes the previously-open folder.

ScreenFlow.mp4

---

Code Changes

On the code side of things, there's 3 main changes:

1. I moved a lot of code out of default.lua. My motivation was to be looking at fewer lines-of-code at any given moment.

• moved InputHandlers into a discrete directory for InputHandlers
• moved all SongSearch code into the existing SongSearch directory
• moved wheel_choices into discrete file

2. I restructured SortMenu's primary wheel_choices data structure to an array of keyed tables, each representing one folder:

• name (string)
• open (boolean)
• children (table).

The children table is an array with each item in the format of

{{ top_text, bottom_text, action_if_chosen}, condition_to_be_visible }

My motivation was to have a data structure with a more-clear distinction between a folder-row and a choice-row.

You can see this new structure here:

Simply-Love-SM5/BGAnimations/ScreenSelectMusic overlay/SortMenu/SortMenuRows.lua

Lines 4 to 84 in e0110ca

	-- `wheel_options` is the table that defines the SortMenu's choices
	-- children row structure is:
	-- { {top_text, bottom_text, action_if_chosen}, condition_to_be_visible }
	--
	-- top_text is a string
	-- bottom_text is a string
	-- action_if_chosen is a reference to a function
	-- condition_to_be_visible can be either:
	-- • statement that evaluates to a boolean,
	-- • function that returns a boolean
	--
	-- top_text, bottom_text, and action_if_chosen are required
	-- condition_to_be_visible is optional, and assumed true (row is visible in SortMenu) if absent
	local wheel_options = {
	{
	name="FolderCommon",
	open=true,
	children={
	{ {"SortBy", "Group", ChangeSort} },
	{ {"SortBy", "Title", ChangeSort} },
	{ {"SortBy", "Recent", ChangeSort} },
	-- Casual players often accidentally choose ITG mode and an experienced player in the area may notice this
	-- and offer to switch them back to Casual mode using this option in the SortMenu.
	{ {"ChangeMode", "Casual", ChangeMode}, SL.Global.Stages.PlayedThisGame == 0 },
	{ {"ImLovinIt", "AddFavorite", AddSongToFavorites}, function() return GAMESTATE:GetCurrentSong() ~= nil end },
	{ AddFavoritesRow(PLAYER_1), function() return GAMESTATE:IsHumanPlayer(PLAYER_1) end },
	{ AddFavoritesRow(PLAYER_2), function() return GAMESTATE:IsHumanPlayer(PLAYER_2) end },
	{ {"GrooveStats", "Leaderboard", ShowLeaderboard}, function() return GAMESTATE:GetCurrentSong() ~= nil end },
	}
	},
	{
	name="FolderFindASong",
	open=false,
	children={
	{ { "WhereforeArtThou", "SongSearch", ShowSongSearch}, not GAMESTATE:IsCourseMode() and ThemePrefs.Get("KeyboardFeatures")},
	{ {"SortBy", "Group", ChangeSort} },
	{ {"SortBy", "Title", ChangeSort} },
	{ {"SortBy", "Artist", ChangeSort} },
	{ {"SortBy", "Genre", ChangeSort} },
	{ {"SortBy", "BPM", ChangeSort} },
	{ {"SortBy", "Length", ChangeSort} },
	{ {"SortBy", "Meter", ChangeSort} },
	{ {"SortBy", "Popularity", ChangeSort} },
	{ {"SortBy", "Recent", ChangeSort} },
	{ {"SortBy", "TopGrades", ChangeSort} },
	{ {"SortBy", "PopularityP1", ChangeSort}, function() return PROFILEMAN:IsPersistentProfile(PLAYER_1) end },
	{ {"SortBy", "RecentP1", ChangeSort}, function() return PROFILEMAN:IsPersistentProfile(PLAYER_1) end },
	{ {"SortBy", "TopP1Grades", ChangeSort}, function() return PROFILEMAN:IsPersistentProfile(PLAYER_1) end },
	{ {"SortBy", "PopularityP2", ChangeSort}, function() return PROFILEMAN:IsPersistentProfile(PLAYER_2) end },
	{ {"SortBy", "RecentP2", ChangeSort}, function() return PROFILEMAN:IsPersistentProfile(PLAYER_2) end },
	{ {"SortBy", "TopP2Grades", ChangeSort}, function() return PROFILEMAN:IsPersistentProfile(PLAYER_2) end },
	}
	},
	{
	name="FolderAdvanced",
	open=false,
	children={
	{ {"FeelingSalty", "TestInput", ShowTestInput }, GAMESTATE:IsEventMode() },
	{ {"HardTime", "PracticeMode", ShowPracticeMode }, function() return GAMESTATE:IsEventMode() and GAMESTATE:GetCurrentSong() ~= nil and ThemePrefs.Get("KeyboardFeatures") end },
	{ {"TakeABreather", "LoadNewSongs", ShowLoadNewSongs } },
	{ {"NeedMoreRam", "ViewDownloads", ShowDownloads }, DownloadsExist },
	{ {"NextPlease", "SwitchProfile", ShowSelectProfile }, ThemePrefs.Get("AllowScreenSelectProfile") },
	{ {"SetSummaryText", "SetSummary", ShowSetSummary }, SL.Global.Stages.PlayedThisGame > 0 },
	}
	},
	{
	name="FolderStyles",
	open=false,
	children=GetChangeableStylesRows,
	},
	{
	name="FolderPlaylists",
	open=false,
	children=AddPlaylistsRows,
	},
	}

3. I moved "action" code (what happens when the user presses START on a given choice in the SortMenu) out of SortMenu_InputHandler, into a dedicated file for reusable helper functions, and those helper functions are referenced as the 3rd element in each child row (denoted above as action_if_chosen).

The SortMenu_InputHandler was previously making some complicated inferences to determine what kind of row it was operating on, like knowing the current row was a folder because the top_text started with the substring "Category", or knowing the current row was a playlist because it had a new_overlay key. It worked, but the long if-else chain could be tricky for an unfamiliar dev to work with.

I think the new structure should be easier for future devs to jump into — the function to dictate what happens when a row is chosen is bundled with that row.

---

In all, I changed a lot of code, so feel free to ask questions! I did try to write lots of new inline comments where I thought they'd help.

It'd be good if @CrashCringle12 could test this branch and both ensure I didn't break existing functionality and give feedback.

[quietly-turning]

Is there a better way to do an "import" like this in StepMania-flavored Lua?

[quietly-turning]

Give me, like, one more day to add to language files to the best of my ability and to clean up one more piece of code.

[quietly-turning]

Give me, like, one more day to add to language files to the best of my ability and to clean up one more piece of code.

Okay, I'm good now. :)

I included an additional commit to incorporate @CrashCringle12's proposed changes from #665 that exposed the SortMenu's wheel_options to SL modules.

[quietly-turning]

@SimplyViper16 @JustMoneko @TheBreak1
Sorry to ping you! 😅

Can you help proofread the machine translation I used in this PR? Thank you for considering!

[JustMoneko]

Re: translation
For pl.ini it's left mostly in english, and some of the lines are taken out of context. Can look into fixing it up late this weekend

[JustMoneko]

Bit late, but made a pr.

You can also copy this if it's easier for you:

FolderCommon=Ogólne
FolderFindASong=Znajdź utwór
FolderStyles=Zmień styl
FolderAdvanced=Zaawansowane
FolderPlaylists=Playlisty
# ToggleFolder= intentionally empty (would appear as "Top Text" for SortMenu folder)
ToggleFolder=

# Favorites / Playlists Text
AddFavorite=Dodaj do ulubionych
ImLovinIt=Daj serduszko
Preferred=Ulubione
P1Preferred=Ulubione P1
P2Preferred=Ulubione P2
MixTape=Obczaj moją składankę
NoPlayerFavoritesAvailable=%s nie posiada ulubionych!```

I will probably go and touch up the rest of the translation before the next release anyways, but sortmenu is good to go

[teejusb]

Just to confirm -- is #665 still required or would this PR cover it?

[teejusb]

Still reading through, but is there a way to create top level options (ones that aren't part of any folder?). I think that is a useful thing to support, especially if people want to have customized menus with quicker to access options.

[quietly-turning]

Just to confirm -- is #665 still required or would this PR cover it?

This should cover #665, though @CrashCringle12 should sign off on that actually being the case. :^)

---

Still reading through, but is there a way to create top level options (ones that aren't part of any folder?).

I removed support for choices-not-in-any-folder, though could add it to this PR without UI changes fairly easily. But, I wouldn't be in favor of that.

When thinking about the SorMenu and the issues I've had interacting with it over time (too many choices in the past, getting lost navigating it more recently), I designed this PR for:

• choices belong to folders
• folders cannot be nested
• choices appear different than folders
• opening one folder does not remove other folders from view

Allowing choices-in-folders and choices-not-in-folders to coexist feels like different interaction model, one I hadn't considered, so I'd need to think about how to present that.

[teejusb]

I do agree with the sort menu having the sort of choice overload, but one thing that has been brought by a good number of people is that with the current nesting, their more oft used options now take longer to get to (switch profile, leaderboard, etc.). While I don't think any choices-not-in-folders need to be default atm, I think the functionality should exist for people who want to lift their often used options out of a folder which will reduce an additional button press for them, especially now that it can be manipulated with modules.

[CrashCringle12]

• Yeah, i tested it out and yeah the change still works*. Though in practice...because the toptext is going to try to grab a theme string im realizing it'll never visually work without error. Thinking we should maybe add something to the structure of wheel_options that tells WheelItemMT to use the string that's there (similar to what we're doing for playlists already).

• I think Switch Profile should be in Common or the not-in-folder equivalent. In my experience visiting cabs it is far more commonly used than I thought when I tucked it in Advanced Options

• What do we think about moving some of the profile specific sorts into their own category? They can remain under Find a Song as well, but they're tucked pretty deep under the category. I think it's no longer immediately clear to a new user the menu wraps around so you would be inclined to scroll down all the way through to reach it.

• Start a 1 player game where the player has a profile selected. In the menu we'll see 'FAVORITES' under Common & Playlists which is expected. If you then join a second player with or without favorites.. 'FAVORITES' under Playlists changes to 'P1 Favorites' and a new option 'P2 Favorites' may appear; however, Favorites under Common remains 'FAVORITES' which I think implies any user who selects that we'll see their respective favorites when really this corresponds to P1's favorites specifically. P2s favorites do not appear until leaving the screen.

• I wonder if 'Find a Song' is the best name since I feel like the clear rank sorts and most played are less 'find a song' and looking at your profile deets. I dont have really any hard thoughts on this but i think implying we are now in the actual sort menu here would be good.

• If you have 2 players joined both with favorites and then 1 player unjoins, selecting 'Favorites' or 'P# Favorites' will say that the player does not have favorites.

• I don't think I'm in favor of splitting up Favorites into 2 options instead of 1. Curious of folks thoughts on this previous behavior had Favorites show up in the sort menu and depending on the player who selects it you'll get their respective favorites. I can see that this technically adds clarity to what happens, but I think it's much gain? 'Add to Favorites' is not listed as 'Add to P1 Favorites' 'Add to P2 Favorites' and instead operates on whoever selects it thats whose favorites it gets added to. I think both Adding and viewing favorites should maintain this functionality. Fwiw ik I added a lot of the additional sorts to the game and theme like this that have the P1/P2 distinction but I always figured once stuff got refractored we could make it so we could condense those options to 1 that works for both players instead of 2 for each. I think we could do that more easily now and maybe slide all those under their own category?

As an example "P1 Recently Played" and "P2 Recently Played" being turned into 1 "My Recently Played" (or something like that) and clicking on that calls a function similar to ChangeSort() except it takes a player as an argument to determine which sort to change to.

local function PlayRecent(player)
	if player == nil then return end
	if "P1" == ToEnumShortString(player) then
		MESSAGEMAN:Broadcast('Sort', { order = "RecentP1" })
	elseif "P2" == ToEnumShortString(player) then
		MESSAGEMAN:Broadcast('Sort', { order = "RecentP2" })
	end

	MESSAGEMAN:Broadcast('ResetHeaderText')
	SCREENMAN:GetTopScreen():GetChild("Overlay"):queuecommand("DirectInputToEngine")
end
-- (In practice probs should be more generic but writing it out like this for the example)

I think that would clean up a lot of the options and make it a lot easier for folks to scroll through.

[quietly-turning]

Thanks for the thorough review, @CrashCringle12!

The bugs you found could be fixed, but you've given me a lot to think about (e.g. choices that affect both players vs. choices that affect one player) and I'm no longer confident this PR is a good change I want to work more on.

I'll reflect on it for a week or so and respond here.

[quietly-turning]

because the toptext is going to try to grab a theme string im realizing it'll never visually work without error.

Fixed in 163ad96 and e5f7d1c

I also added validation to some SortMenu helper functions in 263ac2a, since I figured user-supplied modules may try to set invalid SortOrders or Styles.

---

Start a 1 player game where the player has a profile selected. [...]

Fixed in d9a4f14

---

I think Switch Profile should be in Common or the not-in-folder equivalent.

Added Switch Profile to Common in d9a4f14. It's also in Advanced.

---

If you have 2 players joined both with favorites and then 1 player unjoins [...]

Should be fixed by d9a4f14

---

I don't think I'm in favor of splitting up Favorites into 2 options instead of 1. [...]

Condensed distinct "P1 Favorites" and "P2 Favorites" WheelItems into just "Favorites" in d9a4f14

---

What do we think about moving some of the profile specific sorts into their own category? [...]

I wonder if 'Find a Song' is the best name [...]

"P1 Recently Played" and "P2 Recently Played" being turned into 1 "My Recently Played" [...]

I appreciate that you're thinking about this, and I think further reorganization of this SortMenu can be a separate PR effort led by someone else.

Working on this SortMenu reskin over the past month has helped show me all the ways the UX model still falls short (too many similar-looking, secret-context-dependent choices crammed into too small a space), so the next time I work on this, it'll be a wholly-new overlay with different UIUX.

For now, I think this is a minor improvement over the previous SortMenu.