Further Group enhancements.

- Adjust the variable scope of group attributes
- Remove the now redundant GroupBannerPaths vector and replace its usage.
- Migrate ini loading logic to Group class
- Better clarity on comments for Group
- Remove GetTotalSongs in favor of GetSongs.
- Add Year as a value to Group.ini
- Ensure new group and song maps are cleared properly when freeing songs

src/Group.cpp

@@ -1,12 +1,18 @@
 #include "global.h"
-#include "Song.h"
+#include "SongManager.h"
-#include "Group.h"
-#include "Style.h"
 #include "RageFile.h"
+#include "RageUtil.h"
 #include "RageFileManager.h"
-#include "RageSurface.h"
+#include "RageLog.h"
-#include <vector>
+#include "Song.h"
+#include "SongCacheIndex.h"
+#include "SongUtil.h"
+#include "TitleSubstitution.h"
+#include "Group.h"
 
+#include <cstddef>
+#include <tuple>
+#include <vector>
 
 Group::Group() {
     m_sDisplayTitle = "";
@@ -17,12 +23,60 @@ Group::Group() {
     m_sSeries = "";
     m_iSyncOffset = 0;
     m_bHasGroupIni = false;
-    iTotalSongs = 0;
+    m_iYearReleased = 0;
     m_sBannerPath = "";
     m_sCredits.clear();
     m_sAuthorsNotes = "";
 }
 
+Group::Group(const RString &sPath) {
+    RString sGroupIniPath = sPath + "/Group.ini";
+    RString credits = "";
+    if (FILEMAN->DoesFileExist(sGroupIniPath)) {
+        IniFile ini;
+        ini.ReadFile(sGroupIniPath);
+        ini.GetValue("Group", "DisplayTitle", m_sDisplayTitle);
+        if (m_sDisplayTitle.empty()) {
+            m_sDisplayTitle = m_sGroupName;
+        }
+        ini.GetValue("Group", "Banner", m_sBannerPath);
+        ini.GetValue("Group", "SortTitle", m_sSortTitle);
+        if (m_sSortTitle.empty()) {
+            m_sSortTitle = m_sDisplayTitle;
+        }
+        ini.GetValue("Group", "TranslitTitle", m_sTranslitTitle);
+        if (m_sTranslitTitle.empty()) {
+            m_sTranslitTitle = m_sDisplayTitle;
+        }
+        ini.GetValue("Group", "Series", m_sSeries);
+        RString sValue = "";
+        ini.GetValue("Group", "SyncOffset", sValue);
+        if (sValue.CompareNoCase("null") == 0) {
+            m_iSyncOffset = 0;
+        } else if (sValue.CompareNoCase("itg") == 0) {
+            m_iSyncOffset = 0.009f;
+        } else {
+            m_iSyncOffset = StringToFloat(sValue);
+        }
+        ini.GetValue("Group", "Year", m_iYearReleased);
+        ini.GetValue("Group", "AuthorsNotes", m_sAuthorsNotes);
+
+        std::vector<RString> credits_vector;
+        ini.GetValue("Group", "Credits", credits);
+        split(credits, ";", credits_vector);
+        m_sCredits = credits_vector;
+        m_bHasGroupIni = true;
+    } else {
+        m_bHasGroupIni = false;
+    }
+    
+}
+
+const std::vector<Song *> &Group::GetSongs() const
+{
+    return SONGMAN->GetSongs(m_sGroupName);
+}
+
 
 
 #include "LuaBinding.h"
@@ -71,9 +125,10 @@ public:
         return 1;
     }
 
-    static int GetTotalSongs( T* p, lua_State *L )
+    static int GetSongs( T* p, lua_State *L )
     {
-        lua_pushnumber(L, p->iTotalSongs);
+        const std::vector<Song*> &v = p->GetSongs();
+        LuaHelpers::CreateTableFromArray<Song*>( v, L );
         return 1;
     }
 
@@ -96,6 +151,12 @@ public:
         return 1;
     }
 
+    static int GetYearReleased( T* p, lua_State *L )
+    {
+        lua_pushnumber(L, p->GetYearReleased());
+        return 1;
+    }
+
 	LunaGroup()
 	{
 		ADD_METHOD( GetGroupName );
@@ -105,10 +166,11 @@ public:
 		ADD_METHOD( GetSeries );
 		ADD_METHOD( GetSyncOffset );
 		ADD_METHOD( HasGroupIni );
-		ADD_METHOD( GetTotalSongs );
+		ADD_METHOD( GetSongs );
 		ADD_METHOD( GetBannerPath );
 		ADD_METHOD( GetStepArtistCredits );
 		ADD_METHOD( GetAuthorsNotes );
+        ADD_METHOD( GetYearReleased );
 	}
 
 };

