From a7b208b92daa134ee6ed1a31c3bf1bb85ae7c70d Mon Sep 17 00:00:00 2001 From: Salanto Date: Sun, 21 Mar 2021 04:59:15 +0100 Subject: [PATCH 1/4] small refactor handle_music --- src/courtroom.cpp | 90 +++++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 61 deletions(-) diff --git a/src/courtroom.cpp b/src/courtroom.cpp index fe5f74d..6446387 100644 --- a/src/courtroom.cpp +++ b/src/courtroom.cpp @@ -3677,6 +3677,10 @@ void Courtroom::handle_song(QStringList *p_contents) if (f_contents.size() < 2) return; + bool looping = false; // No loop due to outdated server using serverside looping + int channel = 0; // Channel 0 is 'master music', other for ambient + int effect_flags = 0; // No effects by default - vanilla functionality + QString f_song = f_contents.at(0); QString f_song_clear = f_song.left(f_song.lastIndexOf(".")); if (f_song.startsWith("http")) { @@ -3684,69 +3688,31 @@ void Courtroom::handle_song(QStringList *p_contents) QString f_song_decoded = QUrl::fromPercentEncoding(f_song_bytearray); f_song_clear = f_song_decoded.left(f_song_decoded.lastIndexOf(".")); } - f_song_clear = f_song_clear.right(f_song_clear.length() - - (f_song_clear.lastIndexOf("/") + 1)); + f_song_clear = f_song_clear.right(f_song_clear.length() - (f_song_clear.lastIndexOf("/") + 1)); + int n_char = f_contents.at(1).toInt(); - // Assume the song doesn't loop unless told otherwise (due to most outdated - // servers handling looping through serverside) - bool looping = false; - // Channel 0 is the 'master music', other channels would commonly be used for - // ambience - int channel = 0; - // No effects assumed by default - vanilla functionality - int effect_flags = 0; + if (p_contents->length() > 3 && p_contents->at(3) == "1") + looping = true; - if (n_char < 0 || n_char >= char_list.size()) { - int channel = 0; - if (p_contents->length() > 3 && p_contents->at(3) == "1") - looping = true; + if (p_contents->length() > 4) // eyyy we want to change this song's CHANNEL huh + channel = p_contents->at(4).toInt(); // let the music player handle it if + // it's bigger than the channel list - if (p_contents->length() > - 4) // eyyy we want to change this song's CHANNEL huh - channel = p_contents->at(4).toInt(); // let the music player handle it if - // it's bigger than the channel list - - if (p_contents->length() > 5) // Flags provided to us by server such as Fade + if (p_contents->length() > 5) { // Flags provided to us by server such as Fade // In, Fade Out, Sync Pos etc. - { - effect_flags = p_contents->at(5).toInt(); - } - music_player->play(f_song, channel, looping, effect_flags); - if (f_song == "~stop.mp3") - ui_music_name->setText(tr("None")); - else if (channel == 0) { - if (file_exists(ao_app->get_sfx_suffix(ao_app->get_music_path(f_song))) && !f_song.startsWith("http")) - ui_music_name->setText(f_song_clear); - else if (f_song.startsWith("http")) - ui_music_name->setText(tr("[STREAM] %1").arg(f_song_clear)); - else - ui_music_name->setText(tr("[MISSING] %1").arg(f_song_clear)); - } + effect_flags = p_contents->at(5).toInt(); } - else { + + bool is_stop = (f_song == "~stop.mp3"); + if (!(n_char < 0) && !(n_char >= char_list.size())) { QString str_char = char_list.at(n_char).name; QString str_show = ao_app->get_showname(str_char); - if (p_contents->length() > 2) { if (p_contents->at(2) != "") { str_show = p_contents->at(2); } } - if (p_contents->length() > 3 && p_contents->at(3) == "1") { - looping = true; - } - if (p_contents->length() > - 4) // eyyy we want to change this song's CHANNEL huh - channel = p_contents->at(4).toInt(); // let the music player handle it if - // it's bigger than the channel list - - if (p_contents->length() > 5) // Flags provided to us by server such as Fade - // In, Fade Out, Sync Pos etc. - { - effect_flags = p_contents->at(5).toInt(); - } - bool is_stop = f_song == "~stop.mp3"; if (!mute_map.value(n_char)) { if (is_stop) { log_ic_text(str_char, str_show, "", tr("has stopped the music")); @@ -3756,19 +3722,21 @@ void Courtroom::handle_song(QStringList *p_contents) log_ic_text(str_char, str_show, f_song, tr("has played a song")); append_ic_text(f_song_clear, str_show, tr("has played a song")); } - music_player->play(f_song, channel, looping, effect_flags); - if (is_stop) - ui_music_name->setText(tr("None")); - else if (channel == 0) { - if (file_exists(ao_app->get_sfx_suffix(ao_app->get_music_path(f_song))) && !f_song.startsWith("http")) - ui_music_name->setText(f_song_clear); - else if (f_song.startsWith("http")) - ui_music_name->setText(tr("[STREAM] %1").arg(f_song_clear)); - else - ui_music_name->setText(tr("[MISSING] %1").arg(f_song_clear)); - } } } + + music_player->play(f_song, channel, looping, effect_flags); + if(is_stop) { + ui_music_name->setText(tr("None")); + } + else if (channel == 0) { + if (file_exists(ao_app->get_sfx_suffix(ao_app->get_music_path(f_song))) && !f_song.startsWith("http")) + ui_music_name->setText(f_song_clear); + else if (f_song.startsWith("http")) + ui_music_name->setText(tr("[STREAM] %1").arg(f_song_clear)); + else + ui_music_name->setText(tr("[MISSING] %1").arg(f_song_clear)); + } } void Courtroom::handle_wtce(QString p_wtce, int variant) From 2c5da36992ce4320964b3b2e3b2eaa709ac1be0b Mon Sep 17 00:00:00 2001 From: Salanto Date: Sun, 21 Mar 2021 21:17:52 +0100 Subject: [PATCH 2/4] Apply suggested changes --- src/courtroom.cpp | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/courtroom.cpp b/src/courtroom.cpp index 6446387..36ed19f 100644 --- a/src/courtroom.cpp +++ b/src/courtroom.cpp @@ -3677,6 +3677,7 @@ void Courtroom::handle_song(QStringList *p_contents) if (f_contents.size() < 2) return; + bool ok, ok2, ok3; //CharID,Channel, Effect bool looping = false; // No loop due to outdated server using serverside looping int channel = 0; // Channel 0 is 'master music', other for ambient int effect_flags = 0; // No effects by default - vanilla functionality @@ -3690,18 +3691,25 @@ void Courtroom::handle_song(QStringList *p_contents) } f_song_clear = f_song_clear.right(f_song_clear.length() - (f_song_clear.lastIndexOf("/") + 1)); - int n_char = f_contents.at(1).toInt(); + int n_char = f_contents.at(1).toInt(&ok); + if (!ok) + return; if (p_contents->length() > 3 && p_contents->at(3) == "1") looping = true; - if (p_contents->length() > 4) // eyyy we want to change this song's CHANNEL huh - channel = p_contents->at(4).toInt(); // let the music player handle it if - // it's bigger than the channel list - - if (p_contents->length() > 5) { // Flags provided to us by server such as Fade - // In, Fade Out, Sync Pos etc. - effect_flags = p_contents->at(5).toInt(); + if (p_contents->length() > 4) { + // eyyy we want to change this song's CHANNEL huh + // let the music player handle it if it's bigger than the channel list + channel = p_contents->at(4).toInt(&ok2); + if (!ok2) + return; + } + if (p_contents->length() > 5) { + // Flags provided to us by server such as Fade In, Fade Out, Sync Pos etc. + effect_flags = p_contents->at(5).toInt(&ok3); + if (!ok3) + return; } bool is_stop = (f_song == "~stop.mp3"); @@ -3726,7 +3734,7 @@ void Courtroom::handle_song(QStringList *p_contents) } music_player->play(f_song, channel, looping, effect_flags); - if(is_stop) { + if (is_stop) { ui_music_name->setText(tr("None")); } else if (channel == 0) { From 298422d45344b8e92ca5a8dd7002ecf92e185ea6 Mon Sep 17 00:00:00 2001 From: Salanto Date: Mon, 22 Mar 2021 20:45:10 +0100 Subject: [PATCH 3/4] Update courtroom.cpp to apply change requests --- src/courtroom.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/courtroom.cpp b/src/courtroom.cpp index 36ed19f..e6bd57b 100644 --- a/src/courtroom.cpp +++ b/src/courtroom.cpp @@ -3677,7 +3677,7 @@ void Courtroom::handle_song(QStringList *p_contents) if (f_contents.size() < 2) return; - bool ok, ok2, ok3; //CharID,Channel, Effect + bool ok; // Used for charID, channel, effect check bool looping = false; // No loop due to outdated server using serverside looping int channel = 0; // Channel 0 is 'master music', other for ambient int effect_flags = 0; // No effects by default - vanilla functionality @@ -3701,19 +3701,19 @@ void Courtroom::handle_song(QStringList *p_contents) if (p_contents->length() > 4) { // eyyy we want to change this song's CHANNEL huh // let the music player handle it if it's bigger than the channel list - channel = p_contents->at(4).toInt(&ok2); - if (!ok2) + channel = p_contents->at(4).toInt(&ok); + if (!ok) return; } if (p_contents->length() > 5) { // Flags provided to us by server such as Fade In, Fade Out, Sync Pos etc. - effect_flags = p_contents->at(5).toInt(&ok3); - if (!ok3) + effect_flags = p_contents->at(5).toInt(&ok); + if (!ok) return; } bool is_stop = (f_song == "~stop.mp3"); - if (!(n_char < 0) && !(n_char >= char_list.size())) { + if (n_char > 0 && n_char < char_list.size()) { QString str_char = char_list.at(n_char).name; QString str_show = ao_app->get_showname(str_char); if (p_contents->length() > 2) { From 2a1905d009aafaf571b9c2cd50fdc3326194def3 Mon Sep 17 00:00:00 2001 From: oldmud0 Date: Tue, 30 Mar 2021 22:48:10 -0500 Subject: [PATCH 4/4] Trivial bounds check fix --- src/courtroom.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courtroom.cpp b/src/courtroom.cpp index e6bd57b..fabcac8 100644 --- a/src/courtroom.cpp +++ b/src/courtroom.cpp @@ -3713,7 +3713,7 @@ void Courtroom::handle_song(QStringList *p_contents) } bool is_stop = (f_song == "~stop.mp3"); - if (n_char > 0 && n_char < char_list.size()) { + if (n_char >= 0 && n_char < char_list.size()) { QString str_char = char_list.at(n_char).name; QString str_show = ao_app->get_showname(str_char); if (p_contents->length() > 2) {