UI & UX Changes

[smithcol11]

Hello, I have been a big fan of the project and wanted to give back in some way!
I am not sure if there have been plans to update the UI/UX, but I thought I would throw something together to see what you and others thought. As far as I can tell I haven't introduced any problems or regressions in functionality.

Key Changes

• Moved all HTML generation into the html files themselves. This allows for quicker prototyping for visual changes.
• Introduced tailwindcss to help style the application.
• Changed the UX/UI of the app in general. I tried to think how most users would use it and where relevant buttons should be, but this is all subjective and am interested in what you think. This involved removing some logical components from 'launcher.js'.
• Updated the README to reflect the html and tailwind introduction. I also added some resolutions to problems I ran into while trying to build and run on Linux.

Screenshots of Updated UI/UX

Thoughts

I am very keen and open to hearing your feedback. I am aware that this is likely just a draft or something you will scrap entirely. Please reach out with questions and suggestions!

Cheers

[smithcol11]

When trying to build on Fedora 39, LUA_GLOBALSINDEX kept throwing an error.
Seems to be a version issue? The solution was to change to LUA_REGISTRYINDEX; based on this GitHub post:
poelzi/ulatencyd#51

[Adamcake]

The removal of LUA_GLOBALSINDEX was a widely unpopular change from Lua 5.1 to 5.2, but Bolt uses LuaJIT which is based on 5.1, so most likely you're building against the wrong version. In any case, unless you want to use the highly experimental RS3 plugin loader, you can entirely turn off the Lua dependency with -D BOLT_SKIP_LIBRARIES=1.

[smithcol11]

Ah I see, my system is on Lua 5.4. I will revert this change.

[Adamcake]

The second drop-down seems buggy now. I have two game accounts on my Jagex account but when I log in it only shows the first one. If I log in again, the top drop-down has two entries in it as expected, but after switching between them the bottom one either has 1 or 0 entries in it, when it should always have 2.

[Adamcake]

Well, to be honest I was hoping someone would do this so I wouldn't have to do it myself, so thank you. I have a few points of feedback on the PR itself.

• Overall I like the look and feel of it. Who owns the icons - did you make them?
• No need to change this now (I don't think it's actually possible to), but for future reference, git commit messages should be a maximum of 50 characters. If you want to add context, run git commit --amend and add two newlines after the original message, then type your extended message there.
• I mention in my readme that you should use a development environment that supports Editorconfig Actually I don't - I thought I did. Anyway, issue is you've made .gitignore, oauth.html, launcher.html and game_auth.html not end with a newline anymore. My editorconfig file would have fixed this automatically, so please use it.
• It took me a few seconds to work out how to close the settings menu. I think most people would look for a red "x" at the top-right rather than the top-left. It's also fairly universal that a menu like that should be closed by clicking outside it or pressing Esc. That said however, make sure the initial disclaimer can't be closed by any means other than pressing "I understand".
• The reason I avoided using getElementById in favour of a file that dynamically generates itself was to avoid needless computations. getElementById is fast, but it's not as fast as already having a reference. ReactJS was created to solve this problem, but comes with the burden of being a huge pain to set up in a build system. Still, some kind of generator would be nice. Any thoughts on that?
• Readme additions could possibly be moved to a later "troubleshooting" section

[smithcol11]

The second drop-down seems buggy now. I have two game accounts on my Jagex account but when I log in it only shows the first one. If I log in again, the top drop-down has two entries in it as expected, but after switching between them the bottom one either has 1 or 0 entries in it, when it should always have 2.

I was worried I introduced some kind of bug like this. I only have one account with one character so my testing was pretty limited. I will look into this and expand my testing!

[smithcol11]

Glad to hear these changes are welcome!
Regarding your feedback in order:

