Skip to content

Commit 433c1b7

Browse files
FlamefireFlow86
authored andcommitted
Fix song selection in music player window
Selecting a start song when the playlist isn't prepared yet isn't possible because the `order` list isn't filled and hence the selected song isn't found. Make sure we prepare it first and add test for that. Fixes #1833 Update the translations, fixes #1832
1 parent 6da68ac commit 433c1b7

File tree

4 files changed

+26
-13
lines changed

4 files changed

+26
-13
lines changed

libs/s25main/Playlist.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ bool Playlist::Load(Log& logger, const boost::filesystem::path& filepath)
8888

8989
void Playlist::SetStartSong(const unsigned id)
9090
{
91+
if(order_.empty())
92+
Prepare();
9193
const auto itEl = helpers::find(order_, id);
9294
if(itEl != order_.end())
9395
std::swap(order_.front(), *itEl);

libs/s25main/ingameWindows/iwMusicPlayer.cpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
1+
// Copyright (C) 2005 - 2025 Settlers Freaks (sf-team at siedler25.org)
22
//
33
// SPDX-License-Identifier: GPL-2.0-or-later
44

@@ -86,15 +86,15 @@ iwMusicPlayer::iwMusicPlayer()
8686
AddText(ID_txtPlaylist, DrawPoint(20, 240), _("Playlist:"), COLOR_YELLOW, FontStyle{}, NormalFont);
8787
AddComboBox(ID_cbPlaylist, DrawPoint(20, 260), Extent(330, 22), TextureColor::Green1, NormalFont, 200);
8888

89-
// Playlistbuttons
89+
// Playlist buttons
9090
const unsigned short button_distance = 10;
9191
const Extent buttonSize((330 - button_distance) / 2, 22);
9292
ctrlButton* b1 =
9393
AddTextButton(ID_btAddPlaylist, DrawPoint(20, 288), buttonSize, TextureColor::Green2, _("Add"), NormalFont);
9494
AddTextButton(ID_btRemovePlaylist, b1->GetPos() + DrawPoint(buttonSize.x + button_distance, 0), buttonSize,
9595
TextureColor::Green2, _("Remove"), NormalFont);
9696

97-
// Buttons für die Musikstücke
97+
// Track buttons
9898
AddImageButton(ID_btAddTrack, DrawPoint(360, 30), Extent(30, 40), TextureColor::Grey, LOADER.GetImageN("io", 138),
9999
_("Add track"));
100100
AddImageButton(ID_btAddTrackDir, DrawPoint(390, 30), Extent(30, 40), TextureColor::Grey,
@@ -116,7 +116,7 @@ iwMusicPlayer::iwMusicPlayer()
116116
AddImageButton(ID_btSave, DrawPoint(370, 270), Extent(40, 40), TextureColor::Grey, LOADER.GetImageN("io", 37),
117117
_("Save playlist"));
118118

119-
// Mit Werten füllen
119+
// Fill values for current playlist
120120
UpdateFromPlaylist(MUSICPLAYER.GetPlaylist());
121121
UpdatePlaylistCombo(SETTINGS.sound.playlist);
122122
auto* cbPlayList = GetCtrl<ctrlComboBox>(ID_cbPlaylist);
@@ -259,7 +259,7 @@ void iwMusicPlayer::Msg_ButtonClick(const unsigned ctrl_id)
259259
{
260260
const std::string playlistName(GetCtrl<ctrlComboBox>(ID_cbPlaylist)->GetText(*selection));
261261

262-
// RTTR-Playlisten dürfen nicht gelöscht werden
262+
// RTTR-playlists must not be deleted
263263
if(isReadonlyPlaylist(playlistName))
264264
{
265265
WINDOWMANAGER.Show(
@@ -355,18 +355,16 @@ void iwMusicPlayer::Msg_Input(const unsigned win_id, const std::string& msg)
355355
valid = true;
356356
else
357357
{
358-
// Evtl ein Siedlerstück ("sNN")?
358+
// Could be an original The settlers song ("sNN")?
359359
if(msg.length() == 3 && msg[0] == 's')
360360
{
361361
if(s25util::fromStringClassicDef(msg.substr(1), 999u) <= 14u)
362362
valid = true;
363363
}
364364
}
365365

366-
// Gültiges Siedlerstück?
367366
if(valid)
368367
{
369-
// Hinzufügen
370368
GetCtrl<ctrlList>(ID_lstSongs)->AddString(msg);
371369
changed = true;
372370
} else
@@ -376,17 +374,15 @@ void iwMusicPlayer::Msg_Input(const unsigned win_id, const std::string& msg)
376374
break;
377375
case ID_wndAddPlaylist:
378376
{
379-
// Ungültige Namen ausschließen
377+
// Playlist name must start with a letter
380378
const auto isLetter = [](const char c) { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); };
381379
const boost::filesystem::path fullPlaylistPath = GetFullPlaylistPath(msg);
382380
if(!msg.empty() && isLetter(msg.front()) && Playlist().SaveAs(fullPlaylistPath))
383381
{
384-
// Combobox updaten
385382
UpdatePlaylistCombo(fullPlaylistPath.string());
386383
changed = true;
387384
} else
388385
{
389-
// Fehler, konnte nicht gespeichert werden
390386
WINDOWMANAGER.Show(std::make_unique<iwMsgbox>(_("Error"), _("The specified file couldn't be saved!"),
391387
this, MsgboxButton::Ok, MsgboxIcon::ExclamationRed));
392388
}
@@ -428,7 +424,6 @@ void iwMusicPlayer::SetRandomPlayback(const bool random_playback)
428424
->SetTooltip(random_playback ? _("Playback in this order") : _("Random playback"));
429425
}
430426

431-
/// Updatet die Playlist - Combo
432427
void iwMusicPlayer::UpdatePlaylistCombo(const std::string& highlight_entry)
433428
{
434429
auto* cbPlaylist = GetCtrl<ctrlComboBox>(ID_cbPlaylist);

tests/s25Main/audio/testPlaylist.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,20 @@ BOOST_AUTO_TEST_CASE(SaveLoadResultsInSamePlaylist)
145145
BOOST_TEST(helpers::contains(songs, firstSong));
146146
}
147147

148+
BOOST_AUTO_TEST_CASE(SetNextSong)
149+
{
150+
const std::vector<std::string> songs{"s01", "s02", "s03", "s04", "s05"};
151+
Playlist pl(songs, 0, false);
152+
BOOST_TEST(pl.getNextSong() == "s01");
153+
pl.SetStartSong(3);
154+
BOOST_TEST(pl.getNextSong() == "s04");
155+
BOOST_TEST(pl.getNextSong() == "s03");
156+
BOOST_TEST(pl.getNextSong() == "s02");
157+
158+
// Works also when playlist was not started yet (see #1833)
159+
pl = Playlist(songs, 0, false);
160+
pl.SetStartSong(3);
161+
BOOST_TEST(pl.getNextSong() == "s04");
162+
}
163+
148164
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)