Stereo Mode: Add "shf" (Shuffle) option

This basically came to me in a dream, who am I to look the horse in its mouth

- Press to start a shuffled sequence, losing your current position in the autosequence.
    - Press again to disable, but keep your current track.
- Adjust horizontal offset of Stereo buttons slightly.
- More judiciously comment Sound Test functionality, to assist future maintainers.

src/k_menu.h

@@ -1309,6 +1309,7 @@ typedef enum
 	stereospecial_pause,
 	stereospecial_play,
 	stereospecial_seq,
+	stereospecial_shf,
 	stereospecial_vol,
 	stereospecial_track,
 } stereospecial_e;

src/k_menudraw.c

@@ -7089,6 +7089,11 @@ void M_DrawSoundTest(void)
 				if (soundtest.autosequence == true)
 					y = currentMenu->y + 6;
 			}
+			else if (currentMenu->menuitems[i].mvar2 == stereospecial_shf) // shf
+			{
+				if (soundtest.shuffle == true)
+					y = currentMenu->y + 6;
+			}
 
 			// Button is being pressed
 			if (i == itemOn && !soundtest.justopened && M_MenuConfirmHeld(pid))
@@ -7199,7 +7204,7 @@ void M_DrawSoundTest(void)
 			V_DrawCenteredThinString(x + 13, y + 1, 0, currentMenu->menuitems[i].text);
 		}
 
-		x += 27;
+		x += 25;
 	}
 
 	V_DrawCharacter(cursorx - 4, currentMenu->y - 8 - (skullAnimCounter/5),

src/menus/transient/sound-test.c

@@ -82,6 +82,26 @@ static void M_SoundTestSeq(INT32 choice)
 	soundtest.autosequence ^= true;
 }
 
