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

"Submit run to speedrun.com" Button in context menu #2211

Closed
wants to merge 3 commits into from
Closed

"Submit run to speedrun.com" Button in context menu #2211

wants to merge 3 commits into from

Conversation

eltharynd
Copy link

So... the share to speedrun.com feature has never worked for me..

So i implemented my own..
You have to provide your API key in the Additional Info tab, then on the right click context menu you can chose submit to sr.com to automatically post the run with a placeholder video.

c# and .net are not exactly my forte so if you guys could have a look at it that would be great..
tyvm

@wooferzfg
Copy link
Member

Would it be possible to fix the issues with the existing functionality instead?

@eltharynd
Copy link
Author

I guess it would but i'm not even sure what they were actually intended to do..
Cause the name of the buttons did not represent what was in the code...

I actually wanted to remove those since (according to google) nobody has managed to make those work for years...
But i wanted to leave this decision to you guys..

I might look into those tomorrow and see if i get it

@eltharynd
Copy link
Author

eltharynd commented Jan 12, 2022

In all honesty..

  • The "associate with speedrun.com" button doesn't make sense at all... Especially since it's in the splits options and not on a run per run basis... I would remove that
  • The "submit run" can't work at all cause you need to provide an API key for that... Besides it doesn't make any sense that that would be in the splits options...

I might spend some more time on it and delete those two buttons and related functionality and polish my solution a bit better... (maybe by moving the API key in global settings for example)
What do you folks think?

Here's how i implemented the new one:
1
2

@wooferzfg
Copy link
Member

  • The "associate with speedrun.com" button only appears if you have a new PB. Once the run is associated, it'll change to "view on speedrun.com" until you get a new PB. Because of that, it is per-run.
  • When you click on the button for the first time, it should open a window that tries to retrieve an API key. (Not sure if that's still working?)

@eltharynd
Copy link
Author

eltharynd commented Jan 13, 2022

Currently submit run opens a window that tells you you need to add an API key (but there's no way to add an API key anywhere...

Meanwhile associate run opens a popup that asks for the run url, but it accepts no url whatsoever.. i tried with many combinations... Googling around it appears it actually is supposed to work with splits.io or something.. but that's now a paid service and, frankly i never used it and o don't know many people who use it in my community...

But now that that i understand better what the original thought behind those was i will look into it more and fix it..

Imo tho the "submit" button should not be in the edit splits/additional info.. that's hidden away behind too many clicks... I wanna sett hat stuff up once when i set my splits up then just right click and submit from the context menu...

I will remove the old buttons and their (not working) functionality and make the submit function a bit better.
This button already checks if the run is ready to be uploaded, i will also grey the option out, but i don't see reason to disable it if it's not a PB... One might still want to upload their run for statistics, or maybe they didn't get a PB on their category but it's a PB for another (games with deathless categories for example), I'd still leave that decision to the user personally...

Might i ask, while I'm at it, some clarification regards the difference between game time and time with no loads? Is it me or they're one and the same for livesplit? All i could find was the real time and another variable that stores the load time, so one minus the other is time w/o loads, but isn't that the same as in-game time too? Is there some documentation about that somewhere?

EDIT: reminder for when it look at this again, look into an automatic timestamp functionality for twitch, and the possibility to create an highlight with the last two timestamps through twitch api

@wooferzfg
Copy link
Member

Sharing a run to speedrun.com previously opened a web page on speedrun.com that showed the API key, which LiveSplit then retrieved. If this isn't working anymore, we should update it to make it work.

"Associate run" should accept a speedrun.com run URL.

You should be able to submit a run from either "Edit Splits" or the "Share" dialog. In order to get a PR merged, I'd suggest not trying to change this behavior.

IGT and Time Without Loads are the same in LiveSplit, but distinct on Speedrun.com. It depends on which timing methods the game accepts in its rules.

@wooferzfg
Copy link
Member

Highlighting functionality already exists: https://github.com/furious/LiveSplit.AutoStreamMarker

@eltharynd
Copy link
Author

eltharynd commented Jan 13, 2022

I don't see how you can make an automatic API key retrieval tho...
It's obviously hidden behind authentication.

This is the link used by the app:
https://www.speedrun.com/api/auth
The code then parsed the page for the API key.

That URL doesn't exist anymore, but even if you used the current one (https://www.speedrun.com/eltharynd/settings/api) it wouldn't work without you authenticating first.
This said:

  • Speedrun.com doesn't offer an external API level authentication besides the API key
  • if you opened, say, a headless browser and authenticated inside of LiveSplit it would
    • not guarantee you wouldn't have to authenticate again all the time
    • be a security risk bad practice

I understand what the thought behind this was, but it wasn't a good one (on SR.com's part, since it worked that way apparently).
I will fix the functionality, but I will replace the API key retriever with a manual field in settings (we can put a "where to find this?" link next to it that opens the right page on a browser so that the user is most likely already securely logged in.

That way we'd be authenticating through the regular, intended SR procedure...

@wooferzfg
Copy link
Member

Rather than a new field in the settings, can we just replace the API key retrieval that we currently have with a dialog window that has an input field for the API key? (Should be similar to the window that opens when you click "Open Splits from URL".)

@wooferzfg
Copy link
Member

The Speedrun.com API key should be in the WebCredentials class, not in the Settings.

""comment"": ""Automatically submitted by LiveSplit, video might be temporary."",
""video"": ""https://youtube.com/watch?v=mumblefoo""
VARIABLES_PLACEHOLDER
}}
Copy link
Member

@wooferzfg wooferzfg Jan 13, 2022

Choose a reason for hiding this comment

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

We already have SpeedrunComSharp for all of this. Please limit the scope of this PR to just replacing the current way of retrieving the API key (opening a webpage and grabbing it automatically) with a new way (a dialog box with a link to the settings page and an input field for the API key) - otherwise, this won't get merged.

@eltharynd
Copy link
Author

eltharynd commented Jan 15, 2022

There is no "straight to API key" link until you've already logged in or inserted the username... besides...

  • I strongly disagree with the idea of having the user log in inside the app
  • Especially if it's ultimately just to still copy and paste the API key

It would not only be a security risk... I will no develop something that lets user insert their own password for someone else's access not through their own API... that is just bad practice... and a safety risk...
But even if we went beyond that: it's ultimately an annoyance... Everybody is already logged in to speedrun.com in their browser, in which they have access to many a way to make inserting the password even more secure...

I can fix the rest of the code if you want...
But i will replace the login part with just pasting the API key inside the settings.. that way they will be available for all splits/games/cats they have... As I had already changed, btw...

image

Let me know if you would accept this compromise, otherwise I will have to make this as a plugin instead

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.

None yet

2 participants