AUDIO: Document issues with semantics of Audio::MidiPlayer::isPlaying()

2 files changed, 19 insertions, 1 deletions

diff --git a/audio/midiplayer.cpp b/audio/midiplayer.cpp
index 1e39b999f9..613ad2eddd 100644
--- a/audio/midiplayer.cpp
+++ b/audio/midiplayer.cpp
@@ -174,8 +174,8 @@ void MidiPlayer::stop() {
 
 void MidiPlayer::pause() {
 //	debugC(2, kDraciSoundDebugLevel, "Pausing track %d", _track);
-	setVolume(-1);	// FIXME: This should be 0, shouldn't it?
 	_isPlaying = false;
+	setVolume(-1);	// FIXME: This should be 0, shouldn't it?
 }
 
 void MidiPlayer::resume() {
diff --git a/audio/midiplayer.h b/audio/midiplayer.h
index 1bb7942343..fe96e0d5e2 100644
--- a/audio/midiplayer.h
+++ b/audio/midiplayer.h
@@ -69,6 +69,24 @@ public:
 	virtual void pause();
 	virtual void resume();
 
+	/**
+	 * Return whether there is currently any MIDI music playing.
+	 *
+	 * @todo There is a subtle difference between the semantics of this in
+	 *       various subclasses, related to paused music: Namely, should this
+	 *       function return true or false if a MIDI song is currently loaded,
+	 *       but paused? In the base implementation of pause/resume, "false"
+	 *       will be returned (that is, it is not possible to distinguish between
+	 *       nothing being played, and an active but paused MIDI tune).
+	 *       But in several subclasses (e.g. in HUGO), there is a separate _paused
+	 *       variable, which is used to pause playback, and for these, "true"
+	 *       will be returned.
+	 *       And in SAGA, isPlaying is overwritten to return the value
+	 *       of _parser->isPlaying() (which should amount to "true" in the
+	 *       described situation).
+	 *       We really should unify this and clearly define the desired
+	 *       semantics of this method.
+	 */
 	bool isPlaying() const { return _isPlaying; }
 
 	/**