Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue on mod download failure #756

Open
Alystrasz opened this issue Jul 28, 2024 · 1 comment · May be fixed by #817
Open

Issue on mod download failure #756

Alystrasz opened this issue Jul 28, 2024 · 1 comment · May be fixed by #817
Labels
MAD Related to mod-auto-download

Comments

@Alystrasz
Copy link
Contributor

(Initially posted by @wolf109909 in Discord)

I think I've found some issues that might occur when mad failed to finish any of the install states in current main branches. Heres the walkthrough:
I've done a few temporary fixes to these problems while debugging myself, here is the first part:

  1. user double click on the server with a downloadable mod
  2. mod auto download kicks in, but failed when downloading due to poor internet
  3. user did 1 again, instead of retrying the download, user got sent into the server and disconnected with out of sync with server.
Debugging

In moddownloader.cpp at void ModDownloader::DownloadMod(std::string modName, std::string modVersion), the ScopeGuard cleanup will get called even when you early return in any of the failing conditions which sets the modState.state. this will cause modState.state to always be set to DONE, which makes the UI think the mod is downloaded (detailed info on the second part). I just removed the scope guard and moved to the old code reusing style for temoporary fix.

After that, I tried again. I get these instead:

  1. user double click on the server with a downloadable mod
  2. mod auto download kicks in, but failed when downloading due to poor internet
  3. user gets a download failed warning, and got sent back to the server browser.
  4. user did 1 again, but this time got to 3 immediately.
  5. user retried again, this time he got to 2 but the download progress bar glitches.
  6. things broke

From my debugging, I found 3 is caused by line 39 and line 41 in menu_ns_moddownload.nut where the NSGetModInstallState() call is right next to NSDownloadMod( mod.name, mod.version ), when in CPP the void ModDownloader::DownloadMod(std::string modName, std::string modVersion) creates a requestThread and making the NSDownloadMod in squirrel operation not blocking, giving NSGetModInstallState() with the old g_pModDownloader->modState.state. Immediately the UI will freak out and send user a Download Failed warning BUT in the mean time ModDownloader::DownloadMod is still running and didn't get canceled. Once the user triggers 1 again, the NSGetModInstallState() will give the correct state but its set from the previous NSDownloadMod call. there we got some kind of race condition between two downloader threads that does filesystem opertaions and computer not very happy(its not that bad tbh just glitchy).

Another tempfix

I added simple thread block like the ones used in masterserver.cpp to limit only one concurrent threads at the same time. then I added a new state on top of modState.state, naming it IDLE. I set the state to IDLE before spawning requestThread, making the script able to wait for the first modState.state and not just returning. At this stage, I think the whole process is more stable and user-friendly.
ps: I do this in discord instead of a github issue cause I'm not entirely sure if I'm doing this correctly. there are environmental differences between my testing branch and main branch. I will push the fixed code to my branch, but they are not very clean and are just temp fixes.

@wolf109909
Copy link
Contributor

Relevant code can be found here:
R2NorthstarCN@e01944f
R2NorthstarCN/NorthstarMods@dd23739
note: we are using a custom masterserver which doesn't have connectionless auth from atlas. this means we are still running very ancient server browser nut files and I decided to just change MAD related code instead of implementing the same server browser changes in masterserver.cpp

@Alystrasz Alystrasz linked a pull request Sep 7, 2024 that will close this issue
@Alystrasz Alystrasz added the MAD Related to mod-auto-download label Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MAD Related to mod-auto-download
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants