Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more functions to use in Lua plugins #554

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

barbeque-squared
Copy link
Member

I extensively use the Lua plugin system to stream (for now only the most important parts of) the user interface over the internet. I would actually like to add much more in the future, but this is a good start, plus this already has some issues that's blocking further development anyway.
The most important changes, and issues with them, are listed below.

  • no longer use array [0..UIni.IMaxPlayerCount-1] to define the available functions
  • add functions to get static information related to players (names, colors, levels)
  • add functions to get information related to scores (notescores, goldenscores, linescores)
  • add functions to get static song information (totalbeats, background, artist, title, md5). An alternative would be to have just one function that returns all of these, plus the already existing ones (bpm, songlines) plus the other static information as an object, because I'll want to also read other fields (eg year, creator, etc) in the future.
  • add a function to return which screen is currently open in USDX (currentscreen). For now this only returns score, sing or song; all others return unknown.
  • add a function to get whether the current/last-played song was played to the very end (sungToEndWithoutPausing) whether or not scores should be able to be submitted.
  • add a function to get the registered notes (getPlayerNotes). This is currently an object with just one key, Notes, which contains all the useful information, which should probably be flattened.

Then there are four last functions (getVisibleSongs, getSelectedIndex, getSongNumbers, getSelectedSong) which:

  • most of them don't even work, but this is due to the CatSongs object not returning the information I'm seeing on the screen (it only updates when a song ends?)
  • they really should be in their own file (ULuaScreenSong?) but I got compilation errors when I tried that. Perhaps some of the above functions (for example the static song info, or the current screen) should perhaps also not be in ULuaScreenSing, but have the same cause.

Please note that my Pascal knowledge is not good enough to be able to improve the shortcomings myself, but maintainers can edit the branch. It's also fine to re-implement this in several commits if that makes it easier, and then just reject this PR.

const
ULuaScreenSing_Lib_f: array [0..UIni.IMaxPlayerCount-1] of lual_reg = (
ULuaScreenSing_Lib_f: array [0..29] of lual_reg = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does this magic number 29 come from? Please add a code comment or (even better) move this magic number to the top of the class as a static number.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

29 is just the length of the array (it's either 29 or 30, I guess). If there's a way to do this without using numbers in the first place, that's by far the best solution, otherwise a code comment should suffice (I am against moving this to a constant, because literally no part of the code except the array initializer cares about its size). Does Pascal have a way of instantiating these things without hardcoded numbers?

And even then, I'd argue that a hardcoded number would be more explanatory than using a completely unrelated constant.

(github never gave me any notification about this)

@barbeque-squared
Copy link
Member Author

I'm going to split this PR in the near future. There's two of the functions this PR adds that I use extensively across multiple custom plugins: the Artist and the Title. All the others are for one highly specific usecase which doesn't have much use at the moment, so I'll just keep those local.

I'll close this one once the smaller PR has been made (making more commits to this one is going to look weird in the history, and I'm not force-pushing this one so that the extended API is at least still somewhere).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants