Skip to content

Refactor music playback system: introduce MusicTrackManager, improve state handling, and remove legacy code#1139

Open
NSDeathman wants to merge 3 commits intosmartcmd:mainfrom
NSDeathman:main
Open

Refactor music playback system: introduce MusicTrackManager, improve state handling, and remove legacy code#1139
NSDeathman wants to merge 3 commits intosmartcmd:mainfrom
NSDeathman:main

Conversation

@NSDeathman
Copy link

This pull request overhauls the music playback logic in SoundEngine to make it more maintainable, readable, and less error‑prone. The main focus was on centralising track selection and eliminating a large amount of duplicated/platform‑specific code.

Key Changes
New MusicTrackManager class

Encapsulates track selection with per‑domain history tracking to avoid immediate repeats.

Supports domains: Menu, Overworld Survival, Overworld Creative, Nether, End.

Each domain stores its own index range and a bool[] of recently played tracks.

Removed legacy members and functions

Deleted m_iStream_* min/max variables and m_bHeardTrackA array.

Removed GetRandomishTrack() – logic now handled by MusicTrackManager.

Removed the overloaded SetStreamingSounds() method; domain ranges are now set directly via setDomainRange() in the constructor and via a new setDLCMusicRanges() for DLC packs.

State machine simplification

Rewrote playMusicUpdate() with clear sections and detailed comments.

Domain changes are detected by comparing determineCurrentMusicDomain() with m_currentMusicDomain, eliminating large nested conditional blocks.

Fixed a bug where m_musicID was not updated when the domain changed during playback, causing the wrong track to be selected after a stop.

Fixed initialisation order

Reordered fields in SoundEngine so random is initialised before m_musicTrackManager (which depends on it). This prevented a null‑pointer crash.

Direct domain usage

Replaced old integer constants (MUSIC_MENU, etc.) with MusicTrackManager::Domain.

Removed the intermediary getMusicID(int) function; track IDs are now obtained via getTrackForDomain().

DLC music setup

Added setDLCMusicRanges() to configure DLC‑specific ranges cleanly.

Updated the DLC initialisation code to use the new method instead of the now‑gone SetStreamingSounds.

Why This Refactor?
Maintainability – Music logic is no longer scattered across the file; each domain is managed independently.

Readability – The state machine is now much easier to follow.

Correctness – Domain changes are handled reliably, and the history‑based track selection works per domain, not globally.

Future‑proof – Adding new music domains (e.g., for future dimensions) is trivial – just add a new enum value and a setDomainRange call.

Testing
Verified music playback in all contexts: menu, overworld (survival/creative), nether, end.

Tested with both default music and DLC mash‑up packs.

Ensured music discs (jukebox) continue to work unchanged.

Checked that volume and positioning updates still function correctly.

All changes are backward‑compatible and have been tested on the supported platforms (Windows, Durango, PS3/PSVita/Orbis where applicable).

@tylerreese543-lab
Copy link

Oh wait i guess i didn't look at the entire thing
This is actually a good idea
Hope it gets merged 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants