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

Prevent type mismatch crash #1938

Merged
merged 1 commit into from
Oct 6, 2024

Conversation

TwitchBronBron
Copy link
Contributor

@TwitchBronBron TwitchBronBron commented Sep 28, 2024

Changes

Prevent type mismatches in sdk.bs where we were passing integer arguments into string-only param spots for the Substitute function. (discovered by testing out the BrighterScript v1 alpha release)

image

Proven that this is an issue:
image

Issues

@TwitchBronBron TwitchBronBron requested a review from a team as a code owner September 28, 2024 04:19
@@ -2099,7 +2099,7 @@ namespace api

' Gets an HLS subtitle playlist.
function GetHLSSubtitlePlaylistURL(id as string, streamindex as integer, mediasourceid as string, params = {} as object)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not overly familiar with the purpose of this file, but these three functions that I'm fixing are not called anywhere in the roku codebase. Should they be deleted instead of fixed?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the original sdk file had all the API calls and it was eventually trimmed down to only those we use. I'm fine with removing them.

Copy link
Member

Choose a reason for hiding this comment

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

The original purpose of the file was to have a matching function for every API call that Jellyfin uses (https://api.jellyfin.org/). It was originally created as a roku module then later converted after we switched to brighterscript.

I don't believe we ever trimmed down the functions which is why we have errors for functions we don't use. I'm open to removing any or all of the functions we don't use

Copy link
Member

Choose a reason for hiding this comment

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

We'll clean these up later #1967

@cewert cewert added the code-cleanup This refactors or improves the code in some way; makes the code more maintainable. label Sep 28, 2024
@cewert cewert merged commit 38bb8dd into jellyfin:master Oct 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-cleanup This refactors or improves the code in some way; makes the code more maintainable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants