minetest-world / mtlauncher

GNU Lesser General Public License v2.1
5 stars 3 forks source link

screenshots page #9

Closed recluse4615 closed 1 year ago

recluse4615 commented 1 year ago

it'd be nice to have a simpel screenshots page that just shows all screenshots across all versions, indexed by date

nothing crazy

Miniontoby commented 1 year ago

I have made some code, I haven't tested it yet, but will make an PR when I have tested it.

If you want then you can assign it to me, but yeah

recluse4615 commented 1 year ago

sure, @Miniontoby - assigned!

you can follow the instructions here to get set up: https://github.com/minetest-world/mtlauncher/wiki/initial-set-up#developing-locally

just make sure when you PR to PR in to main!

Miniontoby commented 1 year ago

I get this error when running npm run dev or npm run build

error: linker `link.exe` not found
  |
  = note: program not found

note: the msvc targets depend on the msvc linker but `link.exe` was not found

note: please ensure that VS 2013, VS 2015, VS 2017, VS 2019 or VS 2022 was installed with the Visual C++ option

But I installed the windows 10 sdk and the c++ build tools things

recluse4615 commented 1 year ago

ddid you reboot your computer after installing them?

at a minimum you might need to restart your terminal (to update %PATH%) but probably best to restart your whole computer

Miniontoby commented 1 year ago

ddid you reboot your computer after installing them?

at a minimum you might need to restart your terminal (to update %PATH%) but probably best to restart your whole computer

Only after the C++ buildtools, not yet again for the win10 sdk, will do it now

Miniontoby commented 1 year ago

Rebooted my laptop and it does still not work...

recluse4615 commented 1 year ago

hmm, you might want to try downloading visual studio 2022? https://visualstudio.microsoft.com/vs/

looks like it's noted there at the bottom of the error, maybe i didn't have to do the step bc i already have visual studio installed 🤔

the "community" edition should be fine

Miniontoby commented 1 year ago

hmm, you might want to try downloading visual studio 2022? https://visualstudio.microsoft.com/vs/

looks like it's noted there at the bottom of the error, maybe i didn't have to do the step bc i already have visual studio installed 🤔

the "community" edition should be fine

I am using VS 2022 already!

recluse4615 commented 1 year ago

i'm not too sure what to suggest at this point, googling the error just suggests you need to install the c++ build tools.. i would suggest maybe going through the tauri guide again, and double-y make sure you have everything installed exactly as they suggest, maybe you missed one of the "optional" addons? https://tauri.app/v1/guides/getting-started/prerequisites/#setting-up-windows

Miniontoby commented 1 year ago

Also, may I know how to edit those .svelte files the best way? (as in which IDE/editor do you recommend)

recluse4615 commented 1 year ago

to edit the project you can use whatever you want, svelte files are just standard html+js files although it does have some of its own conventions and quirks

for vscode: https://marketplace.visualstudio.com/items?itemName=svelte.svelte-vscode for idea: https://plugins.jetbrains.com/plugin/12375-svelte

Miniontoby commented 1 year ago

Just opened the Developer Command Prompt, and now I can do link.exe, still not in the main PowerShell from where I run npm run dev

I can try using C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.33.31629\bin\Hostx86\x86\link.exe

recluse4615 commented 1 year ago

maybe you need to add C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.33.31629\bin\Hostx86\x86 to your %PATH%?

Miniontoby commented 1 year ago

maybe you need to add C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.33.31629\bin\Hostx86\x86 to your %PATH%?

Thats what I thought as well, done it and it works

Miniontoby commented 1 year ago

Oke quick question:

I saw in src/lib a file called screenshots.js, but I think it would make more sense to have that inside the src/lib/api folder.

Was the file for a specific reason in that folder or not?

recluse4615 commented 1 year ago

the api folder is supposed to differentiate between "integrations" and "libraries" if that makes sense, which is why the majority of the contents of the api folder are strictly to do fetches and parse data from external sources

it gets a bit muddy, because some of them like versions.js also include some functionality around local files, but was the best differentiator in my eyes.

i'd prefer to keep it under src/lib/, or potentially even make an src/lib/file/ directory for it, but that's mostly semantics tbh so i'm not massively fussed either way - some of the code needs a bit of cleaning up anyways as it went pretty quick from "v quick prototyping" to "hey, here's the source and dev builds!"

Miniontoby commented 1 year ago

Oke will just put it in the src/lib

Also I now get this error and the Minetest Launcher is fully white:

