Review Comments

Fixed AudioMan create and GUISound handling, and moved SoundEventsListMutex to header
Fixed extraneous includes in MOSprite and ContentFile
Fixed bad argument ordering in a few places, and some other little bugs
Fixed some stuff in SoundContainer and moved some things in it and AudioMan into header
Fixed inlines, casts, typos

Author: garethyr

Entities/MOSprite.h

@@ -15,7 +15,6 @@
 // Inclusions of header files
 
 #include "MovableObject.h"
-#include "SoundContainer.h"
 #include "Box.h"
 
 namespace RTE

Entities/SoundContainer.cpp

@@ -59,35 +59,20 @@ namespace RTE {
 				reader.ReportError(std::string("Failed to load the sample from the file"));
 			}
 			m_Sounds.push_back(std::pair<ContentFile, FMOD::Sound *>(newFile, pNewSample));
-		} else if (propName == "LoopSetting")
+		} else if (propName == "LoopSetting") {
 			reader >> m_Loops;
-		else if (propName == "Priority")
+		} else if (propName == "Priority") {
 			reader >> m_Priority;
-		else if (propName == "AffectedByGlobalPitch")
+		} else if (propName == "AffectedByGlobalPitch") {
 			reader >> m_AffectedByGlobalPitch;
-		else
+		} else {
 			// See if the base class(es) can find a match instead
 			return Entity::ReadProperty(propName, reader);
+		}
 
 		return 0;
 	}
 
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-	int SoundContainer::Save(Writer &writer) const {
-		return 0;
-	}
-
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-	void SoundContainer::Destroy(bool notInherited) {
-		// Don't delete Sounds since they are owned in the ContentFile static maps
-		if (!notInherited) {
-			Entity::Destroy();
-		}
-		Clear();
-	}
-
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	void SoundContainer::AddSound(std::string soundPath) {
@@ -131,12 +116,4 @@ namespace RTE {
 		FMOD::Sound *soundToStart;
 		return m_Sounds[m_CurrentSound].second;
 	}
-
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-	void SoundContainer::FadeOut(int fadeOutTime) {
-		if (HasAnySounds()) {
-			return g_AudioMan.FadeOutSound(this, fadeOutTime);
-		}
-	}
 }

Entities/SoundContainer.h

