add karaoke mode via #INSTRUMENTALS or #KARAOKE tag, key to toggle karaoke mode is K while playing. #850
Conversation
game/config.ini
Outdated
There was a problem hiding this comment.
Do not commit your config.ini
There was a problem hiding this comment.
Ideally config.ini never shows up in these commits at all (rewrite history/force push is fine on branches). I could allow it because this is a relatively small PR should it get merged with squashing enabled, but it would set precedent and now the Piano mode PR also wants in this way, and then someone submits a 1500-line net PR across 3 useful commits but it also changes+undoes line ending changes on everything in src/lib, and well, you can see where this is going.
In general I don't like squashing because it loses information, and I've had way too many times (not on USDX) where a big PR got squashed because "rewriting was too much effort" and then 5 years later it's pinpointed that "somewhere" in there a bug was introduced.
Ideally this PR gets reworked.
game/languages/English.ini
Outdated
There was a problem hiding this comment.
This needs to be added to all translation files.
There was a problem hiding this comment.
This part is not translated in any language. If you press tab in German, for instance, it's all English.
src/base/USong.pas
Outdated
| @@ -935,6 +937,14 @@ function TSong.ReadTXTHeader(SongFile: TTextFileStream; ReadCustomTags: Boolean) | |||
| Log.LogError('Can''t find video file in song: ' + FullFileName); | |||
| end | |||
|
|
|||
| // Karaoke Mp3 | |||
| else if (Identifier = 'INSTRUMENTALS') or (Identifier = 'KARAOKE') then | |||
There was a problem hiding this comment.
The identifier is INSTRUMENTAL, see https://usdx.eu/format/.
|
I was not sure what counts on that page as it also lists
|
|
changed it to #INSTRUMENTAL only @bohning, config is reset, language - the help is not translated for any language, so I'd keep that as is for now |
|
anything still to do here? |
|
Not from my side. I think @barbeque-squared hasn't had time to review and merge it. |
|
actually found one more subtle bug while testing this a bit more (mp3 tag after karaoke tag overwrote the karaoke tag), fixed with this commit |
|
and another one found while playing 😇 |
|
So this toggles the use of the normal audio (#MP3) and the instrumental version (#INSTRUMENTAL), right? I just learned that Melody Mania has implemented a slider where you can set the amount of vocals from 0 % (pure karaoke) to 100 % (pure sing-along. Of course that's quite a bit harder to implement, but that would be awesome. |
|
I was testing this branch but could not get it to work. I suspect it's due to a merge conflict from some other change, git is giving me But before that (probably 46cf3f1) it did apply the diff but gave me EAccessViolation when starting a song. It just shows me the video or the background image for a fraction of a second, then crashes. Without this patch it works fine, so I've ruled out config.ini at this point. what bohning mentions should probably be more of a stretch goal, though shorter-time we can do what we do with |
|
That's quite odd. I've used this one on several occasions already over multiple hours without a crash. Not sure what might cause this? I can try to see what it does on the most recent version. |
|
The crash was my fault, fix PR is open in #874. This only occurs after |
|
Took me a while to remember This only happens on songs that DO NOT have the #INSTRUMENTAL tag! Songs that have it are indeed fine, but it's an optional tag. 865 is the AudioPlayback.Open call. I'm assuming CurrentSong.Karaoke is |
|
Ah, yes that's possible. I don't have any in my library without the tag.
I'll fix that this week.
Cheers,
Daniel
…On 21.07.2024 19:25, barbeque-squared wrote:
Took me a while to remember |./configure --enable-debug| so it prints
stacktraces instead of only "Access violation", but with that I get this
output:
|Exception class: EAccessViolation Message: Access violation
$00000000004E2C4E APPEND, line 796 of base/UPath.pas $0000000000519C93
ONSHOWFINISH, line 865 of screens/controllers/UScreenSingController.pas
$000000000045FF0F DRAW, line 448 of menu/UDisplay.pas $000000000047A3D2
MAINLOOP, line 344 of base/UMain.pas $000000000047A166 MAIN, line 269 of
base/UMain.pas $0000000000407487 main, line 414 of ultrastardx.dpr |
*This only happens on songs that DO NOT have the #INSTRUMENTAL tag!*
Songs that have it are indeed fine, but it's an optional tag. 865 is the
AudioPlayback.Open call. I'm assuming CurrentSong.Karaoke is |nil| there
(which it should be if the song doesn't have it!) but it's basically
getting passed a folder now or something.
—
Reply to this email directly, view it on GitHub
<#850 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN2M6UQAVGZO2RZW76ECXLZNPVHBAVCNFSM6AAAAABH7VDESGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRG4YTSMRTGM>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
…s K while playing.
|
alright. fixed the bug, rebased to new master and squashed the commits |
| @@ -860,7 +862,10 @@ function TSong.ReadTXTHeader(SongFile: TTextFileStream; ReadCustomTags: Boolean) | |||
| if (Self.Path.Append(EncFile).IsFile) then | |||
| begin | |||
| self.Mp3 := EncFile; | |||
|
|
|||
| if (not Assigned(self.Karaoke)) or (self.Karaoke = PATH_NONE()) then | |||
There was a problem hiding this comment.
bugfix was here to also check for not Assigned --> to make sure both .Mp3 and .Karaoke members are initialized to an actual audio file
add karaoke mode via #INSTRUMENTALS or #KARAOKE tag, key to toggle
karaoke mode is K while playing.