ThiaudioTT / hoi4-presence

Hearts of Iron IV presence for Discord!
MIT License
13 stars 10 forks source link

[FEATURE] Implement auto-update #11

Closed ThiaudioTT closed 10 months ago

ThiaudioTT commented 1 year ago

Discuss and implement how to automatically update the script.

Wolfmyths commented 11 months ago

This is an issue I am investigating for my projects as well.

I did find this in the github API docs: https://docs.github.com/en/rest/releases/assets?apiVersion=2022-11-28

And there is a tutorial on downloading files using the requests module I found here https://www.tutorialspoint.com/downloading-files-from-web-using-python

Edit:

I did more research and made up my own auto downloader for my project, here's what it looks like.

import requests

def downloadUpdate():

    # Get latest release data
    latestRelease = requests.get('https://api.github.com/repos/Wolfmyths/Myth-Mod-Manager/releases/latest')

    # Find asset data api link
    assetUrl = latestRelease.json()['assets_url']

    # Get asset data
    assetsData: dict = requests.get(assetUrl , headers={'accept':'application/vnd.github+json'}).json()

    # Incase there are muiltiple assets create a for loop
    downloadLink: str = None
    fileName = 'Myth-Mod-Manager.zip'

    for asset in assetsData:

        if asset['name'] == fileName:

            # Found the download link
            downloadLink = asset['browser_download_url']

            break

    if downloadLink:

        # Instance a session
        session = requests.Session()

        # Creating a new file in 'write in binary' mode
        with open(fileName, 'wb') as f:

            # Get data for download
            request = session.get(downloadLink, allow_redirects=True, stream=True)

            # Write it to fileName in chunks
            for chunk in request.iter_content(1024**2):
                f.write(chunk)

            print('Done!')

downloadUpdate()
ThiaudioTT commented 11 months ago

Nice, I was thinking something like it. And I think this is the best way to do it.