19:18:22 [vite] Internal server error: Failed to resolve import "C:/Users/Miniontoby/Documents/Programma" from ".svelte-kit\generated\root.svelte". Does the file exist?
  Plugin: vite:import-analysis
  File: C:/Users/Miniontoby/Documents/Programma's/mtlauncher/.svelte-kit/generated/root.svelte
  708|  }
  709|
  710|  import * as ___SVELTE_HMR_HOT_API from 'C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/svelte-hmr/runtime/hot-api-esm.js';import { adapter as ___SVELTE_HMR_HOT_API_PROXY_ADAPTER } from 'C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/svelte-hmr/runtime/proxy-adapter-dom.js';if (import.meta && import.meta.hot) { if (false) import.meta.hot.accept();; Root = ___SVELTE_HMR_HOT_API.applyHmr({ m: import.meta, id: "C:/Users/Miniontoby/Documents/Programma's/mtlauncher/.svelte-kit/generated/root.svelte", hotOptions: {"preserveLocalState":false,"noPreserveStateKey":["@hmr:reset","@!hmr"],"preserveAllLocalStateKey":"@hmr:keep-all","preserveLocalStateKey":"@hmr:keep","noReload":false,"optimistic":false,"acceptNamedExports":true,"acceptAccessors":true,"injectCss":false,"cssEjectDelay":100,"native":false,"importAdapterName":"___SVELTE_HMR_HOT_API_PROXY_ADAPTER","noOverlay":true,"allowLiveBinding":false}, Component: Root, ProxyAdapter: ___SVELTE_HMR_HOT_API_PROXY_ADAPTER, acceptable: true, preserveLocalState: false, emitCss: true, }); }
     |                                          ^
  711|  export default Root;
  712|
      at formatError (file:///C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/vite/dist/node/chunks/dep-a713b95d.js:40839:46)
      at TransformContext.error (file:///C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/vite/dist/node/chunks/dep-a713b95d.js:40835:19)
      at normalizeUrl (file:///C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/vite/dist/node/chunks/dep-a713b95d.js:37571:33)
      at async TransformContext.transform (file:///C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/vite/dist/node/chunks/dep-a713b95d.js:37705:47)
      at async Object.transform (file:///C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/vite/dist/node/chunks/dep-a713b95d.js:41088:30)
      at async loadAndTransform (file:///C:/Users/Miniontoby/Documents/Programma's/mtlauncher/node_modules/vite/dist/node/chunks/dep-a713b95d.js:37349:29) (x2)
recluse4615 commented 1 year ago

lol, looks like there's an escaping issue within sveltekit

move the mtlauncher to a folder that doesn't contain a ' and it should work just fine

Miniontoby commented 1 year ago

the getInstalledContent inside the src/lib/api/contentdb can be used to list all the screenshots, but yeah it would need to be moved into a different file, cause it doesn't have anything to do with contentdb, but will use it for now

Oh wait I already made that code myself

Miniontoby commented 1 year ago

Not allowed to load local resource: file:///C:/Users/Miniontoby/AppData/Roaming/world.minetest.mtlauncher/versions/5.6.1/screenshots/screenshot_20220927_124848.png What to do with?

recluse4615 commented 1 year ago

how are you trying to load the file?

you might need to utilise this, annoyingly: https://tauri.app/v1/api/js/tauri/#convertfilesrc

tauri has some pretty strict requirements for security, so will need some changes to src-tauri/tauri.conf.json, you probably need to add something like:

"protocol": {
    "asset": true,
    "assetScope": [
        "$APP/versions/*"
    ]
}

before this line: https://github.com/minetest-world/mtlauncher/blob/main/src-tauri/tauri.conf.json#L28

recluse4615 commented 1 year ago

it may be beneficial to also create something like a LocalImage component, eg under $lib/components/image/LocalImage.svelte which would probably look like...

<script>
import { convertFileSrc } from '@tauri-apps/api/tauri';

export let src;
</script>

<img src={convertFileSrc(src)} />

and then you can call it in the screenshots page like...

import LocalImage from '$lib/components/image/LocalImage.svelte';
</script>
// ...
{#each screenshots as screenshot}
    <LocalImage src={screenshot.path} />
{/each}

(depending on implementation, just a rough guideline)

sorry - didn't realise that it would be this intense! lmao

Miniontoby commented 1 year ago

Now giving me this: GET https://asset.localhost/C%3A%5CUsers%5CMiniontoby%5CAppData%5CRoaming%5Cworld.minetest.mtlauncher%5Cversions%2F5.6.1%2Fscreenshots%5Cscreenshot_20220927_124848.png net::ERR_CONNECTION_REFUSED

I do have the protocol thing added

recluse4615 commented 1 year ago

try setting the csp to this: "default-src 'self'; font-src 'self' https://fonts.googleapis.com; img-src 'self' asset: https://asset.localhost https://blog.minetest.net https://content.minetest.net"

on this line: https://github.com/minetest-world/mtlauncher/blob/main/src-tauri/tauri.conf.json#L91

Miniontoby commented 1 year ago

Still it says GET https://asset.localhost/C%3A%5CUsers%5CMiniontoby%5CAppData%5CRoaming%5Cworld.minetest.mtlauncher%5Cversions%2F5.6.1%2Fscreenshots%5Cscreenshot_20220927_124848.png net::ERR_CONNECTION_REFUSED

And yeah I have also read it:

Convert a device file path to an URL that can be loaded by the webview. Note that asset: and https://asset.localhost must be allowed on the csp value configured on tauri.conf.json > tauri > security. Example CSP value: "csp": "default-src 'self'; img-src 'self' asset: https://asset.localhost" to use the asset protocol on image sources.

Additionally, the asset must be allowlisted under tauri.conf.json > tauri > allowlist > protocol, and its access scope must be defined on the assetScope array on the same protocol object.

Even when I got to it on my browser it just doesn't work

But yeah for now I am going to stop working on it, will do it tomorow, btw I also added like the keypress event for the password box for the Enter key to then login instead of having to click the Login button

recluse4615 commented 1 year ago

let me try it myself locally, will report back here - sorry for the issues here lol, i'm still learning tauri too 😅

recluse4615 commented 1 year ago

it seems to work for me! image

using some test code just like this:

<script>
    import { convertFileSrc } from '@tauri-apps/api/tauri';
    import { appDir } from "@tauri-apps/api/path";

    import { onMount } from 'svelte';

    let dir;
    onMount(async () => {
        dir = await appDir();
    })
</script>
{#if dir}
<img src={convertFileSrc(`${dir}/versions/5.6.0/screenshots/screenshot_20220902_160209.png`)} />
{/if}

is it possible that you already have DNS resolution for asset.localhost or something?

recluse4615 commented 1 year ago

(this is with the CSP & protocol changes i mentioned above earlier)

Miniontoby commented 1 year ago

I dont have any subdomain for asset.localhost configured at all, I think

But that commit might help "protocol-asset" wasnt in that file

UPDATE: thanks to that it now works!

Miniontoby commented 1 year ago

Added thanks to https://github.com/minetest-world/mtlauncher/pull/20

Miniontoby commented 1 year ago

Btw if there are any other stuff/features you need, let me know, cause I love to help out with this cool new project.

And because this now mainly focusses on servers (as far as I have seen) you could add a csm install/update/enable/disable menu thing, but then still @rubenwardy needs to add csm upload to contentdb or we need to come up with a way

recluse4615 commented 1 year ago

thanks so much for your contribution!

there's one note i added which is that i think the screenshots page should grab all screenshots from all installed versions, i don't think there's a need to make it all rely on your current version

outside of that, everything looks good!

csm support is being tracked under #11 but since there's no official csm support in contentdb yet (since it's still experimental, iirc) i wouldn't expect to see much in the immediate future we could create our own cdb just for csm in the meantime, but imo that's a bit overkill and i don't particularly want to be a single point of failure for that, lol

the launcher isn't going to be so focused on servers, but moreso supposed to be a "central hub" for everything to reduce the barrier of entry, but obviously a big selling point for something like minetest is its free and interoperable(*) multiplayer, and i think that it makes sense to put some legwork in to making that experience better (re: saved passwords, per-server identities, and comprehensive ways to see, filter, and query for available servers)

the biggest thing outstanding for that, at the moment, ironically is just gutting the current ServerListTable.svelte file to make it more lightweight.. i'd like to be able to include it in multiple locations (eg: game pages, to show "these servers play on this gameid!", or mod pages for the same reason, or even on a "home page" where it shows your favourite servers etc) but it's just far too bulky for that. this is tracked in #3

outside of that, if you feel like it - you're happy to pick any of the existing issues and take a stab at them! #12 is already done, and #4 is a wip on my end on the feature/settings branch, but you're free to jump in to anything that you like the sound of ^^

Miniontoby commented 1 year ago

there's one note i added which is that i think the screenshots page should grab all screenshots from all installed versions, i don't think there's a need to make it all rely on your current version

I can make it so you have the selected version and then below there one with all screenshots. Will add that into the pull request and then you can merge it

Miniontoby commented 1 year ago

Added it to the PR now, You can merge it if you want

Miniontoby commented 1 year ago

It would be nice if you join my IRC, hostname: irc.ircforever.org, ports: 6667 [No-ssl] and 6697 [SSL]. #minetest Cause then we can discuss without having to use github issue...

recluse4615 commented 1 year ago

thanks again for your contribution @Miniontoby ! i'll play around with it and make a few changes that were partially related (eg: click a screenshot to pop up a "higher resolution" version)

if you need to reach me, you can do so via the minetest matrix (i'm recluse4615 there) or via discord (rec#7338)