@@ -40,7 +40,7 @@ namespace RTE {
 		/// </summary>
 		/// <param name="soundPath">A path to the sound for this sound to have.</param>
 		/// <param name="loops">The number of times this SoundContainer's sounds will loop. 0 means play once. -1 means play infinitely until stopped.</param>
-		/// <param name="affectedByGlobalPitch">Whether this SoundContainer's sounds' frequency will be affected by the global pitch</param>
+		/// <param name="affectedByGlobalPitch">Whether this SoundContainer's sounds' frequency will be affected by the global pitch.</param>
 		/// <returns>An error return value signaling success or any particular failure. Anything below 0 is an error signal.</returns>
 		int Create(std::string soundPath, int loops = 0, bool affectedByGlobalPitch = true);
 
@@ -51,6 +51,24 @@ namespace RTE {
 		void AddSound(std::string soundPath);
 #pragma endregion
 
+#pragma region Destruction
+		/// <summary>
+		/// Destructor method used to clean up a SoundContainer object before deletion from system memory.
+		/// </summary>
+		virtual ~SoundContainer() { Destroy(true); }
+
+		/// <summary>
+		/// Destroys and resets (through Clear()) the SoundContainer object. It doesn't delete the Sound files, since they're owned by ContentFile static maps.
+		/// </summary>
+		/// <param name="notInherited">Whether to only destroy the members defined in this derived class, or to destroy all inherited members also.</param>
+		virtual void Destroy(bool notInherited = false) { if (!notInherited) { Entity::Destroy(); } Clear(); }
+
+		/// <summary>
+		/// Resets the entire SoundContainer, including its inherited members, to their default settings or values.
+		/// </summary>
+		virtual void Reset() { Clear(); Entity::Reset(); }
+#pragma endregion
+
 #pragma region INI Handling
 		/// <summary>
 		/// Reads a property value from a Reader stream.
@@ -70,27 +88,7 @@ namespace RTE {
 		/// </summary>
 		/// <param name="writer">A Writer that the SoundContainer will save itself with.</param>
 		/// <returns>An error return value signaling success or any particular failure. Anything below 0 is an error signal.</returns>
-		virtual int Save(Writer &writer) const;
-#pragma endregion
-
-#pragma region Destruction
-		/// <summary>
-		/// Destructor method used to clean up a SoundContainer object before deletion from system memory.
-		/// </summary>
-		virtual ~SoundContainer() { Destroy(true); }
-
-		/// <summary>
-		/// Destroys and resets (through Clear()) the SoundContainer object.
-		/// </summary>
-		/// <param name="notInherited">Whether to only destroy the members defined in this derived class, or to destroy all inherited members also.</param>
-		virtual void Destroy(bool notInherited = false);
-#pragma endregion
-
-#pragma region Virtual Override Methods
-		/// <summary>
-		/// Resets the entire SoundContainer, including its inherited members, to their default settings or values.
-		/// </summary>
-		virtual void Reset() { Clear(); Entity::Reset(); }
+		virtual int Save(Writer &writer) const { return 0; }
 #pragma endregion
 
 #pragma region Getters and Setters
@@ -219,7 +217,7 @@ namespace RTE {
 		/// <param name="attenuation">How much distance attenuation to apply to the SoundContainer.</param>
 		/// <param name="player">Player to start playback of this SoundContainer for.</param>
 		/// <returns>Whether this SoundContainer successfully started playing on any channels.</returns>
-		bool Play(float attenuation, int player) { return HasAnySounds() ? g_AudioMan.PlaySound(this, player, attenuation) : false; }
+		bool Play(float attenuation, int player) { return HasAnySounds() ? g_AudioMan.PlaySound(this, attenuation, player) : false; }
 
 		/// <summary>
 		/// Stops playback of this SoundContainer for all players.
@@ -244,7 +242,7 @@ namespace RTE {
 		/// Fades out playback of the SoundContainer to 0 volume.
 		/// </summary>
 		/// <param name="fadeOutTime">How long the fadeout should take.</param>
-		void FadeOut(int fadeOutTime = 1000);
+		void FadeOut(int fadeOutTime = 1000) { if (HasAnySounds()) { return g_AudioMan.FadeOutSound(this, fadeOutTime); } }
 #pragma endregion
 
 	protected:

Main.cpp

@@ -1880,8 +1880,6 @@ int main(int argc, char *argv[]) {
     new PresetMan();
     new FrameMan();
     new AudioMan();
-    g_AudioMan.Create();
-    new GUISound(); //NOTE: Due to being unable to check if the fmod sound system actually exists and is up and running, AudioMan create should happen before anything that uses audio comes into existence 
     new UInputMan();
     new ActivityMan();
     new MovableMan();
@@ -1909,7 +1907,7 @@ int main(int argc, char *argv[]) {
     g_TimerMan.Create();
     g_PresetMan.Create();
     g_FrameMan.Create();
-    g_AudioMan.Create();
+    g_AudioMan.Create(); //NOTE: By necessity of when things can be instantiated, this internally does: new GUISound()
 	g_GUISound.Create();
     g_UInputMan.Create();
 	if (g_NetworkServer.IsServerModeEnabled()) { g_UInputMan.SetMultiplayerMode(true); }

Managers/AudioMan.cpp

@@ -2,12 +2,12 @@
 #include "ConsoleMan.h"
 #include "SettingsMan.h"
 #include "SoundContainer.h"
+#include "GUISound.h"
 
 namespace RTE {
 	const std::string AudioMan::m_ClassName = "AudioMan";
 
-	// I know this is a crime, but if I include it in FrameMan.h the whole thing will collapse due to int redefinitions in Allegro
-	std::mutex g_SoundEventsListMutex[c_MaxClients];
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	FMOD_RESULT F_CALLBACK AudioMan::MusicChannelEndedCallback(FMOD_CHANNELCONTROL *channelControl, FMOD_CHANNELCONTROL_TYPE channelControlType, FMOD_CHANNELCONTROL_CALLBACK_TYPE callbackType, void *unusedCommandData1, void *unusedCommandData2) {
 		if (channelControlType == FMOD_CHANNELCONTROL_CHANNEL && callbackType == FMOD_CHANNELCONTROL_CALLBACK_END) {
@@ -33,7 +33,7 @@ namespace RTE {
 			}
 
 			if (result != FMOD_OK) {
-				g_ConsoleMan.PrintString("ERROR: An error occured when Ending a sound in SoundContainer " + channelSoundContainer->GetPresetName() + ": " + std::string(FMOD_ErrorString(result)));
+				g_ConsoleMan.PrintString("ERROR: An error occurred when Ending a sound in SoundContainer " + channelSoundContainer->GetPresetName() + ": " + std::string(FMOD_ErrorString(result)));
 				return result;
 			}
 		}
@@ -86,6 +86,8 @@ namespace RTE {
 		SetSoundsVolume(m_SoundsVolume);
 		SetMusicVolume(m_MusicVolume);
 
+		new GUISound(); //NOTE: Anything that instantiates SoundContainers needs to wait until the Audio System is up and running before they start doing that. It'll fail safely even if Audio is not enabled.
+
 		return 0;
 	}
 
@@ -106,9 +108,7 @@ namespace RTE {
 			m_AudioSystem->update();
 
 			// Done waiting for silence
-			if (!IsMusicPlaying() && m_SilenceTimer.IsPastRealTimeLimit()) {
-				PlayNextStream();
-			}
+			if (!IsMusicPlaying() && m_SilenceTimer.IsPastRealTimeLimit()) { PlayNextStream(); }
 		}
 	}
 
@@ -119,15 +119,11 @@ namespace RTE {
 			return;
 		}
 
-		if (m_IsInMultiplayerMode) {
-			RegisterSoundEvent(-1, SOUND_SET_PITCH, 0, 0, std::unordered_set<short int>(), 0, pitch, excludeMusic);
-		}
+		if (m_IsInMultiplayerMode) { RegisterSoundEvent(-1, SOUND_SET_PITCH, 0, 0, std::unordered_set<short int>(), 0, pitch, excludeMusic); }
 
 		m_GlobalPitch = Limit(pitch, 8, 0.125); //Limit pitch change to 8 octaves up or down
 
-		if (!excludeMusic) {
-			m_MusicChannelGroup->setPitch(m_GlobalPitch);
-		}
+		if (!excludeMusic) { m_MusicChannelGroup->setPitch(m_GlobalPitch); }
 
 		int numChannels;
 		FMOD_RESULT result = m_SoundChannelGroup->getNumChannels(&numChannels);
@@ -143,24 +139,12 @@ namespace RTE {
 					result == FMOD_OK ? soundChannel->getUserData(&userData) : result;
 					SoundContainer *channelSoundContainer = (SoundContainer *)userData;
 
-					if (channelSoundContainer->IsAffectedByGlobalPitch()) {
-						soundChannel->setPitch(pitch);
-					}
+					if (channelSoundContainer->IsAffectedByGlobalPitch()) { soundChannel->setPitch(pitch); }
 				}
 			}
 		}
 	}
 
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-	void AudioMan::SetMusicVolume(double volume) {
-		m_MusicVolume = Limit(volume, 1, 0);
-
-		if (m_AudioEnabled) {
-			m_MusicChannelGroup->setVolume(m_MusicVolume);
-		}
-	}
-
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	void AudioMan::SetTempMusicVolume(double volume) {
@@ -179,13 +163,12 @@ namespace RTE {
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	bool AudioMan::SetMusicPitch(float pitch) {
-		if (m_IsInMultiplayerMode) {
-			RegisterMusicEvent(-1, MUSIC_SET_PITCH, 0, 0, 0.0, pitch);
-		}
 		if (!m_AudioEnabled) {
 			return false;
 		}
 
+		if (m_IsInMultiplayerMode) { RegisterMusicEvent(-1, MUSIC_SET_PITCH, 0, 0, 0.0, pitch); }
+
 		pitch = Limit(pitch, 8, 0.125); //Limit pitch change to 8 octaves up or down
 		FMOD_RESULT result = m_MusicChannelGroup->setPitch(pitch);
 
@@ -209,7 +192,7 @@ namespace RTE {
 			if (result != FMOD_OK) {
 				g_ConsoleMan.PrintString("ERROR: Could not get music position: " + std::string(FMOD_ErrorString(result)));
 			}
-			return result == FMOD_OK ? ((double)position) / 1000 : 0;
+			return result == FMOD_OK ? (static_cast<double>(position)) / 1000 : 0;
 		}
 		return 0;
 	}
@@ -235,16 +218,6 @@ namespace RTE {
 		}
 	}
 
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-	void AudioMan::SetSoundsVolume(double volume) {
-		m_SoundsVolume = volume;
-
-		if (m_AudioEnabled) {
-			m_SoundChannelGroup->setVolume(volume);
-		}
-	}
-
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	bool AudioMan::SetSoundAttenuation(SoundContainer *pSoundContainer, float distance) {
@@ -275,6 +248,7 @@ namespace RTE {
 		if (!m_AudioEnabled || !pSoundContainer || !pSoundContainer->IsBeingPlayed()) {
 			return false;
 		}
+
 		if (IsInMultiplayerMode() && pSoundContainer) {
 			RegisterSoundEvent(-1, SOUND_SET_PITCH, pSoundContainer->GetHash(), 0, pSoundContainer->GetPlayingChannels(), pSoundContainer->GetLoopSetting(), pitch, pSoundContainer->IsAffectedByGlobalPitch());
 		}
@@ -298,9 +272,7 @@ namespace RTE {
 
 	void AudioMan::PlayMusic(const char *filePath, int loops, double volumeOverrideIfNotMuted) {
 		if (m_AudioEnabled) {
-			if (m_IsInMultiplayerMode) {
-				RegisterMusicEvent(-1, MUSIC_PLAY, filePath, loops, 0.0, 1.0);
-			}
+			if (m_IsInMultiplayerMode) { RegisterMusicEvent(-1, MUSIC_PLAY, filePath, loops, 0.0, 1.0); }
 
 			FMOD_RESULT result = m_MusicChannelGroup->stop();
 			if (result != FMOD_OK) {
@@ -331,7 +303,7 @@ namespace RTE {
 			if (volumeOverrideIfNotMuted >= 0 && m_MusicVolume > 0) {
 				result = musicChannel->setVolume(volumeOverrideIfNotMuted);
 				if (result != FMOD_OK && (loops != 0 && loops != 1)) {
-					g_ConsoleMan.PrintString("ERROR: Failed to set vollume override for music file: " + std::string(filePath) + ". This means it will stay at " + std::to_string(m_MusicVolume) + ": " + std::string(FMOD_ErrorString(result)));
+					g_ConsoleMan.PrintString("ERROR: Failed to set volume override for music file: " + std::string(filePath) + ". This means it will stay at " + std::to_string(m_MusicVolume) + ": " + std::string(FMOD_ErrorString(result)));
 				}
 			}
 
@@ -385,9 +357,7 @@ namespace RTE {
 
 	void AudioMan::StopMusic() {
 		if (m_AudioEnabled) {
-			if (m_IsInMultiplayerMode) {
-				RegisterMusicEvent(-1, MUSIC_STOP, 0, 0, 0.0, 0.0);
-			}
+			if (m_IsInMultiplayerMode) { RegisterMusicEvent(-1, MUSIC_STOP, 0, 0, 0.0, 0.0); }
 
 			FMOD_RESULT result = m_MusicChannelGroup->stop();
 			if (result != FMOD_OK) {
@@ -449,6 +419,7 @@ namespace RTE {
 		if (!m_AudioEnabled || !pSoundContainer) {
 			return false;
 		}
+
 		priority = priority < 0 ? pSoundContainer->GetPriority() : priority;
 		pitch = pSoundContainer->IsAffectedByGlobalPitch() ? m_GlobalPitch : pitch;
 

Managers/AudioMan.h

@@ -155,7 +155,7 @@ namespace RTE {
 		/// Sets the music to a specific volume. Does not affect sounds.
 		/// </summary>
 		/// <param name="volume">The desired volume scalar. 0.0-1.0.</param>
-		void SetMusicVolume(double volume = 1.0);
+		void SetMusicVolume(double volume = 1.0) { m_MusicVolume = Limit(volume, 1, 0); if (m_AudioEnabled) { m_MusicChannelGroup->setVolume(m_MusicVolume); } }
 
 		/// <summary>
 		/// Sets the music to a specific volume, but it will only last until a new song is played. Useful for fading etc.
@@ -200,7 +200,7 @@ namespace RTE {
 		/// Sets the volume of all sounds to a specific volume. Does not affect music.
 		/// </summary>
 		/// <param name="volume">The desired volume scalar. 0.0-1.0.</param>
-		void SetSoundsVolume(double volume = 1.0);
+		void SetSoundsVolume(double volume = 1.0) { m_SoundsVolume = volume; if (m_AudioEnabled) { m_SoundChannelGroup->setVolume(volume); } }
 
 		/// <summary>
 		/// Sets/updates the distance attenuation for a specific SoundContainer. Will only have an effect if the sound is currently being played.
@@ -269,7 +269,7 @@ namespace RTE {
 		/// </summary>
 		/// <param name="filePath">The path to the sound file to play.</param>
 		/// <returns>Returns the new sound object being played. OWNERSHIP IS TRANSFERRED!</returns>
-		SoundContainer *PlaySound(const char *filePath) { return PlaySound(filePath, -1, 0); }
+		SoundContainer *PlaySound(const char *filePath) { return PlaySound(filePath, 0, -1); }
 
 		/// <summary>
 		/// Starts playing a certain sound file with a certain attenuation for a certain player.
@@ -278,7 +278,7 @@ namespace RTE {
 		/// <param name="attenuation">Distance attenuation scalar: 0 = full volume, 1.0 = max distant, but not silent.</param>
 		/// <param name="player">Which player to play the SoundContainer's sounds for, -1 means all players.</param>
 		/// <returns></returns>
-		SoundContainer *PlaySound(const char *filePath, float attenuation, int player) { return PlaySound(filePath, player, attenuation, 0, PRIORITY_NORMAL, -1); }
+		SoundContainer *PlaySound(const char *filePath, float attenuation, int player) { return PlaySound(filePath, attenuation, player, 0, PRIORITY_NORMAL, -1); }
 
 		/// <summary>
 		/// Starts playing a certain sound file with various configuration settings.
@@ -304,7 +304,7 @@ namespace RTE {
 		/// <param name="priority">The priority of this sound - higher priority sounds are more likely to be heard. -1 means it'll use the SoundContainer's value. Defaults to -1.</param>
 		/// <param name="pitch">/// The pitch to play this SoundContainer's at where 1 is unmodified frequency and each multiple of 2 is an octave up or down. Defaults to 1.</param>
 		/// <returns>Whether or not playback of the Sound was successful.</returns>
-		bool PlaySound(SoundContainer *pSoundContainer, float attenuation = 0.0, int player = -1, int priority = -1, double pitch = 1);
+		bool PlaySound(SoundContainer *pSoundContainer, float attenuation = 0, int player = -1, int priority = -1, double pitch = 1);
 
 		/// <summary>
 		/// Stops playing all sounds in a given SoundContainer.
@@ -424,5 +424,7 @@ namespace RTE {
 		AudioMan(const AudioMan &reference);
 		AudioMan & operator=(const AudioMan &rhs);
 	};
+
+	std::mutex g_SoundEventsListMutex[c_MaxClients];
 }
 #endif

System/ContentFile.cpp

@@ -1,6 +1,5 @@
 #include "ContentFile.h"
 #include "PresetMan.h"
-#include "AudioMan.h"
 
 namespace RTE {