Don't exit if a mod isn't found, keep checking the rest of the list

[mtfurlan]

When the game updates and I need to update all the mods, it's annoying to have to take my list of mods and remove successes or fails one by one till I have a list of the mods that have updated and the ones that haven't.

It's much nicer if shaber just tells me that some failed, and the rest updated or whatever.

I tested this against mod update and mod download, it seems to be fine for both of those when passed a mixed list of updated and non-updated mods. If there is any other testing you want done let me know.

Also it's real nice to work on bash scripts from someone else that already pass shellcheck, thank you.

[ominitay]

Thank you for this PR! I'll review this tomorrow, and make any changes needed.

[ominitay]

I should mention I'm implementing these changes on my local machine, and will push them to your fork when I'm happy that the code is fully consistent and works reliably.

[ominitay]

@mtfurlan I've committed my changes. You may check over them and give me any thoughts you have, then we can get this merged. If you can though, please do try and test out the functionality as much as you can to ensure that the changes behave correctly :)

[mtfurlan]

It returns 1 on every non-help operation, even when successful.

Seems to be caused by the way you do your final error check, this fixes it:

diff --git a/shaber b/shaber
index 738f8a8..eac1709 100755
--- a/shaber
+++ b/shaber
@@ -920,10 +920,10 @@ parse_helper_mod_download() {
       mod_download_tree "$arg" || main_error=1
     fi
   done
-  [ $main_error -eq 1 ] && {
+  if [ $main_error -eq 1 ]; then
     log_warn "Errors occurred while downloading mods! Please check your logs"
     return 1
-  }
+  fi
 }

 parse_helper_mod_update() {

Also I think it's more readable that way.

I played with mod update, mod enable, mod disable, mod checkupdate, mod download, and they all work as I expect aside from the return code.

I didn't look very in depth at your changes, but the bits I did look at looked like good changes. If you want me to spend more time and look at everything really carefully, I can do that.

[ominitay]

Ah I didn't notice that. Will amend that commit tomorrow. I think I'll hold off merging for a bit just to be sure that I've not introduced any regressions. If you'd like to test it out a bit more, that'd be great, but you don't have to at all :)

[ominitay]

Apologies on taking so long on this, life is fairly busy at the moment. I'll aim to get this merged soon though, and that'll go towards your hacktoberfest if you are participating in that :)

[mtfurlan]

No worries, I get how life happens. I did find a bug in this though that isn't present on dev:

1mark@mephala:~/beatSaber/ShellSaber (fix/no-early-fail-if-missing *=)$ ./shaber mod download PlaylistManager                                           
Info: Searching for dependencies for 'PlaylistManager'                                                                                                  
Warn: Mod 'PlaylistManager' is already downloaded. Run 'shaber mod update PlaylistManager' to update it.                                                
mark@mephala:~/beatSaber/ShellSaber (fix/no-early-fail-if-missing *=)$ ./shaber mod update PlaylistManager                                              
Info: Mod 'PlaylistManager' is up-to-date                                                                                                               
1mark@mephala:~/beatSaber/ShellSaber (fix/no-early-fail-if-missing *=)$ ./shaber mod enable PlaylistManager                                             
Error: Dependency 'BeatSaberPlaylistsLib' not downloaded                                                                                                
Error: Failed to get dependency tree for mod 'PlaylistManager'                                                                                          
Error: Failed to enable mod 'PlaylistManager'                                                                                                           
Warn: Errors occured while enabling mods! Please check your logs

Let me know if you can't easily replicate it and I'll debug further.

[ominitay]

Yep I can reproduce that too. ShellSaber seemingly isn't downloading the dependencies for the mod. So something bad is clearly happening between creating the dependency list and downloading them. I'll try and debug that later.

[ominitay]

Fixed it, will push an amendment now.

[ominitay]

Made a few final changes, and have finally stopped it from erroring out after running. LGTM

[ominitay]

Nearly forgot to rebase on dev, oops!

[ominitay]

These changes will be included in the next release, which I hope to put out halfway through next week, after I make another couple of changes.

[mtfurlan]

Thank you <3

[ominitay]

Thank you so much for your contributions :)