• I'm glad you like it overall, I wanted to keep it simple but modern looking. The icons are FOSS from FontAwesome. They use a CC license, as mentioned here on their site. If this isn't okay I can make/use different icons.
• My bad with the commit length! I will see if I can fix that.
• I'll be sure to use your editorconfig, thanks.
• I agree about the settings menu. I actually messed with that window quite a bit. I will make it more intuitive to close in the ways you suggested!
• Very true! With a page this size I figured any cost added by getElementById would be worth it. I didn't want to change the core stack with my changes, so introducing something like React or Vue or Dioxus didn't occur to me. It sounds like this is something you might be interested in adding? I can do so if you would like! I prefer working with one, personally. Adding something like this will likely only slow it down (not noticeably though). I like to reference this bench marking site by krausest; vanilla js is usually fastest for most things.
• I will move those in the README.

I appreciate the comments and feedback! I'll fix things we discussed above. Let me know what you think about adding a front end framework.

Thanks for you time! Cheers

[Adamcake]

React or similar would be nice if you can add it. It can make the build system quite bloated but the generated code in release mode is really small in my experience, so that's fine with me. There's also another issue, that anything involving npm is a massive headache to use in build systems such as Flatpak. I think the best solution to this is to commit packed sources to the repo and include an optional build step explaining how to repack them if people want to do that. I saw you've already committed tailwind's output.css.

On the other hand if you think React would make the page slower to load or interact then I trust your judgement, you seem to know what you're talking about. It seems like it would reduce development overhead a lot though, it's quite a chore to add features to right now as I'm sure you've noticed.

[Adamcake]

The icons are FOSS from FontAwesome. They use a CC license, as mentioned here on their site.

That's fine, thanks, just checking

[smithcol11]

Agreed that a front end framework would make development a lot easier! just need to figure out some nuances like you mentioned. Yeah, committing the tailwind output was to allow people to just use it or repack it.

I should clarify that I am a fan of using frameworks but am by no means an expert. I think their performance shouldn't be an issue. Companies like Netflix and Facebook rely on React for their apps, and they run great. Lol yeah good point, it was a bit of a chore!

I'll experiment with adding a framework and converting the html/js to using it. Then, report my findings and/or commit what I have working. Thanks for the feedback.

[TormStorm]

I really like the overall design and aesthetics, i just think it looks a bit weird that the buttons to launch the different clients are in the big box in the top right. I would suggest moving them below the account dropdowns in the side panel and use the big box for something like updates to Bolt. I also prefer the side panel being on the right side, but that is just my personal preference.

[smithcol11]

@TormStorm thanks for the feedback! I agree that the UI in its current state looks a little off. I spent a lot of time moving things around and have yet to be totally happy with it. You might be right, maybe I should take some design choices from the official launcher, which implements a few things you mentioned. I will post new screenshots as I have them to see what people think.

[TormStorm]

I was working on something similar a while back, so i understand that it can be difficult to figure out where the different elements should be. I don't think taking some design choices from the official client would be a bad thing, it might also make it a bit easier for people who are used to the official client. For reference here is an image of what i was working on, although i think yours looks much cleaner.

[smithcol11]

Ooo yeah I like where you were going with that a lot. I think as I try to construct the interface using a framework I will try to move things around to mimic yours and the official launcher a bit more. I appreciate the inspiration!

[smithcol11]

Hey, just wanted to give an update with what I have been up to regarding UX changes and introducing a framework!

UX

Considering I was going to be taking the UI apart, I decided to do a redesign that I think is a lot better.

As you can see it takes a lot of inspiration from the official launcher and comments left by you both.

• I decided to center the 'launch/play' section as currently there isn't anything that would occupy the empty space if it was put on the left or right side. If something gets introduced, then this is incredibly simple to move :)

Framework

I did a bit of research before deciding on an implementation.

• I went with Svelte. It is a bit of a newer kid on the block. But it is one of the most loved, simple, and performant.
• Svelte recommends using SvelteKit, I felt it had a lot of overkill features for what is needed here, and I wasn't successful in launching the window using it.
• Base Svelte is implemented and working, at least the UI side of things.
• With the rewrite of the front end, I decided to swap to TypeScript, as it is much better to work with when developing. I am a fan of strong types.

Here is what the file structure looks like, in a new folder called 'app', which would replace 'html':

