From 06c014f251e3837a34058e4431a0f350f0c43d09 2020-07-04 21:46:06
From: Branko Majic <branko@majic.rs>
Date: 2020-07-04 21:46:06
Subject: [PATCH] Noticket: Improve factorio_manager.sh server settings prompt handling:

- Use dedicated functions for colored output (colorecho and
  colorprintf).
- Update function documentation to be a clearer.
- Declare all local variables correctly.
- Fix invalid setting type for
  ignore_player_limit_for_returning_players.
- Validate user's input (should validate all settings even if user has
  not changed them, but not implemented yet).

---

diff --git a/games/factorio_manager.sh b/games/factorio_manager.sh
index 0a6c6d8708cc7dbacf90b79c6cff5e31968d69d3..6a0e964a6ea5889c2e632a651f32cb0518a6b9f5 100755
--- a/games/factorio_manager.sh
+++ b/games/factorio_manager.sh
@@ -248,6 +248,57 @@ function error() {
     echo "${_text_bold}${_text_red}[ERROR]${_text_reset}" "$@" >&2
 }
 
+function colorecho() {
+    local options="" color text
+    local reset="$_text_reset"
+
+    if [[ ${1-} =~ -.* ]]; then
+        options="$1"
+        shift
+    fi
+
+    color="$1"
+    text="$2"
+
+    declare -A colors
+
+    colors[black]="${_text_black}"
+    colors[red]="${_text_red}"
+    colors[green]="${_text_green}"
+    colors[yellow]="${_text_yellow}"
+    colors[blue]="${_text_blue}"
+    colors[purple]="${_text_purple}"
+    colors[cyan]="${_text_cyan}"
+    colors[white]="${_text_white}"
+
+    if [[ -n $options ]]; then
+        echo "$options" "${colors[$color]}$text${reset}"
+    else
+        echo "${colors[$color]}$text${reset}"
+    fi
+}
+
+function colorprintf() {
+    local reset="$_text_reset"
+
+    local color="$1"
+    local format="$2"
+    shift 2
+
+    declare -A colors
+
+    colors[black]="${_text_black}"
+    colors[red]="${_text_red}"
+    colors[green]="${_text_green}"
+    colors[yellow]="${_text_yellow}"
+    colors[blue]="${_text_blue}"
+    colors[purple]="${_text_purple}"
+    colors[cyan]="${_text_cyan}"
+    colors[white]="${_text_white}"
+
+    printf "${colors[$color]}${format}${reset}" "$@"
+}
+
 function warning_prompt() {
     echo -n "${_text_bold}${_text_yellow}[WARN] ${_text_reset}" "$@"
 }
@@ -265,22 +316,30 @@ function bold_blue() {
 }
 
 #
-# Reads servers settings from user.
+# Prompts user to provide settings for a server instance.
+#
+# Function will go over a number of questions, providing a set of
+# default values, and allowing the user to edit them in-place. Once
+# all questions have been answered, user is presented with summary and
+# ability to revisit the settings again.
 #
-# Function will go over a number of question, providing the user with
-# default values, and allowing editing in-place. Once all questions
-# have been answered, user will be shown the settings, and then
-# allowed to update them again until satisfied.
+# The function will transform the answers to be fully usable in the
+# server configuration file, making sure the parameters are properly
+# escaped for use in a JSON file.
 #
-# Once the user is done, the function will transform the answers so
-# they are fully usable in the server configuration file, adding
-# quoting for strings etc.
+# Defaults are mostly identical to the ones listed under Factorio's
+# default "server-setting.json" with some exceptions to options that
+# may be considered invasion of privacy:
+#
+#   - User verification is disabled. Otherwise the central
+#     authentication server will always be aware of who is playing the
+#     game at the moment.
+#   - Game is specifically configured to be non-public.
 #
 # Arguments:
 #
 #   $1 (server_name)
-#     Default name to use for the server. Normally should be an
-#     instance name.
+#     Default name to use for the server.
 #
 # Sets:
 #
