Some improvements to the new Lua music API

- Allow removing a track by index and fixup the "set track" interface
- Commit music changes only after event completes, not after any control=flow tag is executed
- Allow inserting a track into the middle of the playlist
- Allow assigning a config to an existing track to overwrite it
- Use std::find when checking for duplicate tracks

data/lua/wml-utils.lua

@@ -154,8 +154,6 @@ function utils.handle_event_commands(cfg, scope_type)
 		end
 		current_exit = "none"
 	end
-	-- Apply music alterations once all the commands have been processed.
-	wesnoth.music_list.force_refresh()
 	return current_exit
 end
 

src/actions/undo_action.cpp

@@ -18,6 +18,7 @@
 #include "variable.hpp" // vconfig
 #include "game_data.hpp"
 #include "units/unit.hpp"
+#include "sound.hpp"
 
 #include <cassert>
 #include <iterator>
@@ -113,6 +114,7 @@ namespace {
 
 		game_events::queued_event q(tag, "", map_location(x1, y1, wml_loc()), map_location(x2, y2, wml_loc()), e.data);
 		resources::lua_kernel->run_wml_action("command", vconfig(e.commands), q);
+		sound::commit_music_changes();
 
 		x1 = oldx1; y1 = oldy1;
 		x2 = oldx2; y2 = oldy2;

src/game_events/handlers.cpp

@@ -30,6 +30,7 @@
 #include "reports.hpp"
 #include "scripting/game_lua_kernel.hpp"
 #include "serialization/string_utils.hpp"
+#include "sound.hpp"
 #include "soundsource.hpp"
 
 #include <iostream>
@@ -127,6 +128,7 @@ void event_handler::handle_event(const queued_event& event_info, handler_ptr& ha
 	// *WARNING*: At this point, dereferencing this could be a memory violation!
 
 	lk.run_wml_action("command", vcfg, event_info);
+	sound::commit_music_changes();
 }
 
 bool event_handler::matches_name(const std::string &name, const game_data * gd) const

src/scripting/lua_audio.cpp

@@ -20,6 +20,7 @@ See the COPYING file for more details.
 #include "sound_music_track.hpp"
 #include "config_assign.hpp"
 #include "preferences.hpp"
+#include <set>
 
 static const char* Track = "music track";
 
@@ -80,8 +81,23 @@ static int impl_music_get(lua_State* L) {
 
 static int impl_music_set(lua_State* L) {
 	if(lua_isnumber(L, 2)) {
-		music_track& track = *get_track(L, 3);
-		sound::set_track(lua_tointeger(L, 2), *track);
+		int i = lua_tointeger(L, 2) - 1;
+		config cfg;
+		if(lua_isnil(L, 3)) {
+			sound::remove_track(i);
+		} else if(luaW_toconfig(L, 3, cfg)) {
+			// Don't allow play_once=yes
+			if(cfg["play_once"]) {
+				return luaL_argerror(L, 3, "For play_once, use wesnoth.music_list.play instead");
+			}
+			// Remove the track at that index and add the new one in its place
+			// It's a little inefficient though...
+			sound::remove_track(i);
+			sound::play_music_config(cfg, i);
+		} else {
+			music_track& track = *get_track(L, 3);
+			sound::set_track(i, *track);
+		}
 		return 0;
 	}
 	const char* m = luaL_checkstring(L, 2);
@@ -101,6 +117,11 @@ static int intf_music_play(lua_State* L) {
 }
 
 static int intf_music_add(lua_State* L) {
+	int i = -1;
+	if(lua_isinteger(L, 1)) {
+		i = lua_tointeger(L, 1);
+		lua_remove(L, 1);
+	}
 	config cfg = config_of
 		("name", luaL_checkstring(L, 1))
 		("append", true);
@@ -126,7 +147,7 @@ static int intf_music_add(lua_State* L) {
 			return luaL_argerror(L, i, "unrecognized argument");
 		}
 	}
-	sound::play_music_config(cfg);
+	sound::play_music_config(cfg, i);
 	return 0;
 }
 
@@ -135,6 +156,18 @@ static int intf_music_clear(lua_State*) {
 	return 0;
 }
 
+static int intf_music_remove(lua_State* L) {
+	// Use a non-standard comparator to ensure iteration in descending order
+	std::set<int, std::greater<int>> to_remove;
+	for(int i = 1; i <= lua_gettop(L); i++) {
+		to_remove.insert(luaL_checkinteger(L, i));
+	}
+	for(int i : to_remove) {
+		sound::remove_track(i);
+	}
+	return 0;
+}
+
 static int intf_music_commit(lua_State*) {
 	sound::commit_music_changes();
 	return 0;
@@ -204,6 +237,7 @@ namespace lua_audio {
 			{ "play", intf_music_play },
 			{ "add", intf_music_add },
 			{ "clear", intf_music_clear },
+			{ "remove", intf_music_remove },
 			{ "force_refresh", intf_music_commit },
 			{ nullptr, nullptr },
 		};

src/sound.cpp

@@ -190,6 +190,21 @@ namespace sound {
 			current_track_list[i] = to;
 		}
 	}
+
+	void remove_track(unsigned int i) {
+		if(i >= current_track_list.size()) {
+			return;
+		}
+		if(i == current_track_index) {
+			// Let the track finish playing
+			current_track.set_play_once(true);
+			// Set current index to the new size of the list
+			current_track_index = current_track_list.size() - 1;
+		} else if(i < current_track_index) {
+			current_track_index--;
+		}
+		current_track_list.erase(current_track_list.begin() + i);
+	}
 }
 
 static bool track_ok(const std::string& id)
@@ -562,7 +577,7 @@ void play_music_repeatedly(const std::string &id)
 	}
 }
 
-void play_music_config(const config &music_node)
+void play_music_config(const config &music_node, int i)
 {
 	music_track track( music_node );
 
@@ -583,18 +598,21 @@ void play_music_config(const config &music_node)
 		current_track_list.clear();
 	}
 
+	auto iter = current_track_list.end();
 	if(track.valid()) {
 		// Avoid 2 tracks with the same name, since that can cause an infinite loop
 		// in choose_track(), 2 tracks with the same name will always return the
 		// current track and track_ok() doesn't allow that.
-		std::vector<music_track>::const_iterator itor = current_track_list.begin();
-		while(itor != current_track_list.end()) {
-			if(track == *itor) break;
-			++itor;
-		}
-
-		if(itor == current_track_list.end()) {
-			current_track_list.push_back(track);
+		if(std::find(current_track_list.begin(), current_track_list.end(), track) == current_track_list.end()) {
+			if(i < 0 || static_cast<size_t>(i) >= current_track_list.size()) {
+				current_track_list.emplace_back(track);
+				iter = current_track_list.end() - 1;
+			} else {
+				iter = current_track_list.emplace(current_track_list.begin() + 1, track);
+				if(current_track_index >= static_cast<size_t>(i)) {
+					current_track_index++;
+				}
+			}
 		} else {
 			ERR_AUDIO << "tried to add duplicate track '" << track.file_path() << "'" << std::endl;
 		}
@@ -602,8 +620,8 @@ void play_music_config(const config &music_node)
 
 	// They can tell us to start playing this list immediately.
 	if (track.immediate()) {
-		current_track = track;
-		current_track_index = current_track_list.size() - 1;
+		current_track = *iter;
+		current_track_index = iter - current_track_list.begin();
 		play_music();
 	} else if (!track.append()) { // Make sure the current track is finished
 		current_track.set_play_once(true);

src/sound.hpp

@@ -41,7 +41,7 @@ void stop_UI_sound();
 void stop_bell();
 
 // Read config entry, alter track list accordingly.
-void play_music_config(const config &music_node);
+void play_music_config(const config &music_node, int i = -1);
 // Act on any track list changes from above.
 void commit_music_changes();
 
@@ -105,6 +105,7 @@ void set_UI_volume(int vol);
 
 unsigned int get_current_track();
 unsigned int get_num_tracks();
+void remove_track(unsigned int i);
 
 }