src/Group.h

@@ -19,87 +19,143 @@ class Group
 {
 public:
 	Group();
+    Group( const RString &sPath);
+    ~Group();
     // Lua
 	void PushSelf( lua_State *L );
-    RString m_sDisplayTitle;
+
-    RString m_sSortTitle;
+	/**
-    RString m_sPath;
+	 * @brief This is the title of the group as its displayed to the user 
-    RString m_sGroupName;
+     * and supersedes the actual folder name on disk.
-    RString m_sTranslitTitle;
+	 * 
-    RString m_sSeries;
+	 * @return The display title of the group. 
-    float m_iSyncOffset;
+	 */
-    bool m_bHasGroupIni;
-    RString m_sBannerPath;
-    std::vector<RString> m_sCredits;
-    RString m_sAuthorsNotes;
-	// Get the display title of the group.
     const RString GetDisplayTitle() const { return m_sDisplayTitle; };
 
-    // Set the display title of the group.
+    /**
-    void SetDisplayTitle(const RString sDisplayTitle) { m_sDisplayTitle = sDisplayTitle; };
+     * @brief This is the value considered when sorting the group by its title.
-
+     * 
-    // Get the sort title of the group.
+     * @return const RString
+     */
     const RString GetSortTitle() const { return m_sSortTitle; };
 
-    // Set the sort title of the group.
+    /**
-    void SetSortTitle(const RString sSortTitle) { m_sSortTitle = sSortTitle; };
+     * @brief The path to the group folder.
-
+     * 
-    // Get the path of the group.
+     * @return the path
+     */
     const RString GetPath() const { return m_sPath; };
 
-    // Set the path of the group.
+    /**
-    void SetPath(const RString sPath) { m_sPath = sPath; };
+     * @brief The actual name of the group folder on disk.
-
+     * 
-    // Get the group name.
+     * @return const RString 
+     */
     const RString GetGroupName() const { return m_sGroupName; };
 
-    // Set the group name.
-    void SetGroupName(const RString sGroupName) { m_sGroupName = sGroupName; };
 
-    // Get the transliterated title of the group.
+    /**
+     * @brief Allows transliteration of the group title.
+     * 
+     * @return const RString 
+     */
     const RString GetTranslitTitle() const { return m_sTranslitTitle; };
 
-    // Set the transliterated title of the group.
-    void SetTranslitTitle(const RString sTranslitTitle) { m_sTranslitTitle = sTranslitTitle; };
 
-    // Get the series of the group.
+    /**
+     * @brief The series the group belongs to
+     * 
+     * @return const RString 
+     */
     const RString GetSeries() const { return m_sSeries; };
 
-    // Set the series of the group.
-    void SetSeries(const RString sSeries) { m_sSeries = sSeries; };
 
-    // Get the sync offset of the group.
+    /**
+     * @brief Defines the offset applied to all songs within the group.
+     * 
+     * @return float 
+     */
     float GetSyncOffset() const { return m_iSyncOffset; };
 
-    // Set the sync offset of the group.
+    /**
-    void SetSyncOffset(const float fSyncOffset) { m_iSyncOffset = fSyncOffset; };
+     * @brief Whether the group has a group.ini file.
-
+     * 
-    // Determine if the group has a Group.ini file.
+     * @return true if the group has a group.ini file, false otherwise.
-
+     */
     bool HasGroupIni() const { return m_bHasGroupIni; };
 
-    // Set if the group has a Group.ini file.
-    void SetGroupIni(const bool bHasGroupIni) { m_bHasGroupIni = bHasGroupIni; };
     
-    // Get the banner path of the group.
+    /**
+     * @brief The path to the group's banner.
+     * 
+     * @return const RString 
+     */
     const RString GetBannerPath() const { return m_sBannerPath; };
 
-    // Set the banner path of the group.
+    /**
-    void SetBannerPath(const RString sBannerPath) { m_sBannerPath = sBannerPath; };
+     * @brief The persons who worked with the group who should be credited.
-    
+     * 
-    // Get the credits of the group.
+     * @return const std::vector<RString> 
+     */
     const std::vector<RString> GetStepArtistCredits() const { return m_sCredits; };
 
-    // Set the credits of the group.
+    /**
-    void SetStepArtistCredits(const std::vector<RString> sCredits) { m_sCredits = sCredits; };
+     * @brief Additional notes about the group.
-
+     * 
-    // Get the authors notes of the group.
+     * @return const RString 
+     */
     const RString GetAuthorsNotes() const { return m_sAuthorsNotes; };
 
-    // Set the authors notes of the group.
+    /**
-    void SetAuthorsNotes(const RString sAuthorsNotes) { m_sAuthorsNotes = sAuthorsNotes; };
+     * @brief Get the songs in the group.
+	 * @return the songs that belong in the group. */
+	const std::vector<Song*> &GetSongs() const;
 
-    int iTotalSongs;
+    /**
+     * @brief The year the group was released
+     * 
+     * @return int 
+     */
+    int GetYearReleased() const { return m_iYearReleased; };
+
+    private:
+        /**
+         * @brief This is the title of the group as its displayed to the user 
+         * and supersedes the actual folder name on disk. */
+        RString m_sDisplayTitle;
+
+        /** @brief This is the value considered when sorting the group by its title. */
+        RString m_sSortTitle;
+
+        /** @brief The path to the group folder. */
+        RString m_sPath;
+
+        /** @brief The actual name of the group folder on disk. */
+        RString m_sGroupName;
+
+        /** @brief Allows transliteration of the group title. */
+        RString m_sTranslitTitle;
+
+        /** @brief The series the group belongs to */
+        RString m_sSeries;
+
+        /** @brief Defines the offset applied to all songs within the group. */
+        float m_iSyncOffset;
+
+        /** @brief Whether the group has a group.ini file. */
+        bool m_bHasGroupIni;
+
+        /** @brief The path to the group's banner. */
+        RString m_sBannerPath;
+
+        /** @brief The persons who worked with the group who should be credited. */
+        std::vector<RString> m_sCredits;
+
+        /** @brief Additional notes about the group. */
+        RString m_sAuthorsNotes;
+
+        /** @brief The year the group was released */
+        int m_iYearReleased;
 
 };
 

