SCI: More SongIterator refactoring.

* Added SongIterator::clone()
* got rid of songit_clone()
* removed SIMSG_CLONE and related code
* removed SongIterator::flags
* turned songit_new_tee into TeeSongIterator constructor

svn-id: r39288

engines/sci/sfx/core.cpp

@@ -244,7 +244,7 @@ static void _update_single_song(sfx_state_t *self) {
 		_thaw_time(self); /* Recover song delay time */
 
 		if (newsong && player) {
-			SongIterator *clonesong = songit_clone(newsong->it, newsong->_delay);
+			SongIterator *clonesong = newsong->it->clone(newsong->_delay);
 
 			player->add_iterator(clonesong, newsong->_wakeupTime.msecs());
 		}
@@ -314,7 +314,7 @@ static void _update_multi_song(sfx_state_t *self) {
 			if (self->debug & SFX_DEBUG_SONGS)
 				sciprintf("[SFX] Adding song %lx\n", newseeker->it->ID);
 
-			SongIterator *clonesong = songit_clone(newseeker->it, newseeker->_delay);
+			SongIterator *clonesong = newseeker->it->clone(newseeker->_delay);
 			player->add_iterator(clonesong, g_system->getMillis());
 		}
 		_sfx_set_song_status(self, newseeker,

engines/sci/sfx/iterator.cpp

@@ -602,11 +602,6 @@ SongIterator *Sci0SongIterator::handleMessage(SongIteratorMessage msg) {
 			loops = msg.args[0].i;
 			break;
 
-		case _SIMSG_BASEMSG_CLONE: {
-			BaseSongIterator *mem = new Sci0SongIterator(*this);
-			return mem; /* Assume caller has another copy of this */
-		}
-
 		case _SIMSG_BASEMSG_STOP: {
 			songit_id_t sought_id = msg.ID;
 
@@ -702,6 +697,11 @@ void Sci0SongIterator::init() {
 		channel.state = SI_STATE_PCM;
 }
 
+SongIterator *Sci0SongIterator::clone(int delta) {
+	Sci0SongIterator *newit = new Sci0SongIterator(*this);
+	return newit;
+}
+
 
 /***************************/
 /*-- SCI1 song iterators --*/
@@ -1092,15 +1092,6 @@ SongIterator *Sci1SongIterator::handleMessage(SongIteratorMessage msg) {
 		}
 		break;
 
-		case _SIMSG_BASEMSG_CLONE: {
-			Sci1SongIterator *mem = new Sci1SongIterator(*this);
-			int delta = msg.args[0].i; /* Delay until next step */
-
-			mem->_delayRemaining += delta;
-
-			return mem; /* Assume caller has another copy of this */
-		}
-
 		case _SIMSG_BASEMSG_STOP: {
 			songit_id_t sought_id = msg.ID;
 			int i;
@@ -1220,6 +1211,13 @@ void Sci1SongIterator::init() {
 Sci1SongIterator::~Sci1SongIterator() {
 }
 
+
+SongIterator *Sci1SongIterator::clone(int delta) {
+	Sci1SongIterator *newit = new Sci1SongIterator(*this);
+	newit->_delayRemaining = delta;
+	return newit;
+}
+
 int Sci1SongIterator::getTimepos() {
 	int max = 0;
 	int i;
@@ -1239,13 +1237,13 @@ public:
 	CleanupSongIterator(uint channels) {
 		channel_mask = channels;
 		ID = 17;
-		flags = 0;
 	}
 
 	int nextCommand(byte *buf, int *result);
 	Audio::AudioStream *getAudioStream() { return NULL; }
 	SongIterator *handleMessage(SongIteratorMessage msg);
 	int getTimepos() { return 0; }
+	SongIterator *clone(int delta) { return new CleanupSongIterator(*this); }
 };
 
 SongIterator *CleanupSongIterator::handleMessage(SongIteratorMessage msg) {
@@ -1319,12 +1317,6 @@ SongIterator *FastForwardSongIterator::handleMessage(SongIteratorMessage msg) {
 	else if (msg.recipient == _SIMSG_BASE) {
 		switch (msg.type) {
 
-		case _SIMSG_BASEMSG_CLONE: {
-			FastForwardSongIterator *clone = new FastForwardSongIterator(*this);
-			songit_handle_message(&clone->_delegate, msg);
-			return clone;
-		}
-
 		case _SIMSG_BASEMSG_PRINT:
 			print_tabs_id(msg.args[0].i, ID);
 			fprintf(stderr, "PLASTICWRAP:\n");
@@ -1352,6 +1344,12 @@ FastForwardSongIterator::FastForwardSongIterator(SongIterator *capsit, int delta
 	channel_mask = capsit->channel_mask;
 }
 
+SongIterator *FastForwardSongIterator::clone(int delta) {
+	FastForwardSongIterator *newit = new FastForwardSongIterator(*this);
+	newit->_delegate = _delegate->clone(delta);
+	return newit;
+}
+
 SongIterator *new_fast_forward_iterator(SongIterator *capsit, int delta) {
 	if (capsit == NULL)
 		return NULL;
@@ -1397,6 +1395,64 @@ static void songit_tee_death_notification(TeeSongIterator *self, SongIterator *c
 	}
 }
 
+TeeSongIterator::TeeSongIterator(SongIterator *left, SongIterator *right) {
+	int i;
+	int firstfree = 1; /* First free channel */
+	int incomplete_map = 0;
+
+	morph_deferred = TEE_MORPH_NONE;
+	_status = TEE_LEFT_ACTIVE | TEE_RIGHT_ACTIVE;
+
+	_children[TEE_LEFT].it = left;
+	_children[TEE_RIGHT].it = right;
+
+	// By default, don't remap
+	for (i = 0; i < 16; i++) {
+		_children[TEE_LEFT].channel_remap[i] = i;
+		_children[TEE_RIGHT].channel_remap[i] = i;
+	}
+
+	/* Default to lhs channels */
+	channel_mask = left->channel_mask;
+	for (i = 0; i < 16; i++)
+		if (channel_mask & (1 << i) & right->channel_mask
+		        && (i != MIDI_RHYTHM_CHANNEL) /* Share rhythm */) { /*conflict*/
+			while ((firstfree == MIDI_RHYTHM_CHANNEL)
+			        /* Either if it's the rhythm channel or if it's taken */
+			        || (firstfree < MIDI_CHANNELS
+			            && ((1 << firstfree) & channel_mask)))
+				++firstfree;
+
+			if (firstfree == MIDI_CHANNELS) {
+				incomplete_map = 1;
+				warning("[songit-tee <%08lx,%08lx>] Could not remap right channel #%d: Out of channels",
+				        left->ID, right->ID, i);
+			} else {
+				_children[TEE_RIGHT].channel_remap[i] = firstfree;
+
+				channel_mask |= (1 << firstfree);
+			}
+		}
+#ifdef DEBUG_TEE_ITERATOR
+	if (incomplete_map) {
+		int c;
+		fprintf(stderr, "[songit-tee <%08lx,%08lx>] Channels:"
+		        " %04x <- %04x | %04x\n",
+		        left->ID, right->ID,
+		        channel_mask,
+		        left->channel_mask, right->channel_mask);
+		for (c = 0 ; c < 2; c++)
+			for (i = 0 ; i < 16; i++)
+				fprintf(stderr, "  map [%d][%d] -> %d\n",
+				        c, i, _children[c].channel_remap[i]);
+	}
+#endif
+
+
+	song_iterator_add_death_listener(left, this);
+	song_iterator_add_death_listener(right, this);
+}
+
 TeeSongIterator::~TeeSongIterator() {
 	// When we die, remove any listeners from our children
 	if (_children[TEE_LEFT].it) {
@@ -1562,19 +1618,6 @@ SongIterator *TeeSongIterator::handleMessage(SongIteratorMessage msg) {
 			msg.args[0].i++;
 			break; /* And continue with our children */
 
-		case _SIMSG_BASEMSG_CLONE: {
-			TeeSongIterator *newit = new TeeSongIterator(*this);
-
-			if (newit->_children[TEE_LEFT].it)
-				newit->_children[TEE_LEFT].it =
-				    songit_clone(newit->_children[TEE_LEFT].it, msg.args[0].i);
-			if (newit->_children[TEE_RIGHT].it)
-				newit->_children[TEE_RIGHT].it =
-				    songit_clone(newit->_children[TEE_RIGHT].it, msg.args[0].i);
-
-			return newit;
-		}
-
 		default:
 			break;
 		}
@@ -1625,67 +1668,15 @@ void TeeSongIterator::init() {
 	_children[TEE_RIGHT].it->init();
 }
 
-SongIterator *songit_new_tee(SongIterator *left, SongIterator *right) {
+SongIterator *TeeSongIterator::clone(int delta) {
-	int i;
+	TeeSongIterator *newit = new TeeSongIterator(*this);
-	int firstfree = 1; /* First free channel */
-	int incomplete_map = 0;
-	TeeSongIterator *it = new TeeSongIterator();
 
-	it->morph_deferred = TEE_MORPH_NONE;
+	if (_children[TEE_LEFT].it)
-	it->_status = TEE_LEFT_ACTIVE | TEE_RIGHT_ACTIVE;
+		newit->_children[TEE_LEFT].it = _children[TEE_LEFT].it->clone(delta);
+	if (_children[TEE_RIGHT].it)
+		newit->_children[TEE_RIGHT].it = _children[TEE_RIGHT].it->clone(delta);
 
-	it->_children[TEE_LEFT].it = left;
+	return newit;
-	it->_children[TEE_RIGHT].it = right;
-
-	/* By default, don't remap */
-	for (i = 0; i < 16; i++)
-		it->_children[TEE_LEFT].channel_remap[i]
-		= it->_children[TEE_RIGHT].channel_remap[i] = i;
-
-	/* Default to lhs channels */
-	it->channel_mask = left->channel_mask;
-	for (i = 0; i < 16; i++)
-		if (it->channel_mask & (1 << i) & right->channel_mask
-		        && (i != MIDI_RHYTHM_CHANNEL) /* Share rhythm */) { /*conflict*/
-			while ((firstfree == MIDI_RHYTHM_CHANNEL)
-			        /* Either if it's the rhythm channel or if it's taken */
-			        || (firstfree < MIDI_CHANNELS
-			            && ((1 << firstfree) & it->channel_mask)))
-				++firstfree;
-
-			if (firstfree == MIDI_CHANNELS) {
-				incomplete_map = 1;
-				fprintf(stderr, "[songit-tee <%08lx,%08lx>] "
-				        "Could not remap right channel #%d:"
-				        " Out of channels\n",
-				        left->ID, right->ID, i);
-			} else {
-				it->_children[TEE_RIGHT].channel_remap[i]
-				= firstfree;
-
-				it->channel_mask |= (1 << firstfree);
-			}
-		}
-#ifdef DEBUG_TEE_ITERATOR
-	if (incomplete_map) {
-		int c;
-		fprintf(stderr, "[songit-tee <%08lx,%08lx>] Channels:"
-		        " %04x <- %04x | %04x\n",
-		        left->ID, right->ID,
-		        it->channel_mask,
-		        left->channel_mask, right->channel_mask);
-		for (c = 0 ; c < 2; c++)
-			for (i = 0 ; i < 16; i++)
-				fprintf(stderr, "  map [%d][%d] -> %d\n",
-				        c, i, it->_children[c].channel_remap[i]);
-	}
-#endif
-
-
-	song_iterator_add_death_listener(left, it);
-	song_iterator_add_death_listener(right, it);
-
-	return it;
 }
 
 
@@ -1748,11 +1739,19 @@ SongIterator::SongIterator() {
 	ID = 0;
 	channel_mask = 0;
 	fade.action = FADE_ACTION_NONE;
-	flags = 0;
 	priority = 0;
 	memset(_deathListeners, 0, sizeof(_deathListeners));
 }
 
+SongIterator::SongIterator(const SongIterator &si) {
+	ID = si.ID;
+	channel_mask = si.channel_mask;
+	fade = si.fade;
+	priority = si.priority;
+	memset(_deathListeners, 0, sizeof(_deathListeners));
+}
+
+
 SongIterator::~SongIterator() {
 	for (int i = 0; i < SONGIT_MAX_LISTENERS; ++i)
 		if (_deathListeners[i])
@@ -1821,13 +1820,6 @@ int songit_handle_message(SongIterator **it_reg_p, SongIteratorMessage msg) {
 	return 1;
 }
 
-SongIterator *songit_clone(SongIterator *it, int delta) {
-	SIMSG_SEND(it, SIMSG_CLONE(delta));
-	memset(it->_deathListeners, 0, sizeof(it->_deathListeners));
-	it->flags |= SONGIT_FLAG_CLONE;
-	return it;
-}
-
 SongIterator *sfx_iterator_combine(SongIterator *it1, SongIterator *it2) {
 	if (it1 == NULL)
 		return it2;
@@ -1835,7 +1827,7 @@ SongIterator *sfx_iterator_combine(SongIterator *it1, SongIterator *it2) {
 		return it1;
 
 	/* Both are non-NULL: */
-	return songit_new_tee(it1, it2); /* 'may destroy' */
+	return new TeeSongIterator(it1, it2);
 }
 
 } // End of namespace Sci

engines/sci/sfx/iterator.h

@@ -58,36 +58,32 @@ struct fade_params_t {
 #define SONG_ITERATOR_MESSAGE_ARGUMENTS_NR 2
 
 /* Helper defs for messages */
+enum {
+	_SIMSG_BASE, /* Any base decoder */
+	_SIMSG_PLASTICWRAP  /* Any "Plastic" (discardable) wrapper decoder */
+};
+
 /* Base messages */
 enum {
-	_SIMSG_BASE = 0, /* Any base decoder */
+	_SIMSG_BASEMSG_SET_LOOPS, /* Set loops */
-	_SIMSG_BASEMSG_SET_LOOPS = 0, /* Set loops */
+	_SIMSG_BASEMSG_SET_PLAYMASK, /* Set the current playmask for filtering */
-
+	_SIMSG_BASEMSG_SET_RHYTHM, /* Activate/deactivate rhythm channel */
-	/**
+	_SIMSG_BASEMSG_ACK_MORPH, /* Acknowledge self-morph */
-	 * Clone object and data. Must provide the (possibly negative)
+	_SIMSG_BASEMSG_STOP, /* Stop iterator */
-	 * number of ticks that have passed since the last delay time
+	_SIMSG_BASEMSG_PRINT, /* Print self to stderr, after printing param1 tabs */
-	 * started being used.
+	_SIMSG_BASEMSG_SET_HOLD, /* Set value of hold parameter to expect */
-	 */
+	_SIMSG_BASEMSG_SET_FADE /* Set fade parameters */
-	_SIMSG_BASEMSG_CLONE = 1,
-
-	_SIMSG_BASEMSG_SET_PLAYMASK = 2, /* Set the current playmask for filtering */
-	_SIMSG_BASEMSG_SET_RHYTHM = 3, /* Activate/deactivate rhythm channel */
-	_SIMSG_BASEMSG_ACK_MORPH = 4, /* Acknowledge self-morph */
-	_SIMSG_BASEMSG_STOP = 5, /* Stop iterator */
-	_SIMSG_BASEMSG_PRINT = 6, /* Print self to stderr, after printing param1 tabs */
-	_SIMSG_BASEMSG_SET_HOLD = 7, /* Set value of hold parameter to expect */
-	_SIMSG_BASEMSG_SET_FADE = 8 /* Set fade parameters */
 };
 
 /* "Plastic" (discardable) wrapper messages */
-#define _SIMSG_PLASTICWRAP 1 /* Any base decoder */
+enum {
-#define _SIMSG_PLASTICWRAP_ACK_MORPH 4 /* Acknowledge self-morph */
+	_SIMSG_PLASTICWRAP_ACK_MORPH = _SIMSG_BASEMSG_ACK_MORPH /* Acknowledge self-morph */
+};
 
 /* Messages */
 #define SIMSG_SET_LOOPS(x) _SIMSG_BASE,_SIMSG_BASEMSG_SET_LOOPS,(x),0
 #define SIMSG_SET_PLAYMASK(x) _SIMSG_BASE,_SIMSG_BASEMSG_SET_PLAYMASK,(x),0
 #define SIMSG_SET_RHYTHM(x) _SIMSG_BASE,_SIMSG_BASEMSG_SET_RHYTHM,(x),0
-#define SIMSG_CLONE(x) _SIMSG_BASE,_SIMSG_BASEMSG_CLONE,(x),0
 #define SIMSG_ACK_MORPH _SIMSG_PLASTICWRAP,_SIMSG_PLASTICWRAP_ACK_MORPH,0,0
 #define SIMSG_STOP _SIMSG_BASE,_SIMSG_BASEMSG_STOP,0,0
 #define SIMSG_PRINT(indentation) _SIMSG_BASE,_SIMSG_BASEMSG_PRINT,(indentation),0
@@ -148,7 +144,6 @@ public:
 	songit_id_t ID;
 	uint16 channel_mask; /* Bitmask of all channels this iterator will use */
 	fade_params_t fade;
-	uint flags;
 	int priority;
 
 	/* Death listeners */
@@ -160,6 +155,7 @@ public:
 
 public:
 	SongIterator();
+	SongIterator(const SongIterator &);
 	virtual ~SongIterator();
 
 	/**
@@ -194,13 +190,13 @@ public:
 
 	/**
 	 * Handles a message to the song iterator.
-	 * Parameters: (SongIterator *) self
+	 * @param msg	the message to handle
-	 *             (song_iterator_messag_t) msg: The message to handle
+	 * @return NULL if the message was not understood,
-	 * Returns   : (SongIterator *) NULL if the message was not understood,
+	 *             this if the message could be handled, or a new song iterator
-	 *             self if the message could be handled, or a new song iterator
 	 *             if the current iterator had to be morphed (but the message could
 	 *             still be handled)
-	 * This function is not supposed to be called directly; use
+	 *
+	 * @note This function is not supposed to be called directly; use
 	 * songit_handle_message() instead. It should not recurse, since songit_handle_message()
 	 * takes care of that and makes sure that its delegate received the message (and
 	 * was morphed) before self.
@@ -212,6 +208,13 @@ public:
 	 */
 	virtual int getTimepos() = 0;
 
+	/**
+	 * Clone this song iterator.
+	 * @param delta		number of ticks that still need to elapse until the
+	 * 					next item should be read from the song iterator
+	 */
+	virtual SongIterator *clone(int delta) = 0;
+
 
 private:
 	// Make the assignment operator unreachable, just in case...
@@ -219,11 +222,6 @@ private:
 };
 
 
-/* Song iterator flags */
-#define SONGIT_FLAG_CLONE	(1 << 0)	/* This flag is set for clones, which are exclusively used in song players.
-** Thus, this flag distinguishes song iterators in the main thread from those
-** in the song-player thread. */
-
 /********************************/
 /*-- Song iterator operations --*/
 /********************************/
@@ -275,13 +273,6 @@ SongIterator *songit_new(unsigned char *data, uint size, int type, songit_id_t i
 **             iterator, or NULL if 'type' was invalid or unsupported
 */
 
-SongIterator *songit_new_tee(SongIterator *left, SongIterator *right);
-/* Combines two iterators, returns the next event available from either
-** Parameters: (SongIterator *) left: One of the iterators
-**             (SongIterator *) right: The other iterator
-** Returns   : (SongIterator *) A combined iterator, as suggested above
-*/
-
 
 int songit_handle_message(SongIterator **it_reg, SongIteratorMessage msg);
 /* Handles a message to the song iterator
@@ -291,18 +282,6 @@ int songit_handle_message(SongIterator **it_reg, SongIteratorMessage msg);
 */
 
 
-SongIterator *songit_clone(SongIterator *it, int delta);
-/* Clones a song iterator
-** Parameters: (SongIterator *) it: The iterator to clone
-**             (int) delta: Number of ticks that still need to elapse until
-**                          the next item should be read from the song iterator
-** Returns   : (SongIterator *) A shallow clone of 'it'.
-** This performs a clone on the bottom-most part (containing the actual song data) _only_.
-** The justification for requiring 'delta' to be passed in here is that this
-** is typically maintained outside of the song iterator.
-*/
-
-
 int sfx_play_iterator_pcm(SongIterator *it, unsigned long handle);
 /* Plays a song iterator that found a PCM through a PCM device, if possible
 ** Parameters: (SongIterator *) it: The iterator to play

engines/sci/sfx/iterator_internal.h

@@ -121,6 +121,7 @@ public:
 	SongIterator *handleMessage(SongIteratorMessage msg);
 	void init();
 	int getTimepos();
+	SongIterator *clone(int delta);
 };
 
 
@@ -165,6 +166,7 @@ public:
 	SongIterator *handleMessage(SongIteratorMessage msg);
 	void init();
 	int getTimepos();
+	SongIterator *clone(int delta);
 };
 
 #define PLAYMASK_NONE 0x0
@@ -185,6 +187,7 @@ public:
 	Audio::AudioStream *getAudioStream();
 	SongIterator *handleMessage(SongIteratorMessage msg);
 	int getTimepos();
+	SongIterator *clone(int delta);
 };
 
 
@@ -208,6 +211,9 @@ enum {
 	TEE_MORPH_READY = 1 /**< Ready to self-morph */
 };
 
+/**
+ * This iterator combines two iterators, returns the next event available from either.
+ */
 class TeeSongIterator : public SongIterator {
 public:
 	int _status;
@@ -226,6 +232,7 @@ public:
 	} _children[2];
 
 public:
+	TeeSongIterator(SongIterator *left, SongIterator *right);
 	~TeeSongIterator();
 
 	int nextCommand(byte *buf, int *result);
@@ -233,6 +240,7 @@ public:
 	SongIterator *handleMessage(SongIteratorMessage msg);
 	void init();
 	int getTimepos() { return 0; }
+	SongIterator *clone(int delta);
 };
 
 } // End of namespace Sci