UltraStar-Deluxe / USDX

The free and open source karaoke singing game UltraStar Deluxe, inspired by Sony SingStarâ„¢
https://usdx.eu
GNU General Public License v2.0
846 stars 161 forks source link

Add ability to play sounds via Lua for party mode plug-ins. #856

Closed hoehermann closed 2 months ago

hoehermann commented 4 months ago

For example, "Hold the line" wants to play the "dismissed" sound.

I do not know if the restructuring I did appeals the devs here. I am up for a code-review and discussion.

Warning: Was tested only with Linux and Lua 5.3. If some developer on Windows could check compatibility, that would be great.

barbeque-squared commented 2 months ago

I finally had time to give this a first quick review; I'll do a more thorough one later on where I'm also going to read all the documentation comments etc.

I'll admit seeing 51 files modified scared me a bit at first, but the majority of them is just trivial music references. I don't really like the GetSound(SOUNDNAME_XXX) all throughout the code though, some people delete/rename the bundled sounds to get rid of them, I don't want this change to start spamming useless loglines in that case. I'm not sure on the "smart" loading of GetSound(string), I think?

I also can't quite figure out if Lua's UnloadSound is meant to be the opposite of OpenSound. It's very clear in UMusic that there's Add and Remove, but if the Lua functions are meant to reflect that I'd expect something like OpenSound+CloseSound or LoadSound+UnloadSound or maybe simply AddSound+RemoveSound?

Alternatively maybe Lua can work with only one function, PlaySound which doesn't touch any of the core game stuff and just loads+plays+unloads it? Disregarding whether that's technically even possible or not, I'd like to know what Lua plugins actually need from USDX, because I don't know (well, I have some ideas and local mods, but they have to do with literally everything except party mode).

hoehermann commented 2 months ago

I finally had time to give this a first quick review

Thank you for your time.

I'll admit seeing 51 files modified scared me a bit at first, but the majority of them is just trivial music references. I don't really like the GetSound(SOUNDNAME_XXX) all throughout the code though, some people delete/rename the bundled sounds to get rid of them, I don't want this change to start spamming useless loglines in that case.

Indeed, while I was messing with the other Lua stuff, I was looking for a trivial way to play a sound via Lua. Then I noticed this TODO and thought "I can do that". Then I was kind of getting carried away. 😅 I have no idea if my proposal is "good" in any fashion. I only know my use-case (casual karaoke party). I do not have a broader overview of the likings of the user-base. If it is just about the log output, I can easily remove that.

I'm not sure on the "smart" loading of GetSound(string), I think?

That kind of… happened. I noticed the sounds are identified by their file-name (or more exactly the file-path). I do not really like introducing arbitrary numerical IDs – we already have the string to identify a sound. After I implemented that, I noticed that the lookup-method is also able to load the requested file on the fly. This gives some resilience when someone forgot to explicitly load a sound before trying to use it. But it can be regarded as bloat, I see that.

I also can't quite figure out if Lua's UnloadSound is meant to be the opposite of OpenSound. It's very clear in UMusic that there's Add and Remove, but if the Lua functions are meant to reflect that I'd expect something like OpenSound+CloseSound or LoadSound+UnloadSound or maybe simply AddSound+RemoveSound?

Yes, putting them in line is a good idea. I was unsure on how to proceed and getting some feed-back by an experienced maintainer seemed to be in order.

Alternatively maybe Lua can work with only one function, PlaySound which doesn't touch any of the core game stuff and just loads+plays+unloads it?

I can do that. Then we would have the core sounds in the public fields of TSoundLibrary and the loadable sounds in the array of TAudioPlaybackStream. I was not sure if that would be the preferred way of doing it – considering the "have an array of TAudioPlaybackStream" TODO. I would implement load, play and unload in separate methods since loading (and decoding) can lead to a short delay ("hiccup") which I dem very okay at start-up, but not during gameplay. Does that sound okay?

Disregarding whether that's technically even possible or not, I'd like to know what Lua plugins actually need from USDX, because I don't know (well, I have some ideas and local mods, but they have to do with literally everything except party mode).

This may sound daft and is an unrelated question, but… how to use plug-ins outside of party mode? I have read it in comments, but I never saw an option for that (neither in the GUI, nor in the code).

barbeque-squared commented 2 months ago

Then we would have the core sounds in the public fields of TSoundLibrary and the loadable sounds in the array of TAudioPlaybackStream.

I think if you want to not have loading delays when playing sounds from Lua, you'll probably need to have the load/play/unload functions all available to Lua. If we can give every plugin its own array of TAudioPlaybackStream (or whatever), then that should work? That way the core game is completely separate from the plugins anyway and it doesn't matter how the core game does its inner workings. (don't worry about deduplicating the sounds between plugins yet, it's irrelevant until we start getting reports of USDX using 4GB of memory or something)

There might need to be an extra layer created somewhere.

This may sound daft and is an unrelated question, but… how to use plug-ins outside of party mode? I have read it in comments, but I never saw an option for that (neither in the GUI, nor in the code).

Usdx.Hook('Display.Draw', 'SomeVoidFunctionInMyLuaPluginThatRunsEveryFrame') That's actually valid Lua code already! It's just not very useful right now because there's extremely little functionality, but see #554 for some inspiration (I need to redo it with less functions/the ones that actually work) and especially https://github.com/UltraStar-Deluxe/USDX/pull/554/files#diff-48d6018385719365825ecda9ff1da5d9b5f38c755bd267d5f3a409bce8d14445R561

That said, more/working hooks would also be a solution, but I'd need to think about which way this should eventually go.

I use this for things like a song log, showing the most recently played x songs, sending information about the current song and progress to an external script (that 'happens' to then send stuff to a lighting console). During 2020/2021 I was able to reliably 'share' the singing screen over the internet with stupidly little delay (think 80ms of which 50-60ms was just ping).

It places more responsibility on the plugin developer this way; Party Mode relies on magic stuff (Sing() or Finish() is Party Mode magic, which I hate -- you could just Usdx.Hook('any string that USDX understands', 'MyFunction'). I'm pretty sure there actually is an actual hook for songs starting already, it just obviously doesn't get called if you're singing songs outside of Party Mode. And (this is speculation on my part) I bet fixing these kind of things is going to break, like, all the Party Mode plugins (I see it the other way around: Party Mode blocks everything else).

hoehermann commented 2 months ago

If we can give every plugin its own array of TAudioPlaybackStream (or whatever), then that should work?

That would probably be commit https://github.com/UltraStar-Deluxe/USDX/pull/856/commits/f61bc1c, however I would need to to is move the array declaration from UMusic's TSoundLib to ULuaCore's TLuaPlugin. No other parts of the code would be changed (and this TODO remains unresolved, but I can live with that). I will close this pull request and come up with another one.

Thank you for sharing your insights about plug-ins. I want to look into the hooks a bit to form a more sophisticated opinion. This pull-request comment does not seem to be the right place for brain-storming/discussions. What is the best place to reach you?