RetroPie forum home
    • Recent
    • Tags
    • Popular
    • Home
    • Docs
    • Register
    • Login
    Please do not post a support request without first reading and following the advice in https://retropie.org.uk/forum/topic/3/read-this-first

    Script module Q - vars outside of functions?

    Scheduled Pinned Locked Moved Help and Support
    script module
    6 Posts 2 Posters 399 Views
    Loading More Posts
    • Oldest to Newest
    • Newest to Oldest
    • Most Votes
    Reply
    • Reply as topic
    Log in to reply
    This topic has been deleted. Only users with topic management privileges can see it.
    • S
      sleve_mcdichael
      last edited by

      I am working on a script module and I find myself repeating a lot of the same vars over and over again in my different functions. Is it okay to define them once outside of any function and then call them from within?

      Example:

      rp_module_id="mymodule"
      rp_module_flags="!all rpi"
      dir1="path/to/dir1"
      file2="path/to/file2.ext"
      
      function install_mymodule() {
          mkUserDir "$dir1"
      }
      
      function configure_mymodule() {
          mv "$file2" "$dir1"
      }
      

      ...instead of:

      rp_module_id="mymodule"
      rp_module_flags="!all rpi"
      
      function install_mymodule() {
          local dir1="path/to/dir1"
          mkUserDir "$dir1"
      }
      
      function configure_mymodule() {
          local dir1="path/to/dir1"
          local file2="path/to/file2.ext"
          mv "$file2" "$dir1"
      }
      

      ...if yes, what do I do with the initially-undefined var used in "for" loops, i.e.:

      local file
      for file in "$file1" "file2"; do
      

      Would I,

      1: still declare a local file inside each function where I need to use it?

      2: do an empty file="" (or file=()?) outside the functions with the other vars?

      3: just wing it and do for file in without declaring it anywhere, first?

      BuZzB 1 Reply Last reply Reply Quote 0
      • BuZzB
        BuZz administrators @sleve_mcdichael
        last edited by

        @sleve_mcdichael for retropie modules we dont allow global vars apart from the module configuration ones.

        To help us help you - please make sure you read the sticky topics before posting - https://retropie.org.uk/forum/topic/3/read-this-first

        1 Reply Last reply Reply Quote 0
        • BuZzB
          BuZz administrators
          last edited by

          If you need to share paths between functions you can deduplicate this by having a function which outputs the path.

          To help us help you - please make sure you read the sticky topics before posting - https://retropie.org.uk/forum/topic/3/read-this-first

          S 1 Reply Last reply Reply Quote 0
          • S
            sleve_mcdichael @BuZz
            last edited by sleve_mcdichael

            @BuZz said in Script module Q - vars outside of functions?:

            If you need to share paths between functions you can deduplicate this by having a function which outputs the path.

            ...and then I'm calling that function from within the other function right? Something like:

            function _getpath1_mymodule() {
                echo "$path/to/file"
            }
            
            function _getpath2_mymodule() {
                echo "/path/to/$other/file"
            }
            
            function_a_mymodule() {
            path1="$(_getpath1_mymodule)"
            path2="$(_getpath2_mymodule)"
            }
            
            function_b_mymodule() {
            path1="$(_getpath1_mymodule)"
            path2="$(_getpath2_mymodule)"
            }
            

            ...this doesn't seem to save any lines, in fact it uses even more lines since I'm now defining the paths once in each function and once each in their own individual function. I guess the benefit of this method is, if I change the path later, I only have to change it in one place instead of in each function separately.

            Is that all roughly correct?

            BuZzB 1 Reply Last reply Reply Quote 0
            • BuZzB
              BuZz administrators @sleve_mcdichael
              last edited by

              @sleve_mcdichael yes. Have a look at existing scriptmodules as some use this already.

              To help us help you - please make sure you read the sticky topics before posting - https://retropie.org.uk/forum/topic/3/read-this-first

              S 1 Reply Last reply Reply Quote 0
              • S
                sleve_mcdichael @BuZz
                last edited by sleve_mcdichael

                @BuZz thanks, I see something like this in e.g. moonlight.sh.

                Do you have any thoughts or comments on the method I use here in function enable_ vs the method in disable_ and install_ (or, on anything else)? I did it this way in enable_ because I also need to process the files individually, while the other two do them all as a group in the "for" loop:

                #!/usr/bin/env bash
                
                # This file is part of The RetroPie Project
                #
                # The RetroPie Project is the legal property of its developers, whose names are
                # too numerous to list here. Please refer to the COPYRIGHT.md file distributed with this source.
                #
                # See the LICENSE.md file at the top-level directory of this distribution and
                # at https://raw.githubusercontent.com/RetroPie/RetroPie-Setup/master/LICENSE.md
                #
                
                rp_module_id="bgm123"
                rp_module_desc="Straightforward background music player using mpg123"
                rp_module_help="Place your MP3 files in $datadir/bgm"
                rp_module_section="exp"
                rp_module_flags="!all rpi"
                
                function _autostart_bgm123() {
                    echo "$configdir/all/autostart.sh"
                }
                
                function _bashrc_bgm123() {
                    echo "$home/.bashrc"
                }
                
                function _onstart_bgm123() {
                    echo "$configdir/all/runcommand-onstart.sh"
                }
                
                function _onend_bgm123() {
                    echo "$configdir/all/runcommand-onend.sh"
                }
                
                function depends_bgm123() {
                    getDepends mpg123
                }
                
                function install_bin_bgm123() {
                    local scriptname="bgm_vol_fade.sh"
                    local files=(
                        "$(_autostart_bgm123)"
                        "$(_bashrc_bgm123)"
                        "$(_onstart_bgm123)"
                        "$(_onend_bgm123)"
                    )
                    local file
                
                    cp "$md_data/$scriptname" "$md_inst"
                    chmod +x "$md_inst/$scriptname"
                
                    for file in "${files[@]}"; do
                        # preserve original file versions
                        if [[ -f "$file" && ! -f "$file.old.bgm123" ]]; then
                            cp "$file" "$file.old.bgm123"
                            chown $user:$user "$file.old.bgm123"
                        fi
                    done
                }
                
                function disable_bgm123() {
                    local files=(
                        "$(_autostart_bgm123)"
                        "$(_bashrc_bgm123)"
                        "$(_onstart_bgm123)"
                        "$(_onend_bgm123)"
                    )
                    local file
                
                    # kill player now since .bashrc won't do it later
                    vcgencmd force_audio hdmi 0 >/dev/null
                    pgrep mpg123 >/dev/null && pkill mpg123
                
                    for file in "${files[@]}"; do
                        if [[ -f "$file" ]]; then
                            # backup file and attempt to remove any existing bgm config
                            cp -f "$file" "$file.bak"
                            chown $user:$user "$file.bak"
                            sed -i '/#bgm/d' "$file"
                
                            # if file is now empty, remove it
                            [[ ! -s "$file" ]] && rm -f "$file"
                        fi
                    done
                }
                
                function remove_bgm123() {
                    disable_bgm123
                    remove_share_samba "bgm"
                    restart_samba
                }
                
                function enable_bgm123() {
                    local fadescript="$md_inst/bgm_vol_fade.sh"
                    local autostart="$(_autostart_bgm123)"
                    local bashrc="$(_bashrc_bgm123)"
                    local onstart="$(_onstart_bgm123)"
                    local onend="$(_onend_bgm123)"
                    local file
                
                    disable_bgm123
                
                    for file in "$autostart" "$bashrc" "$onstart" "$onend"; do
                        touch "$file"
                        chown $user:$user "$file"
                    done
                
                    echo -e "$(echo -e 'while pgrep omxplayer >/dev/null; do sleep 1; done #bgm123\n(vcgencmd force_audio hdmi 1 >/dev/null; sleep 8; mpg123 -Z "'$datadir'/bgm/"*.[mM][pP]3 >/dev/null 2>&1) & #bgm123'; cat $autostart)" > "$autostart"
                    echo -e '[[ "$(tty)" == "/dev/tty1" ]] && (vcgencmd force_audio hdmi 0 >/dev/null; pkill mpg123) #bgm123' >> "$bashrc"
                    echo -e '"'"$fadescript"'" -STOP & #bgm123' >> "$onstart"
                    echo -e '(sleep 1; "'"$fadescript"'" -CONT) & #bgm123' >> "$onend"
                }
                
                function configure_bgm123() {
                    [[ "$md_mode" == "remove" ]] && return
                
                    local share="$datadir/bgm"
                
                    mkUserDir "$share"
                    add_share_samba "bgm" "$share"
                    restart_samba
                
                    enable_bgm123
                }
                
                function play_pause_bgm123() {
                    if pgrep mpg123 >/dev/null; then
                        su $user -c "$md_inst/bgm_vol_fade.sh &"
                    else
                        su $user -c "(vcgencmd force_audio hdmi 1 >/dev/null; sleep 1; mpg123 -Z $datadir/bgm/*.[mM][pP]3 >/dev/null 2>&1) &"
                    fi
                }
                
                function next_track_bgm123() {
                    pgrep mpg123 >/dev/null && pkill mpg123
                    su $user -c "(vcgencmd force_audio hdmi 1 >/dev/null; sleep 1; mpg123 -Z $datadir/bgm/*.[mM][pP]3 >/dev/null 2>&1) &"
                }
                
                function gui_bgm123() {
                    local cmd=(dialog --backtitle "$__backtitle" --cancel-label "Back" --menu "Choose an option." 22 86 16)
                    while true; do
                        local enabled=0
                        grep '#bgm123' "$configdir/all/autostart.sh" >/dev/null && enabled=1
                        local options=()
                        if [[ "$enabled" -eq 1 ]]; then
                            if pgrep emulationstatio >/dev/null; then
                                options+=(
                                    P "Play / pause"
                                    N "Next track"
                                )
                            fi
                            options+=(
                                1 "Enable background music (currently: Enabled)"
                            )
                        else
                            options+=(
                                1 "Enable background music (currently: Disabled)"
                            )
                        fi
                        local choice=$("${cmd[@]}" "${options[@]}" 2>&1 >/dev/tty)
                        if [[ -n "$choice" ]]; then
                            case "$choice" in
                                P)
                                    play_pause_bgm123
                                    ;;
                                N)
                                    next_track_bgm123
                                    ;;
                                1)
                                    if [[ "$enabled" -eq 1 ]]; then
                                        disable_bgm123
                                        printMsgs "dialog" "Background music disabled."
                                    else
                                        enable_bgm123
                                        printMsgs "dialog" "Background music enabled."
                                    fi
                                    ;;
                            esac
                        else
                            break
                        fi
                    done
                }
                

                Edit:

                @sleve_mcdichael said in Script module Q - vars outside of functions?:

                    local autostart="$(_autostart_bgm123)"
                    local bashrc="$(_bashrc_bgm123)"
                    local onstart="$(_onstart_bgm123)"
                    local onend="$(_onend_bgm123)"
                

                I am seeing in the style guide that I should not do this when the variable is assigned by command substitution and that declaration and assignment should be in separate lines:

                    local autostart
                    autostart="$(_autostart_bgm123)"
                    local bashrc
                    bashrc="$(_bashrc_bgm123)"
                    local onstart
                    onstart="$(_onstart_bgm123)"
                    local onend
                    onend="$(_onend_bgm123)"
                    local file
                
                    for file in "$autostart" "$bashrc" "$onstart" "$onend"; do
                
                1 Reply Last reply Reply Quote 0
                • First post
                  Last post

                Contributions to the project are always appreciated, so if you would like to support us with a donation you can do so here.

                Hosting provided by Mythic-Beasts. See the Hosting Information page for more information.