search

2bc4 / streamlink-ttvlol

Streamlink Twitch plugin modified to work with the TTV.LOL API
BSD 2-Clause "Simplified" License
158 stars 6 forks source link

batch install #15

Closed Sokoloft closed 1 year ago

Sokoloft commented 1 year ago

slightly edited the readme. Might want to change that up if you want. As well as ensure to zip the release in a way that. The user extracts a folder with the two files inside of it. The batch is looking for the twitch.py to be inside its working directory.

2bc4 commented 1 year ago

What do you think about renaming the batch file to just install.bat? It would look cleaner I think.

As well as ensure to zip the release in a way that. The user extracts a folder with the two files inside of it.

I'm confused by this. I don't think there's a way to change the automatically generated zip by github, are you suggesting that I make a new zip?

Sokoloft commented 1 year ago

Okay. Batch name was changed with commit b9a74d75beb5559e223bd31e144161f6e1a2e773.

As well as ensure to zip the release in a way that. The user extracts a folder with the two files inside of it.

I'm confused by this. I don't think there's a way to change the automatically generated zip by github, are you suggesting that I make a new zip?

Yes. If you clone the repository once this PR is merged. You will have the "streamlink-ttvlol" folder. Zip that folder. So that when the user opens the zip. They can drop the folder onto their desktop. The batch file will be in the folder along with the twitch.py. Which needs to be the case since the batch is looking for the python file in its working directory. streamlink-ttvlol.zip is the zip of my current fork.

You should be able to manually upload a release file iirc. I did that with my streamcounter project.

2bc4 commented 1 year ago

OK perfect I understand that now.

However I found an issue. If you run the streamlink windows installer as Admin and install it for all users it won't create the %AppData%\streamlink folder, since the folder isn't guaranteed to exist the batch script will fail. Perhaps the script should create the folder if it doesn't exist.

I also found that if install.bat is on a drive other than C: it will fail to find AppData, not sure if this is easily fixable as the problem seems to be cd %AppData% not working.

Sokoloft commented 1 year ago

OK perfect I understand that now.

However I found an issue. If you run the streamlink windows installer as Admin and install it for all users it won't create the %AppData%\streamlink folder, since the folder isn't guaranteed to exist the batch script will fail. Perhaps the script should create the folder if it doesn't exist.

I also found that if install.bat is on a drive other than C: it will fail to find AppData, not sure if this is easily fixable as the problem seems to be cd %AppData% not working.

Okay. I'll ensure the script cd's to C since thats the only drive letter windows can be on iirc. It already checks for the streamlink dir. I knew that might be an issue. However in the else I can have it make the streamlink directory and then goto a loop and restart.

Sokoloft commented 1 year ago

Okay. Check commit dd11dcf920dcdbc5b23410885216f7000f0d9bc8.

I ran it on my E: drive. Installed just fine. However I did not test running it as admin. I don't see that being an issue. It now checks if the streamlink dir exists. If it doesn't, it'll make it. The user will then need to resume the script. Since the streamlink directory wasn't there. That indicates a problem so they should be aware of that. Idk how to word it in echo. I thought maybe saying like. Is streamlink installed? Since streamlink creates that appdata dir. 🤷

Sokoloft commented 1 year ago

oops I just realized I did not change the window title to install.bat. Lmk before you merge to master and I'll fix that beforehand. I could just remove the _install from that line as well. So its just the repo name. That makes sense to me.

2bc4 commented 1 year ago

I could just remove the _install from that line as well. So its just the repo name.

This sounds good. After this I'll change the formatting in the README slightly and then merge.

Sokoloft commented 1 year ago

Alright. All set 👍