+static void M_SoundTestShf(INT32 choice)
+{
+	(void)choice;
+
+	if (soundtest.shuffle)
+	{
+		soundtest.shuffle = false;
+		soundtest.sequence.shuffleinfo = 0;
+	}
+	else
+	{
+		S_SoundTestStop();
+
+		soundtest.playing = true;
+		soundtest.autosequence = true;
+		soundtest.shuffle = true;
+		S_UpdateSoundTestDef(false, false, false);
+	}
+}
+
 consvar_t *M_GetSoundTestVolumeCvar(void)
 {
 	if (soundtest.current == NULL)
@@ -160,18 +180,20 @@ static void M_SoundTestTick(void)
 menuitem_t MISC_SoundTest[] =
 {
 	{IT_STRING | IT_CALL,   "Back",  "STER_IC0", NULL, {.routine = M_GoBack},               0,  stereospecial_back},
-	{IT_SPACE, NULL, NULL, NULL, {NULL}, 11, 0},
+	{IT_SPACE, NULL, NULL, NULL, {NULL}, 6, 0},
 	{IT_STRING | IT_CALL,   "Stop",  "STER_IC1", NULL, {.routine = M_SoundTestMainControl}, 0,  0},
-	{IT_SPACE, NULL, NULL, NULL, {NULL},  8, 0},
+	{IT_SPACE, NULL, NULL, NULL, {NULL},  6, 0},
 	{IT_STRING | IT_CALL,   "Pause", "STER_IC2", NULL, {.routine = M_SoundTestMainControl}, 2,  stereospecial_pause},
 	{IT_STRING | IT_CALL,   "Play",  "STER_IC3", NULL, {.routine = M_SoundTestMainControl}, 1,  stereospecial_play},
-	{IT_SPACE, NULL, NULL, NULL, {NULL},  8, 0},
+	{IT_SPACE, NULL, NULL, NULL, {NULL},  6, 0},
 	{IT_STRING | IT_CALL,   "Prev",  "STER_IC4", NULL, {.routine = M_SoundTestNextPrev},   -1,  0},
 	{IT_STRING | IT_CALL,   "Next",  "STER_IC5", NULL, {.routine = M_SoundTestNextPrev},    1,  0},
-	{IT_SPACE, NULL, NULL, NULL, {NULL},  8, 0},
+	{IT_SPACE, NULL, NULL, NULL, {NULL},  6, 0},
 	{IT_STRING | IT_ARROWS, "Seq",   "STER_IC6", NULL, {.routine = M_SoundTestSeq},         0,  stereospecial_seq},
+	{IT_STRING | IT_ARROWS, "Shf",  "STER_IC7", NULL, {.routine = M_SoundTestShf},        0,  stereospecial_shf},
 	{IT_SPACE, NULL, NULL, NULL, {NULL}, 0, 244},
 	{IT_STRING | IT_ARROWS, "Vol",   NULL,       NULL, {.routine = M_SoundTestVol},         0,  stereospecial_vol},
+	{IT_SPACE, NULL, NULL, NULL, {NULL},  2, 0},
 	{IT_STRING | IT_ARROWS, "Track", NULL,       NULL, {.routine = M_SoundTestTrack},       0,  stereospecial_track},
 };
 

src/s_sound.c

@@ -32,7 +32,7 @@
 #include "lua_hook.h" // MusicChange hook
 #include "byteptr.h"
 #include "k_menu.h" // M_PlayMenuJam
-#include "m_random.h" // P_RandomKey
+#include "m_random.h" // M_RandomKey
 #include "i_time.h"
 #include "v_video.h" // V_ThinStringWidth
 #include "music.h"
@@ -1366,6 +1366,10 @@ void S_PopulateSoundTestSequence(void)
 	if (soundtest.sequence.id == 0)
 		soundtest.sequence.id = 1;
 
+	// Prepare shuffle material.
+	soundtest.sequence.shuffleinfo = 0;
+	soundtest.sequence.shufflenext = NULL;
+
 	soundtest.sequence.next = NULL;
 
 	tail = &soundtest.sequence.next;
@@ -1437,6 +1441,11 @@ void S_PopulateSoundTestSequence(void)
 
 		for (def = musicdefstart; def; def = def->next)
 		{
+			// This is the simplest set of checks,
+			// so let's wipe the shuffle data here.
+			def->sequence.shuffleinfo = 0;
+			def->sequence.shufflenext = NULL;
+
 			if (def->sequence.id == soundtest.sequence.id)
 				continue;
 
@@ -1467,12 +1476,11 @@ static boolean S_SoundTestDefLocked(musicdef_t *def)
 
 void S_UpdateSoundTestDef(boolean reverse, boolean dotracks, boolean skipnull)
 {
-	musicdef_t *newdef;
+	musicdef_t *newdef = NULL;
-
-	newdef = NULL;
 
 	if (reverse == false)
 	{
+		// Track update
 		if (dotracks == true && soundtest.current != NULL
 			&& soundtest.currenttrack < soundtest.current->numtracks-1)
 		{
@@ -1480,22 +1488,146 @@ void S_UpdateSoundTestDef(boolean reverse, boolean dotracks, boolean skipnull)
 			goto updatetrackonly;
 		}
 
-		newdef = (soundtest.current != NULL)
+		if (soundtest.shuffle == true && soundtest.sequence.shuffleinfo == 0)
-			? soundtest.current->sequence.next
-			: soundtest.sequence.next;
-		while (newdef != NULL && S_SoundTestDefLocked(newdef))
-			newdef = newdef->sequence.next;
-		if (newdef == NULL && skipnull == true)
 		{
+			// The shuffle data isn't initialised.
+			// Count the valid set of musicdefs we can randomly select from!
+			// This will later liberally be passed to M_RandomKey.
+
 			newdef = soundtest.sequence.next;
+			while (newdef != NULL)
+			{
+				if (S_SoundTestDefLocked(newdef) == false)
+				{
+					newdef->sequence.shuffleinfo = 0;
+					soundtest.sequence.shuffleinfo++;
+				}
+				else
+				{
+					// Don't permit if it gets unlocked before shuffle count gets reset
+					newdef->sequence.shuffleinfo = (size_t)-1;
+				}
+				newdef->sequence.shufflenext = NULL;
+
+				newdef = newdef->sequence.next;
+			}
+			soundtest.sequence.shufflenext = NULL;
+		}
+
+		if (soundtest.shuffle == true)
+		{
+			// Do we have it cached..?
+			newdef = soundtest.current != NULL
+				? soundtest.current->sequence.shufflenext
+				: soundtest.sequence.shufflenext;
+
+			if (newdef != NULL)
+				;
+			else if (soundtest.sequence.shuffleinfo != 0)
+			{
+				// Nope, not cached. Grab a random entry and hunt for it.
+				size_t shuffleseek = M_RandomKey(soundtest.sequence.shuffleinfo);
+				size_t shuffleseekcopy = shuffleseek;
+
+				// Since these are sequential, we can sometimes
+				// get a small benefit by starting partway down the list.
+				if (
+					soundtest.current != NULL
+					&& soundtest.current->sequence.shuffleinfo != 0
+					&& soundtest.current->sequence.shuffleinfo <= shuffleseek
+				)
+				{
+					newdef = soundtest.current;
+					shuffleseek -= (soundtest.current->sequence.shuffleinfo - 1);
+				}
+				else
+				{
+					newdef = soundtest.sequence.next;
+				}
+
+				// ...yeah, though, this is basically O(n). I could provide a
+				// great many excuses, but the basic impetus is that I saw
+				// a thread on an open-source software development forum where,
+				// since 2014, a parade of users have been asking for the same
+				// basic QoL feature and been consecutively berated by one developer
+				// extremely against the idea of implmenting something imperfect.
+				// I have enough self-awareness as a programmer to recognise that
+				// that is a chronic case of "PROGRAMMER BRAIN". Sometimes you
+				// just need to do a feature "badly" because it's more important
+				// for it to exist at all than to channel mathematical elegance.
+				// ~toast 220923
+
+				for (; newdef != NULL; newdef = newdef->sequence.next)
+				{
+					if (newdef->sequence.shuffleinfo != 0)
+						continue;
+
+					if (S_SoundTestDefLocked(newdef) == true)
+						continue;
+
+					if (shuffleseek != 0)
+					{
+						shuffleseek--;
+						continue;
+					}
+					break;
+				}
+
+				if (newdef == NULL)
+				{
+					// Fell short!? Try again later
+					soundtest.sequence.shuffleinfo = 0;
+				}
+				else
+				{
+					// Don't select the same entry twice
+					if (soundtest.sequence.shuffleinfo)
+						soundtest.sequence.shuffleinfo--;
+
+					// One-indexed so the first shuffled entry has a valid shuffleinfo
+					newdef->sequence.shuffleinfo = shuffleseekcopy+1;
+
+					// Link it to the end of the chain
+					if (soundtest.current && soundtest.current->sequence.shuffleinfo != 0)
+					{
+						soundtest.current->sequence.shufflenext = newdef;
+					}
+					else
+					{
+						soundtest.sequence.shufflenext = newdef;
+					}
+				}
+			}
+		}
+		else
+		{
+			// Just blaze through the musicdefs
+			newdef = (soundtest.current != NULL)
+				? soundtest.current->sequence.next
+				: soundtest.sequence.next;
 			while (newdef != NULL && S_SoundTestDefLocked(newdef))
 				newdef = newdef->sequence.next;
+
+			if (newdef == NULL && skipnull == true)
+			{
+				newdef = soundtest.sequence.next;
+				while (newdef != NULL && S_SoundTestDefLocked(newdef))
+					newdef = newdef->sequence.next;
+			}
 		}
 	}
 	else
 	{
+		// Everything in this case is doing a full-on O(n) search
+		// for the previous entry in one of two singly linked lists.
+		// I know there are better solutions. It basically boils
+		// down to the fact that this code only runs on direct user
+		// input on a menu, never in the background, and therefore
+		// is straight up less important than the forwards direction.
+
 		musicdef_t *def, *lastdef = NULL;
 
+		// Track update
 		if (dotracks == true && soundtest.current != NULL
 			&& soundtest.currenttrack > 0)
 		{
@@ -1503,6 +1635,34 @@ void S_UpdateSoundTestDef(boolean reverse, boolean dotracks, boolean skipnull)
 			goto updatetrackonly;
 		}
 
+		if (soundtest.shuffle && soundtest.current != NULL)
+		{
+			// Basically identical structure to the sequence.next case... templates might be cool one day
+
+			if (soundtest.sequence.shufflenext == soundtest.current)
+				;
+			else for (def = soundtest.sequence.shufflenext; def; def = def->sequence.shufflenext)
+			{
+				if (!S_SoundTestDefLocked(def))
+				{
+					lastdef = def;
+				}
+
+				if (def->sequence.shufflenext != soundtest.current)
+				{
+					continue;
+				}
+
+				newdef = lastdef;
+				break;
+			}
+
+			goto updatecurrent;
+		}
+
+		soundtest.shuffle = false;
+		soundtest.sequence.shuffleinfo = 0;
+
 		if (soundtest.current == soundtest.sequence.next
 			&& skipnull == false)
 		{
@@ -1617,6 +1777,8 @@ void S_SoundTestStop(void)
 
 	soundtest.playing = false;
 	soundtest.autosequence = false;
+	soundtest.shuffle = false;
+	soundtest.sequence.shuffleinfo = 0;
 
 	Music_Stop("stereo");
 	Music_Stop("stereo_fade");

src/s_sound.h

@@ -150,6 +150,9 @@ struct soundtestsequence_t
 	UINT8 id;
 	UINT16 map;
 	musicdef_t *next;
+
+	size_t shuffleinfo;
+	musicdef_t *shufflenext;
 };
 
 // Music credits
@@ -195,6 +198,7 @@ extern struct soundtest
 	soundtestsequence_t sequence;		// Sequence head
 
 	boolean autosequence;				// In auto sequence mode?
+	boolean shuffle;					// In shuffle mode;
 } soundtest;
 
 void S_PopulateSoundTestSequence(void);