We could download it on a temporary folder on Windows like %temp% (That's a good feature to have in your project btw) and execute it.

Wolfmyths commented 11 months ago

(That's a good feature to have in your project btw)

My script already cleans up unneeded files dependent on the outcome (Cancel, fail, success). So I'm not sure if that is needed, are there other advantages to using %temp% than just putting temp files in there?

ThiaudioTT commented 11 months ago

are there other advantages to using %temp% than just putting temp files in there?

If you already clean the mess, I think isn't necessary then. The only case when %temp% could be useful is when the script fails, for some reason like energy crash or permissions, when cleaning the mess. Using %temp% we delegate to the OS to clean up everything.

Wolfmyths commented 11 months ago

Better safe than sorry, I'll probably end up managing downloads into %temp% for my project then. Thanks for the suggestion.

If you want me to implement auto installation for hoi4 presence I'd be glad to.

Also another thing to consider, are we going to update hoi4 presence without their permission? Imo I think without would probably work out better? More streamlined and rich presence is in the background anyways.

But if not, how'd we prompt it?

ThiaudioTT commented 11 months ago

If you want me to implement auto installation for hoi4 presence I'd be glad to.

No problem, I will assign you then.

Also another thing to consider, are we going to update hoi4 presence without their permission?

Yeah, we will go in this way. Update the presence without their permission.

Wolfmyths commented 11 months ago

So I designed the auto updater, haven't tested it yet but here's how it works: https://github.com/ThiaudioTT/hoi4-presence/commit/4d4ff4996cdb3d2ceda2e7d57c93b0ed0fc760a8

I do have a couple concerns atm:

And one more thing I'll include in the next commit: If the user fails to download and tries again, the downloaded files in the temp folder will be deleted if they exist since shutil usually doesn't like overrwriting files

Edit:

I removed an item from the tuple requiredFiles in setup.exe and forgot to put it back, that'll be fixed in the next commit

ThiaudioTT commented 11 months ago

Updating .exe files can be tricky, as windows does not allow them to be deleted during their runtime. This concern is towards checkupdate.exe

We could just download the release executing the installer script. The installer can stop the scripts running (If that's the case) and install and execute the new ones. The Checkupdate's job could be only executing the installer and exiting if successful.

This is what it came from my mind. Feel free to think and choose the best solution. We can always refactor later.

Wolfmyths commented 11 months ago

We could just download the release executing the installer script.

I changed the code in my latest commit so it uses os.openFile() instead of subproccess.call() which basically is the equivalent of opening a file as if you were to double-click it in file explorer according to the os module docs.

So now checkUpdate.py opens the newly downloaded installer and closes, you can still pass args with os.openFile() so at the end of the setup.py's installation with the correct arg, it will os.openFile() runRPC.bat and close like normal.

However, I ran into an issue that has to do with the structure of the project and I'm unsure how to fix it as I've never had an error like this before. In checkupdate.py, I'm having a ModuleNotFoundError for a .py file that is in the same folder as checkupdate.py

Here is the link to the fork, with a project having multiple exes like this I'm a little stumped. There error is both in the script and the exe.

https://github.com/Wolfmyths/hoi4-presence/blob/test-branch/src/checkupdate/checkupdate.py

ThiaudioTT commented 11 months ago

However, I ran into an issue that has to do with the structure of the project and I'm unsure how to fix it as I've never had an error like this before. In checkupdate.py, I'm having a ModuleNotFoundError for a .py file that is in the same folder as checkupdate.py

Could you explain further and give some images? Doing this, I could help.

I cloned your fork and changing this line (It was giving ModuleNotFoundError) I was able to execute it.

There are some ways of importing python scripts to another, I changed to:

from downloadupdate import downloadUpdate

The script executed normally until this line:

os.startfile(installerPath, "-update")

I don't know why, but the script couldn't start the setup.exe. (I can execute setup.exe normally if I open using the explorer)

The error:

os.startfile(installerPath, "-update")
OSError: [WinError 1155] No application is associated with the specified file for this operation: 'C:\\Users\\USER\\AppData\\Local\\Temp\\hoi4-presence-v1.1.0\\setup.exe'

Doing a search on the internet, I found that is due to the OS who couldn't find how to open the file. But I don't know if this is the case.

Wolfmyths commented 11 months ago

There are some ways of importing python scripts to another, I changed to:

Ah yeah my bad, nowadays I usually use python to develop software so I was used to having a main.py and the cwd being where the main.py path.

I don't know why, but the script couldn't start the setup.exe. (I can execute setup.exe normally if I open using the explorer)

I had to replace os.startFile() with subproccess.Popen() which is more suited for our needs it seems. I was able to launch the new setup.exe with the correct argument -update in command prompt, this isn't something I can fully test unless we have somewhere to download this experimental version from. Because if we run setup.exe it will download and run the latest release's setup.exe. I just have to make sure subproccess.Popen() is passing the argument correctly and we should be good to go.

Wolfmyths commented 11 months ago

I made a new commit to my fork and I'm almost ready to send a PR but I have a question on an issue I found with automation.

If the user doesn't have their documents or game folder in the default directory where setup.exe goes to look first, the user will have to manually input their game/documents directory.

I found a way to fix this by adding more arguments to setup.exe and I pass this value in checkUpdate.exe to determine the documents folder directory.

documentsFolder = os.path.dirname(os.getcwd())

But an issue rises for the game's directory unless I'm missing something. Theres no way to really find the game's path in checkupdate.exe's scope.

Maybe setup.exe can save values for checkupdate.exe in a json? I was wondering what you'd prefer if you even want this kind of automation for the updater.

When checkupdate.exe opens setup.exe the call should look like this and shouldn't run into any "can't find path" errors:

subprocess.Popen([installerPath, "-update", documentsFolder, gameFolder], start_new_session=True)
ThiaudioTT commented 11 months ago

Maybe setup.exe can save values for checkupdate.exe in a json? I was wondering what you'd prefer if you even want this kind of automation for the updater.

I was thinking the same when I created this issue. But I don't know if this is a Feature Creep. Maybe the best thing to do is to implement the checkupdate and let the user prompt the path for now. Later we create another issue to add this feature (The update saving a JSON file).