UI Bugfix for Character Select

[smithcol11]

Hey! Sorry for the delay with looking at this, haven't had a chance until now. I think I've got the problem fixed, I tested it by adding a few dummy accounts and characters, it seemed to have the selectedPlay.character updating properly. Let me know if it isn't resolved and I will look at it again.

Regarding your comment on #30

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.

You are absolutely right that this could be done better! Svelte was new to me with this project and I didn't take full advantage of its features. I think a separate PR could be done for this; cleaning up bits of how the Svelte code is interacting with the UI and other components. Maybe also changing Maps to be Vectors/Arrays? I made them Maps for ease of use in code, but it is overkill I imagine. Let me know if these are something you would like me look into.

Another thing I noticed while opening this PR - Anytime a change is done to the app, the dist files needs to be rebuilt and get renamed. This causes there be lots of changes with these files unless they are manually renamed. The current one looks like minify was run on it, then bun format, so the variable names aren't super helpful but the code is legible. The one I have in this PR did not have minify run. How should these be committed going forward? Maybe we could add something to the readme about it? If we commit minified code, then only one line change would ever be recorded in versioning, but debugging the UI would be more difficult for someone not building it themselves. Just wanted your thoughts, thanks!

Cheers

[Adamcake]

Thanks I'll take a look.

How should these be committed going forward? Maybe we could add something to the readme about it?

The committed contents of app/dist should be the version that will be used in production. This is so packagers like flatpak won't have to install bun/node/npm/etc to make a source build. (I know it's technically possible, but Javascript in reproducible build systems doesn't work very well and I'd rather just not deal with it.) If I formatted it after minifying then that wasn't intentional. Ideally, bun run lint and bun run format should not operate on dist because there's no reason why it would need to look at the packed sources as well as the original sources. Is there a way we can make it exclude that directory?

debugging the UI would be more difficult for someone not building it themselves

Dev tools already aren't available for people not making a source build, so that's fine by me.

[smithcol11]

Okay Cool! So, I added a .prettierignore file that will exclude dist. Then, I ran bun run minify to make the files significantly smaller for production. Now when dist gets rebuilt, in version control it will be like a few lines changed every time, which is nice.

[Adamcake]

Thanks - works fine now. I messed around with two jagex accounts with two characters on each, and everything I did worked as expected.

If you want to do a separate PR tidying up and making better use of svelte features, I'll gladly accept it, but for now I'm going to merge this and try to get version 0.9 out today.

Thanks again for looking into this.

[smithcol11]

Excited to see it in the release build!

I will look into tidying up and see what I can come up with.

Cheers