Next Task

The next thing I am working on is moving the 'launcher.js' code into respective components. This is going to take more of my time to implement I think. Reasons being I did the good old foot gun technique by introducing TS and having loads of errors in the original JS file; and relocating / refactoring the code will require me to understand it a lot more to ensure I move things where they belong while not introducing bugs.

Thanks for reading my update. Let me know what you all think! I look forward to your comments, suggestions, and questions.

Cheers

[Adamcake]

Thanks, I appreciate you giving updates. If you're taking launcher.js apart then it'll be important to know that getting a login_provider in your credentials is not possible anymore, this was for non-jagex accounts which can no longer be used via the launcher. So you might like to save yourself some work by cutting out anything to do with login_provider.

[smithcol11]

Yeah of course! Good to know, I will look to removing anything using login_provider. Thanks for the tip.

[smithcol11]

Hello again, hope the weekend is going well! I wanted to give another update and push my changes for others to look at.

The frontend has been moved into app, using Svelte, TypeScript, and Tailwind; things seem to be working for the most part. I have been trying to do things to break it and have fixed everything I found up till now.

Some key updates and additions since the last update

• TSLint: This will check the code in a number of ways and will help with code consistency in the frontend.
• Prettier: An opinionated formatter, both it and TSLint are entirely customizable. I set it to follow your editorconfig rules.
• Vitest: I added vitest to the dev dependancy. But I have yet to write any tests.
• README: I updated the readme for the frontend to include all additions to building/developing and caveats.

Current tasks

• The config behaves strangely sometimes (mostly around the preference of client), so I must have some kind of bug there and need to test it more.
• I want to sync the naming convention of everything in the frontend. Prettier and TSLint can enforce this once set. For example a lot of what I wrote was in snake_case while existing code was in camelCase. It will be good to make it all the same. I think the standard for JS and TS is to use camelCase, so I will probably go with that.
• The any type (which I am not a huge fan of). In about 30 places across the files, I use the any type but want to remove these. I make note of this in the README, but sometimes it is really tedious to work around the type system, especially when it could be multiple different values, or null for example. I want to see about remove as many of these as I can.
• Finding and fixing problems I introduced with my changes.

Overall, I am happy with the progress I have made. I look forward to what you all think! Feedback, comments, concerns, and questions are appreciated. Let me know any bugs you find and I will set out to fixing them asap.

Thanks

[Adamcake]

Bolt/app/src/lib/TopBar.svelte

Line 47 in 5f5d83b

// if no account is signed in, open the Jagex login

Don't put the company name in the repo please.

[Adamcake]

Wanted to mention, I changed the package manager to bun instead of npm.

Yeah I know about bun. It certainly runs better and has a nice build system (zig.) There are a few things I don't like about it but none of them are relevant here.

[Adamcake]

Okay, last few issues:

• when it prints a message with the system time next to it, it doesn't add any leading zeroes to the time, e.g. 12:9:3
  should be 12:09:03
• when downloading a game client, it says "undefined:undefined:undefined" instead of the system time
• In OSRS settings "rich presence" is misspelled as "rich presense"
• couple of typos in app/readme.md:
  • "missedd" -> "missed"
  • "dependancy" -> "dependency", and "dependancies" -> "dependencies"
  • "initializing", "optimize", "optimizing" -> "initialising", "optimise", "optimising" (British English spelling preferred)
  • "lets see" and "lets minify" -> "let's see" and "let's minify"

Beyond that I agree, this is about ready to go.

[smithcol11]

Thanks for finding some more bugs! It is funny how the order in which I Implemented things meant I never saw that undefined time one.

I just pushed a couple changes that fix everything you mentioned.

Let me know if you have anything else regarding style/code you want me to change.

[Adamcake]

Ah, just found another problem, and I really should've tested this sooner: logins expire after 30 minutes.

Okay, so, when a set of oauth credentials is granted by the login server, it has an "expiry" field which usually indicates a time 30 minutes after it was granted. If that has expired, you won't be able to use the credentials anymore for things like the displayName endpoint, but you can still use the refresh_token to renew them. checkRenewCreds renews them if they're expired.