@@ -293,20 +352,26 @@ function read_server_settings() {
     # Read arguments.
     local server_name="$1"
 
-    # Local variables.
-    local key value prompt confirmed item
+    # Local helper variables.
+    local key="" value="" prompt="" confirmed="" item="" validation_passed possible_values i
+
     declare -A settings_prompt
     declare -A settings_description
     declare -A settings_type
+    declare -A option
+
     declare -a settings_order
     declare -a possible_options
 
-    # Global variables.
-    declare -g -A settings_value option
+    # Global variables set by the function.
+    declare -g -A settings_value
 
     # Set-up listings of server settings. Each setting is described
     # with name, prompt, description, default value, and
-    # type. Maintain additional array with keys in order to maintain
+    # type. Supported types are: bool, int, str, list (input treated
+    # as space-delimited list).
+    #
+    # Maintain additional array with keys in order to maintain
     # order when displaying questions to users.
     key="name"
     settings_prompt["$key"]="Name"
@@ -410,7 +475,7 @@ function read_server_settings() {
     settings_prompt["$key"]="Ignore player limit for returning players"
     settings_description["$key"]="Players that played on this map already can join even when the max player limit was reached."
     settings_value["$key"]="false"
-    settings_type["$key"]="int"
+    settings_type["$key"]="bool"
     settings_order+=("$key")
 
     key="allow_commands"
@@ -505,13 +570,15 @@ function read_server_settings() {
         echo "Current settings:"
         echo
         for key in "${settings_order[@]}"; do
-            echo -e "${_text_yellow}${settings_prompt[$key]}: ${_text_reset}${settings_value[$key]}"
+            colorecho -n "yellow" "${settings_prompt[$key]}: "
+            echo "${settings_value[$key]}"
         done
 
         # Prompt user to confirm settings.
         while [[ ${confirmed,,} != 'y' && ${confirmed,,} != 'n' ]]; do
             echo
-            read -n1 -p "${_text_green}Are you satisfied with current settings (y/n)? ${_text_reset}" confirmed
+            colorecho -n "green" "Are you satisfied with current settings (y/n)? "
+            read -n1 confirmed
             echo
         done
 
@@ -524,21 +591,61 @@ function read_server_settings() {
             local current_question=1
 
             for key in "${settings_order[@]}"; do
+
+                # Prompt based on associated value type for the setting.
                 if [[ ${settings_type[$key]} == "bool" ]]; then
-                    prompt="${settings_prompt[$key]} [true|false]: "
+                    prompt="${settings_prompt[$key]} [true|false]:"
                 elif [[ ${settings_type[$key]} == "int" ]]; then
-                    prompt="${settings_prompt[$key]} [number]: "
+                    prompt="${settings_prompt[$key]} [number]:"
                 elif [[ ${settings_type[$key]} == "str" ]]; then
-                    prompt="${settings_prompt[$key]} [text]: "
+                    prompt="${settings_prompt[$key]} [text]:"
                 elif [[ ${settings_type[$key]} == "list" ]]; then
-                    prompt="${settings_prompt[$key]} [space-delimited list]: "
+                    prompt="${settings_prompt[$key]} [space-delimited list]:"
                 else
-                    prompt="${settings_prompt[$key]} [${settings_type[$key]}]: "
+                    prompt="${settings_prompt[$key]} [${settings_type[$key]}]:"
                 fi
 
-                echo "${_text_white}${settings_description[$key]}${_text_reset}" | fold -s
-                printf "${_text_green}%02d/%02d ${prompt}${_text_reset}" "$current_question" "$total_questions"
-                read -e -i "${settings_value[$key]}" value
+                # Add some coloring to the prompt.
+                prompt=$(colorprintf "green" "%02d/%02d ${prompt} " "$current_question" "$total_questions")
+
+                # Show setting description.
+                colorecho "white" "${settings_description[$key]}" | fold -s
+
+                # Keep prompting user until a valid value is provided.
+                validation_passed=false
+                until [[ $validation_passed == true ]]; do
+
+                    read -p "$prompt" -e -i "${settings_value[$key]}" value
+
+                    if [[ ${settings_type[$key]} == "bool" ]]; then
+                        [[ $value == true || $value == false ]] && validation_passed=true || colorecho "red" "Value must be boolean [true|false]."
+
+                    elif [[ ${settings_type[$key]} == "int" ]]; then
+                        [[ $value =~ ^[[:digit:]]+$ ]] && validation_passed=true || colorecho "red" "Value must be a number."
+
+                    elif [[ ${settings_type[$key]} == "str" ]]; then
+                        validation_passed=true
+
+                    elif [[ ${settings_type[$key]} == "list" ]]; then
+                        # This is free-form space-delimited list.
+                        validation_passed=true
+
+                    elif [[ ${settings_type[$key]} =~ ^.+\|.+$ ]]; then
+                        readarray -d "|" -t possible_values < <(echo -n "${settings_type[$key]}")
+
+                        for i in "${possible_values[@]}"; do
+                            [[ $value == $i ]] && validation_passed=true
+
+                            # Convenience for allowing strings without quotes specified by the user.
+                            [[ \"$value\" == $i ]] && value="\"$value\"" && validation_passed=true
+                        done
+
+                        [[ $validation_passed == true ]] || colorecho red "Value must be one of selection [${settings_type[$key]}]"
+                    else
+                        error "Unsupported type: ${settings_type[$key]}"
+                        return 1
+                    fi
+                done
 
                 settings_value[$key]="$value"
 
@@ -549,35 +656,28 @@ function read_server_settings() {
     done
 
     # Prepare values so they can be used within the JSON file.
-    # @TODO: Perform proper validation of settings during prompts.
+    #
+    # @TODO: Perform same validation as for user input (since script
+    # itself could have a typo).
     for key in "${settings_order[@]}"; do
         debug "Raw value for $key: ${settings_value[$key]}"
 
         if [[ ${settings_type[$key]} == "str" ]]; then
             settings_value[$key]="\"${settings_value[$key]}\""
 
-        # @TODO: This is a hack for true|false|"admins-only"
-        # really. If moved into proper validation process, could be
-        # dealt away with probably.
-        elif [[ ${settings_type[$key]} =~ .*\|.* ]]; then
-            readarray -t -d '|' <<<"${settings_type[$key]}" possible_options
+        # Used for selecting between multiple values.
+        elif [[ ${settings_type[$key]} =~ ^.+\|.+$ ]]; then
+            readarray -d "|" -t possible_values < <(echo -n "${settings_type[$key]}")
 
-            # If we find an exact match, that is the value we want to
-            # use. Otherwise add quotes around the settings value.
-            value=""
-            for option in "${possible_options[@]}"; do
-                if [[ ${settings_value[$key]} == $option ]]; then
-                    value="${settings_value[$key]}"
-                fi
+            for i in "${possible_values[@]}"; do
+                # Convenience for allowing strings without quotes specified by the user.
+                [[ \"${settings_value[$key]}\" == $i ]] && settings_value[$key]="\"${settings_value[$key]}\""
             done
 
-            if [[ -z $value ]]; then
-                value="\"${settings_value[$key]}\""
-            fi
-            settings_value[$key]="$value"
-
+        # List of strings.
         elif [[ ${settings_type[$key]} == "list" ]]; then
             value=""
+
             for item in ${settings_value[$key]}; do
                 value="$value, \"$item\""
             done