[Performous-commit] Lasse Karkkainen : Fixed songlist randomization bugs sometimes causing the list to not be randomized .

[Lasse Kärkkäi. <tr...@us...>]

Module: performous
Branch: master
Commit: 00e5180b9cc5342b48372659f0882a736552bb88

Author: Lasse Karkkainen <tro...@tr...>
Date:   Sun Jul  5 19:04:49 2009 +0300

Fixed songlist randomization bugs sometimes causing the list to not be randomized.

Moved the unsolvable ReleaseBlockers to TODO.

---

 ReleaseBlockers.txt |   16 ----------------
 docs/TODO.txt       |   16 +++++++++++++++-
 game/songs.cc       |   25 ++++++++++++++++++-------
 game/songs.hh       |    3 ++-
 4 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/ReleaseBlockers.txt b/ReleaseBlockers.txt
deleted file mode 100644
index 7733d0c..0000000
--- a/ReleaseBlockers.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Blockers for 0.3.1 release
-
-Entries ending with question mark are optional (and not really blockers).
-
-Device enumeration for libda
-- Display sensible options in config menu?
-
-Text rendering (all parts optional for this release)
-- Correct vertical alignment
-- Use dimensions to specify bounding box (possibly init from SVG)
-- Lyric hilighted should be rendered in front of the others
-
-New config menu
-- Multiple items visible at once
-- Modifying strings and stringlist items.
-
diff --git a/docs/TODO.txt b/docs/TODO.txt
index 66f6cd2..e1e0303 100644
--- a/docs/TODO.txt
+++ b/docs/TODO.txt
@@ -33,7 +33,21 @@ Features:
 - Kiosk/arcade mode
 
 
-Ripper
+Text rendering bugs:
+
+- Correct vertical alignment
+- Use dimensions to specify bounding box (possibly init from SVG)
+- Lyric hilighted should be rendered in front of the others
+
+
+New config menu:
+
+- Multiple items visible at once
+- Modifying strings and stringlist items.
+- Predefined options from config schema and libda device enumeration
+
+
+Ripper:
 
 - Compress into Vorbis Ogg and H.264 MKV
 
diff --git a/game/songs.cc b/game/songs.cc
index d28bf56..5bd4c42 100644
--- a/game/songs.cc
+++ b/game/songs.cc
@@ -15,7 +15,7 @@
 #include <iterator>
 #include <stdexcept>
 
-Songs::Songs(std::string const& songlist): m_songlist(songlist), math_cover(), m_order(), m_dirty(false), m_loading(false) {
+Songs::Songs(std::string const& songlist): m_songlist(songlist), math_cover(), m_order(), m_dirty(false), m_loading(false), m_needShuffle(false) {
 	reload();
 }
 
@@ -31,6 +31,7 @@ void Songs::reload() {
 	m_songdirs.clear();
 	std::transform(sd.begin(), sd.end(), std::inserter(m_songdirs, m_songdirs.end()), pathMangle);
 	// Run loading thread
+	m_needShuffle = false;
 	m_loading = true;
 	m_thread.reset(new boost::thread(boost::bind(&Songs::reload_internal, boost::ref(*this))));
 }
@@ -51,10 +52,11 @@ void Songs::reload_internal() {
 	}
 	if (m_loading) dumpSongs_internal(); // Dump the songlist to file (if requested)
 	m_loading = false;
-	m_dirty = true;  // Force shuffle
+	m_needShuffle = true;  // Force shuffle
 }
 
 void Songs::reload_internal(fs::path const& parent) {
+	static int randomIdx = 0;
 	namespace fs = fs;
 	if (std::distance(parent.begin(), parent.end()) > 20) { std::cout << ">>> Not scanning: " << parent.string() << " (maximum depth reached, possibly due to cyclic symlinks)" << std::endl; return; }
 	try {
@@ -69,7 +71,7 @@ void Songs::reload_internal(fs::path const& parent) {
 			if (name.size() < 5 || !regex_match(name.substr(name.size() - 4).c_str(),match,expression)) continue;
 			try {
 				Song* s = new Song(path, name);
-				s->randomIdx = std::numeric_limits<int>::min();
+				s->randomIdx = ++randomIdx; // Not so random during loading, they are shuffled after load is finished
 				boost::mutex::scoped_lock l(m_mutex);
 				m_songs.push_back(boost::shared_ptr<Song>(s));
 				m_dirty = true;
@@ -111,6 +113,16 @@ class Songs::RestoreSel {
 	}
 };
 
+void Songs::update() {
+	if (m_dirty) filter_internal();
+	// Shuffle the songlist if shuffle is finished and all songs are already filtered
+	if (m_needShuffle && !m_dirty) {
+		randomize_internal();
+		math_cover.setTarget(0, m_songs.size());
+		m_needShuffle = false;
+	}
+}
+
 void Songs::randomize() {
 	RestoreSel restore(*this);
 	randomize_internal();
@@ -124,8 +136,9 @@ void Songs::randomize_internal() {
 	for (SongVector::const_iterator it = m_filtered.begin(); it != m_filtered.end(); ++it) (*it)->randomIdx = gen();
 	*/
 	std::srand(std::time(NULL));
+	// Assign the songs randomIdx that is used for sorting in the "random" mode
 	for (SongVector::const_iterator it = m_filtered.begin(); it != m_filtered.end(); ++it) (*it)->randomIdx = std::rand();
-	m_order = 0;
+	m_order = 0;  // Use randomIdx sort mode
 	sort_internal();
 }
 
@@ -137,6 +150,7 @@ void Songs::setFilter(std::string const& val) {
 
 void Songs::filter_internal() {
 	boost::mutex::scoped_lock l(m_mutex);
+	m_dirty = false;
 	RestoreSel restore(*this);
 	try {
 		SongVector filtered;
@@ -149,11 +163,8 @@ void Songs::filter_internal() {
 	}
 	math_cover.reset();
 	sort_internal();
-	if (m_dirty && !m_loading) { randomize_internal(); math_cover.setTarget(0, m_songs.size()); }
-	m_dirty = false;
 }
 
-
 namespace {
 
     /// A functor that compares songs based on a selected member field of them.
diff --git a/game/songs.hh b/game/songs.hh
index ae7c4f8..69902d1 100644
--- a/game/songs.hh
+++ b/game/songs.hh
@@ -17,7 +17,7 @@ class Songs: boost::noncopyable {
 	Songs(std::string const& songlist = std::string());
 	~Songs();
 	/// updates filtered songlist
-	void update() { if (m_dirty) filter_internal(); }
+	void update();
 	/// reloads songlist
 	void reload();
 	/// array access
@@ -77,6 +77,7 @@ class Songs: boost::noncopyable {
 	void sort_internal();
 	volatile bool m_dirty;
 	volatile bool m_loading;
+	volatile bool m_needShuffle;
 	boost::scoped_ptr<boost::thread> m_thread;
 	mutable boost::mutex m_mutex;
 };