Naprosnia / RetroPie_BGM_Player

A simple background music player to implement on RetroPie and Emulation Station.
37 stars 17 forks source link

cleanup code #2 #2

Closed crcerror closed 5 years ago

crcerror commented 5 years ago
    while true; do
        choice=$(dialog --backtitle "$BACKTITLE" --title " Set Background Music Volume " \
            --ok-label OK --cancel-label Back \
            --menu "Choose the volume." 25 75 20 \
            1 "Volume 100%" \
            2 "Volume 90%" \
.....

Better use statements like this

    while true; do
        choice=$(dialog --backtitle "$BACKTITLE" --title " Set Background Music Volume " \
            --ok-label "OK" --cancel-label "Back" --no-tags \
            --menu "Choose the volume." 25 75 20 \
            32768 "Volume 100%" \
            29491 "Volume 90%" \
            .....

Use --no-tags switch to let tag items vanish and use as "answer" then you do not need all the function calls.

and the "result" is just one codeblock

# the rm is not needed
echo "bgmvolume=$choice" > /home/pi/.bgmvolume
sudo pkill mpg123
mpg123 -q -f $choice -Z ~/RetroPie/roms/music/*.mp3 &
Naprosnia commented 5 years ago

Hello crcerror, yesterday I did not have free time to work on this. Maybe today i can finish this. Thanks for your help, im trying now what you mention above, and it works great, i noticed that i need to put something like [ "$choice" == "" ] && exit, so the "Back" button can exit.

Code for this file, now look like this:

BGM=$HOME"/RetroPie-BGM-Player"
BGMCONTROL=$BGM"/bgm_control"
BGMSETTINGS=$BGM"/bgm_settings.cfg"

#get current volume setting
source $BGMSETTINGS >/dev/null 2>&1

function main_menu() {
    local choice

    while true; do
        choice=$(dialog --backtitle "$BACKTITLE" --title " BGM Volume Settings " \
            --ok-label "OK" --cancel-label "Back" --no-tags \
            --menu "Set volume level" 25 75 20 \
            32768 "Volume 100%" \
            29484 "Volume 90%" \
            26208 "Volume 80%" \
            22932 "Volume 70%" \
            19656 "Volume 60%" \
            16380 "Volume 50%" \
            13104 "Volume 40%" \
            9828 "Volume 30%" \
            6552 "Volume 20%" \
            3276 "Volume 10%" \
            2>&1 > /dev/tty)

            [ "$choice" == "" ] && exit
        bash $BGM/bgm_system.sh -setsetting bgm_volume $choice
        bash $BGM/bgm_system.sh -r

    done
}

main_menu

Much smaller now. I hope finish this today, to put it here on git.

Again, thanks for your hints.

PS. this is the first time i work on bash, im a windows guy and i program on php, mysql and other web based languages. This is why you see me using lots of sudos, and use simple coding technics. But now, i think this version will be better coded.

Naprosnia commented 5 years ago

You know how to remove the red color from "V"olume ? I can't find information about it.

crcerror commented 5 years ago

About the color... No I've no tag at hand now. It will work I know. You can check if you type man dialog into shell command.

About the [ "$choice" == "" ] && exit ... it's a dirty hack do not use this. If you try to compare values against emtpy ones then use the testcommand. The testcommand [[ ... ]] or you use it [ ... ] is very powerfull. If you check for an empty value you better use [[ -z $choice ]] statement.

For the improved coding try to use this constellation to build dialogs

while true; do
cmd=(dialog --title " TEST " \
            --ok-label " Select " \
            --cancel-label " Exit to ES " \
            --no-tags --stdout \
            --menu "This is a menu" 0 0 0)
volume=("32768" "Volume 100%" "29484" "Volume 90%" "26208" "Volume 80%")
choices=$("${cmd[@]}" "${volume[@]}")

# Now the trick with the return code
buttoncode=$?

case $buttoncode in
    0) #OKAY BUTTON 
        echo "OK"
    ;;
    1) # Cancel BUTTON
        echo "CANCEL
    ;;
esac
done