src/MusicWheel.cpp

@@ -758,10 +758,8 @@ void MusicWheel::BuildWheelItemDatas( std::vector<MusicWheelItemData *> &arrayWh
 						Group* pGroup = pSong->GetGroup();
 						if( bUseSections )
 						{
-							RString sThisSection = pSong->GetGroup()->m_sSortTitle;
+							RString sThisSection = pSong->GetGroup()->GetSortTitle();
-							if (pGroup->m_bHasGroupIni) {
+
-								sThisSection = pGroup->m_sGroupName;
-							}
 							if( sThisSection != sLastSection )
 							{
 								int iSectionCount = 0;

src/MusicWheelItem.cpp

@@ -226,10 +226,10 @@ void MusicWheelItem::LoadFromWheelItemData( const WheelItemBaseData *pData, int
 				sDisplayName = SONGMAN->ShortenGroupName(pWID->m_sText);
 			}
 			else {
-				if ( pWID->m_pGroup->m_sSeries.empty() )
+				if ( pWID->m_pGroup->GetSeries().empty() )
 					sDisplayName = SONGMAN->ShortenGroupName(pWID->m_sText);
 				else
-					sDisplayName = SONGMAN->ShortenGroupName("[" + pWID->m_pGroup->m_sSeries +"] " + pWID->m_pGroup->m_sDisplayTitle);
+					sDisplayName = SONGMAN->ShortenGroupName("[" + pWID->m_pGroup->GetSeries() +"] " + pWID->m_pGroup->GetDisplayTitle());
 			}
 			if( GAMESTATE->sExpandedSectionName == pWID->m_sText )
 				type = MusicWheelItemType_SectionExpanded;

src/SongManager.cpp

@@ -280,6 +280,7 @@ void SongManager::AddGroup( RString sDir, RString sGroupDirName, Group group )
 		return; // the group is already added
 
 	RString sBannerPath;
+
 	// Look for a group banner in this group folder
 	std::vector<RString> arrayGroupBanners;
 	
@@ -337,12 +338,7 @@ void SongManager::AddGroup( RString sDir, RString sGroupDirName, Group group )
 			sBackgroundPath = sDir+arrayGroupBackgrounds[0];
 	}
 */
-	/*
-	LOG->Trace( "Group banner for '%s' is '%s'.", sGroupDirName.c_str(),
-				sBannerPath != ""? sBannerPath.c_str():"(none)" );
-	*/
 	m_sSongGroupNames.push_back( sGroupDirName );
-	m_sSongGroupBannerPaths.push_back( sBannerPath );
 
 	if (m_mapGroupsByName.find(sGroupDirName) == m_mapGroupsByName.end())
 	{
@@ -437,80 +433,7 @@ void SongManager::LoadSongDir( RString sDir, LoadingWindow *ld, bool onlyAdditio
 
 		SongPointerVector& index_entry = m_mapSongGroupIndex[sGroupDirName];
 		RString group_base_name= Basename(sGroupDirName);
-
+		Group group = Group(sDir + sGroupDirName);
-		RString sGroupIniPath = sDir + sGroupDirName + "/Group.ini";
-		RString sDisplayTitle = sGroupDirName;
-		RString SortTitle = sGroupDirName;
-		RString TranslitTitle = sGroupDirName;
-		RString Series = "";
-		RString bannerPath = "";
-		RString credits = "";
-		RString authorsNotes = "";
-		// Use our new Group class
-		Group group;
-		float fOffset = 0;
-		
-		if( FILEMAN->DoesFileExist( sGroupIniPath ) && arraySongDirs.size() > 0 )
-		{
-			IniFile ini;
-			ini.ReadFile( sGroupIniPath );
-			ini.GetValue( "Group", "DisplayTitle", sDisplayTitle );
-			if (sDisplayTitle.empty()) {
-				sDisplayTitle = group_base_name;
-			}
-			ini.GetValue( "Group", "Banner", bannerPath );
-			ini.GetValue( "Group", "SortTitle", SortTitle );
-			if (SortTitle.empty()) {
-				SortTitle = sDisplayTitle;
-			}
-			ini.GetValue( "Group", "TranslitTitle", TranslitTitle );
-			if (TranslitTitle.empty()) {
-				TranslitTitle = sDisplayTitle;
-			}
-			ini.GetValue( "Group", "Series", Series );
-			RString sValue = "";
-			
-			ini.GetValue( "Group", "SyncOffset", sValue );
-			if( sValue.CompareNoCase("null")==0 ) {
-				fOffset = 0;
-			} else if ( sValue.CompareNoCase("itg")==0 ) {
-				fOffset = 0.009f;
-			} else {
-				fOffset = StringToFloat( sValue );
-			}
-			ini.GetValue( "Group", "Credits", credits );
-			ini.GetValue( "Group", "AuthorsNotes", authorsNotes );
-
-			LOG->Trace("Loaded group.ini for \"%s\"", (sDir+sGroupDirName).c_str() );
-
-			group.SetGroupIni(true);
-		} else {
-			group.SetGroupIni(false);
-		}
-		// group.m_sDisplayTitle = sDisplayTitle;
-		// group.m_sSortTitle = SortTitle;
-		// group.m_sTranslitTitle = TranslitTitle;
-		// group.m_sSeries = Series;
-		// group.m_iSyncOffset = fOffset;
-		// group.m_sBannerPath = bannerPath;
-		// group.m_sPath = sDir + sGroupDirName;
-		// group.m_sGroupName = group_base_name;
-		// split(credits, ";", group.m_sCredits);
-		// group.m_sAuthorsNotes = authorsNotes;
-
-		group.SetDisplayTitle(sDisplayTitle);
-		group.SetSortTitle(SortTitle);
-		group.SetTranslitTitle(TranslitTitle);
-		group.SetSeries(Series);
-		group.SetSyncOffset(fOffset);
-		group.SetBannerPath(bannerPath);
-		group.SetPath(sDir + sGroupDirName);
-		group.SetGroupName(group_base_name);
-		std::vector<RString> credits_vector;
-		split(credits, ";", credits_vector);
-		group.SetStepArtistCredits(credits_vector);
-		group.SetAuthorsNotes(authorsNotes);
-
 
 		for( unsigned j=0; j< arraySongDirs.size(); ++j )	// for each song dir
 		{
@@ -573,13 +496,10 @@ void SongManager::LoadSongDir( RString sDir, LoadingWindow *ld, bool onlyAdditio
 
 		// Don't add the group name if we didn't load any songs in this group.
 		if(!loaded) continue;
-		// Save amount of songs
-		group.iTotalSongs = loaded;
 
 		// Add this group to the group array.
 		AddGroup(sDir, sGroupDirName, group);
 
-
 		// Cache and load the group banner. (and background if it has one -aj)
 		IMAGECACHE->CacheImage( "Banner", GetSongGroupBannerPath(sGroupDirName) );
 
@@ -664,7 +584,10 @@ void SongManager::PreloadSongImages()
 void SongManager::FreeSongs()
 {
 	m_sSongGroupNames.clear();
-	m_sSongGroupBannerPaths.clear();
+	m_mapGroupsByName.clear();
+	m_mapSongsByDifficulty.clear();
+	m_mapPreferredSectionToSongs.clear();
+
 	//m_sSongGroupBackgroundPaths.clear();
 
 	for (Song *song : m_pSongs)
@@ -680,7 +603,6 @@ void SongManager::FreeSongs()
 	m_pDeletedSongs.clear();
 
 	m_mapSongGroupIndex.clear();
-	m_sSongGroupBannerPaths.clear();
 
 	m_pPopularSongs.clear();
 	m_pShuffledSongs.clear();
@@ -712,13 +634,8 @@ bool SongManager::IsGroupNeverCached(const RString& group) const
 
 RString SongManager::GetSongGroupBannerPath( RString sSongGroup ) const
 {
-	for( unsigned i = 0; i < m_sSongGroupNames.size(); ++i )
+	Group* group = GetGroupFromName(sSongGroup);
-	{
+	return group->GetBannerPath();
-		if( sSongGroup == m_sSongGroupNames[i] )
-			return m_sSongGroupBannerPaths[i];
-	}
-
-	return RString();
 }
 /*
 RString SongManager::GetSongGroupBackgroundPath( RString sSongGroup ) const

src/SongManager.h

@@ -244,7 +244,6 @@ protected:
 	std::map<RString, std::vector<Song*>> m_mapPreferredSectionToSongs;
 	std::vector<PreferredSortSection> m_vPreferredSongSort;
 	std::vector<RString>		m_sSongGroupNames;
-	std::vector<RString>		m_sSongGroupBannerPaths; // each song group may have a banner associated with it
 	//vector<RString>		m_sSongGroupBackgroundPaths; // each song group may have a background associated with it (very rarely)
 	
 	std::map<RString, Group> m_mapGroupsByName;

src/SongUtil.cpp

@@ -596,12 +596,12 @@ void SongUtil::SortSongPointerArrayByGenre( std::vector<Song*> &vpSongsInOut )
 int SongUtil::CompareSongPointersByGroup(const Song *pSong1, const Song *pSong2)
 {
 	// Check if the sort title exists
-	if( pSong1->GetGroup()->m_sSortTitle.empty() || pSong2->GetGroup()->m_sSortTitle.empty() ) {
+	if( pSong1->GetGroup()->GetSortTitle().empty() || pSong2->GetGroup()->GetSortTitle().empty() ) {
 		// LOG the DEETS
 		LOG->Warn("SongUtil::CompareSongPointersByGroup: Song %s or %s has an empty group name. Using group name instead.", pSong1->m_sSongName.c_str(), pSong2->m_sSongName.c_str());
 		return pSong1->m_sGroupName < pSong2->m_sGroupName;
 	} else {
-		return pSong1->GetGroup()->m_sSortTitle < pSong2->GetGroup()->m_sSortTitle;
+		return pSong1->GetGroup()->GetSortTitle() < pSong2->GetGroup()->GetSortTitle();
 	}
 }
 
@@ -609,8 +609,8 @@ static int CompareSongPointersByGroupAndTitle( const Song *pSong1, const Song *p
 {
 
 	// Check if the sort title exists
-	const RString &sGroup1 = pSong1->GetGroup()->m_sSortTitle;
+	const RString &sGroup1 = pSong1->GetGroup()->GetSortTitle();
-	const RString &sGroup2 = pSong2->GetGroup()->m_sSortTitle;
+	const RString &sGroup2 = pSong2->GetGroup()->GetSortTitle();
 
 	if( sGroup1 < sGroup2 )
 		return true;
@@ -655,7 +655,7 @@ RString SongUtil::GetSectionNameFromSongAndSort( const Song* pSong, SortOrder so
 		return SONGMAN->SongToPreferredSortSectionName( pSong );
 	case SORT_GROUP:
 		// guaranteed not empty
-		return pSong->GetGroup()->m_sSortTitle;
+		return pSong->GetGroup()->GetSortTitle();
 	case SORT_TITLE:
 	case SORT_ARTIST:
 		{