Minor improvements & fixes to music player

- music will be selected correctly after rapid leaving and reentering
town screen
- music sets are now store as single std::map instead of unnecessary
"map of maps"
- fixed computation of resume time for restarted/looped music tracks
- updated & clarified some comments
- converted C functions for SDL_Mixer callbacks into lambdas
- formatting fixes

client/CMusicHandler.cpp

@@ -34,17 +34,6 @@ static std::string sounds[] = {
 #undef VCMI_SOUND_NAME
 #undef VCMI_SOUND_FILE
 
-// Not pretty, but there's only one music handler object in the game.
-static void soundFinishedCallbackC(int channel)
-{
-	CCS->soundh->soundFinishedCallback(channel);
-}
-
-static void musicFinishedCallbackC()
-{
-	CCS->musich->musicFinishedCallback();
-}
-
 void CAudioBase::init()
 {
 	if (initialized)
@@ -140,8 +129,10 @@ void CSoundHandler::init()
 
 	if (initialized)
 	{
-		// Load sounds
-		Mix_ChannelFinished(soundFinishedCallbackC);
+		Mix_ChannelFinished([](int channel)
+		{
+			CCS->soundh->soundFinishedCallback(channel);
+		});
 	}
 }
 
@@ -244,7 +235,7 @@ int CSoundHandler::playSoundFromSet(std::vector<soundBase::soundID> &sound_vec)
 	return playSound(*RandomGeneratorUtil::nextItem(sound_vec, CRandomGenerator::getDefault()));
 }
 
-void CSoundHandler::stopSound( int handler )
+void CSoundHandler::stopSound(int handler)
 {
 	if (initialized && handler != -1)
 		Mix_HaltChannel(handler);
@@ -368,24 +359,24 @@ CMusicHandler::CMusicHandler():
 	for(const ResourceID & file : mp3files)
 	{
 		if(boost::algorithm::istarts_with(file.getName(), "MUSIC/Combat"))
-			addEntryToSet("battle", file.getName(), file.getName());
+			addEntryToSet("battle", file.getName());
 		else if(boost::algorithm::istarts_with(file.getName(), "MUSIC/AITheme"))
-			addEntryToSet("enemy-turn", file.getName(), file.getName());
+			addEntryToSet("enemy-turn", file.getName());
 	}
 
 }
 
-void CMusicHandler::loadTerrainSounds()
+void CMusicHandler::loadTerrainMusicThemes()
 {
 	for (const auto & terrain : CGI->terrainTypeHandler->terrains())
 	{
-		addEntryToSet("terrain", terrain.name, "Music/" + terrain.musicFilename);
+		addEntryToSet("terrain_" + terrain.name, "Music/" + terrain.musicFilename);
 	}
 }
 
-void CMusicHandler::addEntryToSet(const std::string & set, const std::string & musicID, const std::string & musicURI)
+void CMusicHandler::addEntryToSet(const std::string & set, const std::string & musicURI)
 {
-	musicsSet[set][musicID] = musicURI;
+	musicsSet[set].push_back(musicURI);
 }
 
 void CMusicHandler::init()
@@ -393,7 +384,12 @@ void CMusicHandler::init()
 	CAudioBase::init();
 
 	if (initialized)
-		Mix_HookMusicFinished(musicFinishedCallbackC);
+	{
+		Mix_HookMusicFinished([]()
+		{
+			CCS->musich->musicFinishedCallback();
+		});
+	}
 }
 
 void CMusicHandler::release()
@@ -413,29 +409,18 @@ void CMusicHandler::release()
 
 void CMusicHandler::playMusic(const std::string & musicURI, bool loop, bool fromStart)
 {
-	if (current && current->isTrack(musicURI))
+	if (current && current->isPlaying() && current->isTrack(musicURI))
 		return;
 
 	queueNext(this, "", musicURI, loop, fromStart);
 }
 
-void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop, bool fromStart)
+void CMusicHandler::playMusicFromSet(const std::string & musicSet, const std::string & entryID, bool loop, bool fromStart)
 {
-	auto selectedSet = musicsSet.find(whichSet);
-	if (selectedSet == musicsSet.end())
-	{
-		logGlobal->error("Error: playing music from non-existing set: %s", whichSet);
-		return;
-	}
-
-	if (current && current->isSet(whichSet))
-		return;
-
-	// in this mode - play random track from set
-	queueNext(this, whichSet, "", loop, fromStart);
+	playMusicFromSet(musicSet + "_" + entryID, loop, fromStart);
 }
 
