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

Add Non-Steam Game: Add feedback when exe is invalid #959

Merged
merged 5 commits into from
Oct 25, 2023

Conversation

sonic2kk
Copy link
Owner

Adds logging, notifier, and terminal output when the executable path for a Non-Steam Game is blank or invalid.

To do this we made a couple of changes:

  1. Since we quote the EXE path in the case statement checking the incoming flags, we get the unquoted string using sed to remove double quotes from the beginning and end of the string. We do the -z and -f check on that. This is not strictly required for the -z check, since we don't quote unless the incoming EXE is -n, but we do need to do this to correctly check for a valid EXE path. Since we quote the string if it's defined. I chose to do this as a stylistic choice over having an -f check in the case block.
  2. Refactor a little to have these checks return 1, which means they'll exit the function and it means we can remove the -n check for NOSTEXEPATH. This lets us have a cleaner and less indented function. It's been bugging me for a while and I finally got around to fixing it. We exit earlier instead of having all of our Add Non-Steam Game logic wrapped in an if block, it reads nicer imo.

When running steamtinkerlaunch ansg without --appname, the GUI will always open, however the GUI can pass a blank EXE. So our presence check will fire there even if it doesn't fire on the commandline. The path check will work for both the GUI and commandline though.


I tested this on the GUI and commandline, and the checks seem to fire correctly. The refactor to remove everything being wrapped in the -n "$NOSTEXEPATH" check also doesn't seem to have broken anything. Adding Non-Steam Games works as expected and I confirmed the function shops if the EXE is invalid or not set by checking both the logs and whether shortcuts.vdf was modified. The SteamGridDB stuff all still works good.

But a better, fuller test is probably a good idea.


TODO:

  • More testing
  • Update langfiles

@sonic2kk
Copy link
Owner Author

sonic2kk commented Oct 25, 2023

This worked in testing and should be good enough, don't want to leave this in PR hell any longer :-) Waiting on Shellcheck to give me the go-ahead and then I'll merge (version was already bumped in the latest merge commit 8361c7d)

@sonic2kk sonic2kk merged commit 1204c3b into master Oct 25, 2023
@sonic2kk sonic2kk deleted the ansg-notifier-logging-echos branch October 25, 2023 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant