hiulit / RetroPie-Fun-Facts-Splashscreens

A tool for RetroPie to generate splashscreens with random video game related Fun Facts!.
MIT License
4 stars 3 forks source link

3rd revision #4

Closed meleu closed 7 years ago

meleu commented 7 years ago

I made quick tests, but maybe it's better you test it too, before merging. ;-)

TODO: make more error checking.

hiulit commented 7 years ago

Things I think are not working

Questions

Comments

Added

meleu commented 7 years ago
  • When I set a color using --text-color red and they try to create a fun fact using --create-fun-fact it doesn't get the color.

I tested it before submitting the PR and it worked fine. But to be honest I didn't test on the actual Raspberry Pi, my dev machine is a x86 Debian Linux.

Can you elaborate how exactly it's not working on your end?

  • What does "$@" do (or mean) in main "$@" and get_options "@$"?

It means "all positional parameters where each parameter is a quoted string seen as a single word". More deteailed answer in that doc I linked to you in the forums: http://tldp.org/LDP/abs/html/internalvariables.html#POSPARAMREF

  • What does >&2 do (or mean) at the end of echo?

Redirect the output to the standard error. A quick reading on this subject: http://tldp.org/HOWTO/Bash-Prog-Intro-HOWTO-3.html

  • What is the difference between exit 0 and exit 1? Same for return 0 and return 1.

The exit command interrupts the execution of a shell, and the number after it is the value returned to the parent process (the process which called that shell). Usually we use 0 for normal exit (everything ran fine) and a non-zero value (such as 1) to indicate that we found a problem during the execution of the program.

The return command works similarly, but it's related to functions.

More info here: http://www.tldp.org/LDP/abs/html/exit-status.html

  • The 2 new lines between functions is like a standard thing or just 'your thing'? (just curious).

My thing. 😅

It's just to add some readability to the code.

  • Added some colors :P (Red for errors, green for successful messages).

To be honest I don't like these colors on output. It usually messes when we redirect/pipe the output to another file/command (e.g. less and more commands). But, again, it's my thing. :)

  • Added echo "$($0 --help)" >&2 to show the help in check_argument.

Maybe it's better to say echo "Try \"$0 --help\" for more info". If the script evolve and the help go big, the user won't see the first error message.

  • Created a config file fun-facts-settings.cfg.

I wouldn't go with this approach for such a simple task. And even if I went, I surely wouldn't use a xml file.

Maybe it's time to start working with the scriptmodule for RetroPie-Extra... 😉

EDIT: on a second thought maybe using a config file can be useful to let the user change configs without editing the /etc/rc.local directly. But I would use a single text file with key = value per line. Example:

splash = "/path/to/splashscreen.png"
text-color = white
hiulit commented 7 years ago

Thanks you very, very much for all your help! I really appreciate it! 🎉 🎉

Added

Changes

I think we are getting there.

Let me know what you think about these changes and if you are ok with them, let's start working on the scriptmodule? 😃 (I have no clue about where to start)

meleu commented 7 years ago

You're doing pretty good bro!

I'm really glad I can inspire other people to code! 😄

hiulit commented 7 years ago

Thanks! 🎉 It means a lot coming from you.