-void CMusicHandler::playMusicFromSet(const std::string & whichSet, const std::string & entryID, bool loop,  bool fromStart)
+void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop, bool fromStart)
 {
 	auto selectedSet = musicsSet.find(whichSet);
 	if (selectedSet == musicsSet.end())
@@ -444,18 +429,11 @@ void CMusicHandler::playMusicFromSet(const std::string & whichSet, const std::st
 		return;
 	}
 
-	auto selectedEntry = selectedSet->second.find(entryID);
-	if (selectedEntry == selectedSet->second.end())
-	{
-		logGlobal->error("Error: playing non-existing entry %s from set: %s", entryID, whichSet);
-		return;
-	}
-
-	if (current && current->isTrack(selectedEntry->second))
+	if (current && current->isPlaying() && current->isSet(whichSet))
 		return;
 
-	// in this mode - play specific track from set
-	queueNext(this, "", selectedEntry->second, loop, fromStart);
+	// in this mode - play random track from set
+	queueNext(this, whichSet, "", loop, fromStart);
 }
 
 void CMusicHandler::queueNext(std::unique_ptr<MusicEntry> queued)
@@ -513,7 +491,7 @@ void CMusicHandler::musicFinishedCallback()
 
 	if (current.get() != nullptr)
 	{
-		//return if current music still not finished
+		// if music is looped, play it again
 		if (current->play())
 			return;
 		else
@@ -530,6 +508,7 @@ void CMusicHandler::musicFinishedCallback()
 MusicEntry::MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped, bool fromStart):
 	owner(owner),
 	music(nullptr),
+	playing(false),
 	startTime(uint32_t(-1)),
 	startPosition(0),
 	loop(looped ? -1 : 1),
@@ -578,12 +557,13 @@ bool MusicEntry::play()
 	if (!setName.empty())
 	{
 		const auto & set = owner->musicsSet[setName];
-		load(RandomGeneratorUtil::nextItem(set, CRandomGenerator::getDefault())->second);
+		const auto & iter = RandomGeneratorUtil::nextItem(set, CRandomGenerator::getDefault());
+		load(*iter);
 	}
 
 	logGlobal->trace("Playing music file %s", currentName);
 
-	if ( !fromStart && owner->trackPositions.count(currentName) > 0 && owner->trackPositions[currentName] > 0)
+	if (!fromStart && owner->trackPositions.count(currentName) > 0 && owner->trackPositions[currentName] > 0)
 	{
 		float timeToStart = owner->trackPositions[currentName];
 		startPosition = std::round(timeToStart * 1000);
@@ -591,7 +571,7 @@ bool MusicEntry::play()
 		// erase stored position:
 		// if music track will be interrupted again - new position will be written in stop() method
 		// if music track is not interrupted and will finish by timeout/end of file - it will restart from begginning as it should
-		owner->trackPositions.erase( owner->trackPositions.find(currentName) );
+		owner->trackPositions.erase(owner->trackPositions.find(currentName));
 
 		if (Mix_FadeInMusicPos(music, 1, 1000, timeToStart) == -1)
 		{
@@ -599,13 +579,19 @@ bool MusicEntry::play()
 			return false;
 		}
 	}
-	else if(Mix_PlayMusic(music, 1) == -1)
+	else
 	{
-		logGlobal->error("Unable to play music (%s)", Mix_GetError());
-		return false;
+		startPosition = 0;
+
+		if(Mix_PlayMusic(music, 1) == -1)
+		{
+			logGlobal->error("Unable to play music (%s)", Mix_GetError());
+			return false;
+		}
 	}
 
 	startTime = SDL_GetTicks();
+	playing = true;
 	return true;
 }
 
@@ -613,6 +599,7 @@ bool MusicEntry::stop(int fade_ms)
 {
 	if (Mix_PlayingMusic())
 	{
+		playing = false;
 		loop = 0;
 		uint32_t endTime = SDL_GetTicks();
 		assert(startTime != uint32_t(-1));
@@ -626,6 +613,11 @@ bool MusicEntry::stop(int fade_ms)
 	return false;
 }
 
+bool MusicEntry::isPlaying()
+{
+	return playing;
+}
+
 bool MusicEntry::isSet(std::string set)
 {
 	return !setName.empty() && set == setName;

client/CMusicHandler.h

@@ -100,22 +100,23 @@ class MusicEntry
 
 	int loop; // -1 = indefinite
 	bool fromStart;
+	bool playing;
 	uint32_t startTime;
 	uint32_t startPosition;
 	//if not null - set from which music will be randomly selected
 	std::string setName;
 	std::string currentName;
 
-
 	void load(std::string musicURI);
 
 public:
-	bool isSet(std::string setName);
-	bool isTrack(std::string trackName);
-
 	MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped, bool fromStart);
 	~MusicEntry();
 
+	bool isSet(std::string setName);
+	bool isTrack(std::string trackName);
+	bool isPlaying();
+
 	bool play();
 	bool stop(int fade_ms=0);
 };
@@ -123,7 +124,6 @@ public:
 class CMusicHandler: public CAudioBase
 {
 private:
-	
 	//update volume on configuration change
 	SettingsListener listener;
 	void onVolumeChange(const JsonNode &volumeNode);
@@ -133,18 +133,21 @@ private:
 
 	void queueNext(CMusicHandler *owner, const std::string & setName, const std::string & musicURI, bool looped, bool fromStart);
 	void queueNext(std::unique_ptr<MusicEntry> queued);
+	void musicFinishedCallback();
 
-	std::map<std::string, std::map<std::string, std::string>> musicsSet;
+	/// map <set name> -> <list of URI's to tracks belonging to the said set>
+	std::map<std::string, std::vector<std::string>> musicsSet;
+	/// stored position, in seconds at which music player should resume playing this track
 	std::map<std::string, float> trackPositions;
+
 public:
-	
 	CMusicHandler();
 
 	/// add entry with URI musicURI in set. Track will have ID musicID
-	void addEntryToSet(const std::string & set, const std::string & entryID, const std::string & musicURI);
+	void addEntryToSet(const std::string & set, const std::string & musicURI);
 
 	void init() override;
-	void loadTerrainSounds();
+	void loadTerrainMusicThemes();
 	void release() override;
 	void setVolume(ui32 percent) override;
 
@@ -152,10 +155,10 @@ public:
 	void playMusic(const std::string & musicURI, bool loop, bool fromStart);
 	/// play random track from this set
 	void playMusicFromSet(const std::string & musicSet, bool loop, bool fromStart);
-	/// play specific track from set
+	/// play random track from set (musicSet, entryID)
 	void playMusicFromSet(const std::string & musicSet, const std::string & entryID, bool loop, bool fromStart);
+	/// stops currently playing music by fading out it over fade_ms and starts next scheduled track, if any
 	void stopMusic(int fade_ms=1000);
-	void musicFinishedCallback();
 
 	friend class MusicEntry;
 };

client/CPlayerInterface.cpp

@@ -152,7 +152,7 @@ void CPlayerInterface::init(std::shared_ptr<Environment> ENV, std::shared_ptr<CC
 	env = ENV;
 
 	CCS->soundh->loadHorseSounds();
-	CCS->musich->loadTerrainSounds();
+	CCS->musich->loadTerrainMusicThemes();
 
 	initializeHeroTownList();
 

lib/VCMIDirs.cpp

@@ -170,6 +170,8 @@ class VCMIDirsWIN32 final : public IVCMIDirs
 
 void VCMIDirsWIN32::init()
 {
+	std::locale::global(boost::locale::generator().generate("en_US.UTF-8"));
+
 	// Call base (init dirs)
 	IVCMIDirs::init();
 
@@ -698,11 +700,6 @@ namespace VCMIDirs
 		static bool initialized = false;
 		if (!initialized)
 		{
-			#ifdef VCMI_WINDOWS
-			std::locale::global(boost::locale::generator().generate("en_US.UTF-8"));
-			#endif
-			boost::filesystem::path::imbue(std::locale());
-
 			singleton.init();
 			initialized = true;
 		}
@@ -710,5 +707,4 @@ namespace VCMIDirs
 	}
 }
 
-
 VCMI_LIB_NAMESPACE_END