I've just checked. 28 minutes after being granted, works fine. 32 minutes after being granted, I get a 401 from displayName. Which either means checkRenewCreds isn't being called or the new credentials aren't being saved to the file. (Or perhaps checkRenewCreds has been changed to operate by value instead of by reference?)

[smithcol11]

Ah, just found another problem, and I really should've tested this sooner: logins expire after 30 minutes.

Yeah, I did notice weird behavior with the login expiring. I chalked it up to the dev environment but guess I shouldn't have assumed that. I will look into fixing it right now.

[Adamcake]

Well I think there is a separate issue where the hot-reload will re-use an old version of the credentials file. That one's my fault, not yours - you might want to turn off hot-reloading while you're investigating.

[smithcol11]

I'm not having too much luck getting passed the xml.send request. I always get this 400 response:

{
    "error": "invalid_grant",
    "error_description": "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client."
}

Leads me to assume something is wrong with the token. I am not super familiar with refresh tokens, but I am looking into it. Any ideas?

[Adamcake]

Just pushed a fix to master for the hot-reload issue if that's any help.

That's the most generic error message unfortunately. Can you show what request you're sending, with the sensitive parts censored out?

[smithcol11]

Okay I merged your change in.
This has to do with the checkRenewCreds request.

request url: https://account.jagex.com/oauth2/token

payload: {
  grant_type: refresh_token
  client_id: com_jagex_auth_desktop_launcher
  refresh_token: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
}

Yeah people online say finding the cause of this error can be a bit of a pain haha.
I will do some more testing.

[Adamcake]

For me, taking the refresh_token from my creds file and putting into this request, I got a 200 response with some new credentials:
curl https://account.jagex.com/oauth2/token -X POST -d "client_id=com_jagex_auth_desktop_launcher&grant_type=refresh_token&refresh_token=95WZ3aGBirQh[.....]"
... but when I took the refresh_token from the response and tried to do the same thing again, I got the same error as you.

[Adamcake]

Nevermind, I think I made a copy-paste error last time. It works every time now.

So what are you doing differently from that?

[smithcol11]

I think my problem had to do with these lines here in checkRenewCreds. These are your original lines.

const c = parseCredentials(xml.response);
if (c) {
  Object.assign(creds, c);
  credentialsAreDirty = true;
  resolve(null);
}

Javascript does like weird pass by reference where you can modify what is inside an object but by reassigning you don't effect the original copy. 'creds' comes in as a parameter. I changed these lines here and I don't think I was assigning the fields properly due to a 'find and replace' bug.

I am testing now, gonna let my login expire and see what happens.

[smithcol11]

Okay I think I've got it working. I testing by forcing my login to send a refresh on every boot and it was working.
Thanks for the help with that!

[Adamcake]

Yep I can't find any more issues with this. Thanks for your hard work.

[smithcol11]

Awesome! Great to have it merged in, thanks for the help and feedback throughout.
Let me know if you need anything else.

[Adamcake]

@smithcol11 I just found an issue in your PR by coincidence while testing something else. If you have multiple game accounts under one jagex account, the drop-down menu visually seems like it remembers which account you had selected, but if you click the play button it will always launch the first listed account unless you explicitly change it to a different one. Do you know why that would be the case?

[smithcol11]

Yeah I have an idea; I keep track of the currently selected account, character, game, etc. I must not be setting it when it loads up preferences for that. I will look into it and open a PR soon, should be an easy fix I imagine.

[Adamcake]

Okay thanks. From what I've learned about Svelte so far I think this menu could be done with fewer variables than this. It says a Select's "value" attribute is the same as the "value" of the currently selected Option, and an Option's "value" can be anything, not just a string. So, if you use the {#each} block to set each Option's "value" to a Character object, and on the Select tag you set bind:value={someVariable}, then someVariable would update according to which Character is selected.

But that's just how I'd look at it. I'll leave it up to you since it's your PR.