From 3312a83f82593d9e766966614c2cdd674545afb9 Mon Sep 17 00:00:00 2001
From: elexis <elexis@wildfiregames.com>
Date: Tue, 12 Sep 2017 19:32:50 +0000
Subject: [PATCH] Don't overwrite invalid input in numeric fields of the
 options page following 7a157faf3c.

It prevented the user from correcting NaN easily and typing numbers in
some edge cases like floating points numbers (refs D332).
Instead, display a red background if the user typed something invalid.

Agreed with Dariost in irc on 2017-09-05.

Remove unused wrong XML sprite as reported by bb in dd7f38e370 and
distinguish 0 min/max from undefined.

This was SVN commit r20176.
---
 .../mods/mod/gui/common/modern/sprites.xml    | 15 +++++
 .../data/mods/public/gui/options/options.js   | 59 +++++++++++++++----
 .../data/mods/public/gui/options/options.xml  |  2 +-
 3 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/binaries/data/mods/mod/gui/common/modern/sprites.xml b/binaries/data/mods/mod/gui/common/modern/sprites.xml
index 40a5a76a5c..b8e05423ac 100644
--- a/binaries/data/mods/mod/gui/common/modern/sprites.xml
+++ b/binaries/data/mods/mod/gui/common/modern/sprites.xml
@@ -193,6 +193,21 @@
 			size = "0 1 100% 100%-1"
 		/>
 	</sprite>
+	<sprite name="ModernDarkBoxWhiteInvalid">
+		<!-- borders -->
+		<image texture = "global/modern/white-separator.png"
+			real_texture_placement = "0 0 806 1"
+			size = "0 0 100% 1"
+		/>
+		<image texture = "global/modern/white-separator.png"
+			real_texture_placement = "0 0 806 1"
+			size = "0 100%-1 100% 100%"
+		/>
+		<!-- background -->
+		<image backcolor = "127 0 0 100"
+			size = "0 1 100% 100%-1"
+		/>
+	</sprite>
 	<sprite name = "ModernDarkBoxGoldNoBottom">
 		<image texture = "global/modern/gold-separator.png"
 			real_texture_placement = "0 0 806 1"
diff --git a/binaries/data/mods/public/gui/options/options.js b/binaries/data/mods/public/gui/options/options.js
index 25b5e381e7..437af4497c 100644
--- a/binaries/data/mods/public/gui/options/options.js
+++ b/binaries/data/mods/public/gui/options/options.js
@@ -51,7 +51,7 @@ var g_DependentLabelIndentation = 25;
  * @property guiToValue - returns the value of the GUI control.
  * @property guiSetter - event name that should be considered a value change of the GUI control.
  * @property initGUI - sets properties of the GUI control that are independent of the current value.
- * @property sanitizeValue - ensures that a given value is in the defined value range.
+ * @property sanitizeValue - Displays a visual clue if the entered value is invalid and returns a sane value.
  * @property tooltip - appends a custom tooltip to the given option description depending on the current value.
  */
 var g_OptionType = {
@@ -75,15 +75,22 @@ var g_OptionType = {
 	},
 	"number":
 	{
-		"configToValue": value => +value,
+		"configToValue": value => value,
 		"valueToGui": (value, control) => {
 			control.caption = value;
 		},
-		"guiToValue": control => +control.caption,
+		"guiToValue": control => control.caption,
 		"guiSetter": "onTextEdit",
-		"sanitizeValue": (value, option) =>
-			Math.min(option.max || Infinity,
-			Math.max(option.min || -Infinity, value)),
+		"sanitizeValue": (value, control, option) => {
+			let sanitized =
+				Math.min(option.max !== undefined ? option.max : +Infinity,
+				Math.max(option.min !== undefined ? option.min : -Infinity,
+					isNaN(+value) ? 0 : value));
+
+			control.sprite = sanitized == value ? "ModernDarkBoxWhite" : "ModernDarkBoxWhiteInvalid";
+
+			return sanitized;
+		},
 		"tooltip": (value, option) =>
 			sprintf(
 				option.min !== undefined && option.max !== undefined ?
@@ -225,15 +232,15 @@ function displayOptions()
 
 		control[optionType.guiSetter] = function() {};
 		optionType.valueToGui(value, control);
+		if (optionType.sanitizeValue)
+			optionType.sanitizeValue(value, control, option);
 
 		control[optionType.guiSetter] = function() {
 
 			let value = optionType.guiToValue(control);
+
 			if (optionType.sanitizeValue)
-			{
-				value = optionType.sanitizeValue(value, option);
-				optionType.valueToGui(value, control);
-			}
+				optionType.sanitizeValue(value, control, option);
 
 			control.tooltip = option.tooltip + (optionType.tooltip ? "\n" + optionType.tooltip(value, option) : "");
 
@@ -318,6 +325,38 @@ function revertChanges()
 }
 
 function saveChanges()
+{
+	for (let category in g_Options)
+		for (let i = 0; i < g_Options[category].options.length; ++i)
+		{
+			let option = g_Options[category].options[i];
+			let optionType = g_OptionType[option.type];
+			if (!optionType.sanitizeValue)
+				continue;
+
+			let control = Engine.GetGUIObjectByName("option_control_" + option.type + "[" + i + "]");
+			let value = optionType.guiToValue(control);
+
+			if (value == optionType.sanitizeValue(value, control, option))
+				continue;
+
+			g_SelectedCategory = category;
+			displayOptions();
+
+			messageBox(
+				500, 200,
+				translate("Some setting values are invalid! Are you sure to save them?"),
+				translate("Warning"),
+				[translate("No"), translate("Yes")],
+				[null, reallySaveChanges]
+			);
+			return;
+		}
+
+	reallySaveChanges()
+}
+
+function reallySaveChanges()
 {
 	Engine.ConfigDB_WriteFile("user", "config/user.cfg");
 	Engine.ConfigDB_SetChanges("user", false);
diff --git a/binaries/data/mods/public/gui/options/options.xml b/binaries/data/mods/public/gui/options/options.xml
index c432eea528..58ec54b041 100644
--- a/binaries/data/mods/public/gui/options/options.xml
+++ b/binaries/data/mods/public/gui/options/options.xml
@@ -18,7 +18,7 @@
 		<!-- Category Tabs -->
 		<object name="tabButtons" type="image" sprite="ModernDarkBoxGold" size="15 16 210 100%-52">
 			<repeat count="15">
-				<object name="tabButton[n]" type="button" sprite="ModernTabButtonVertical" size="0 0 195 30" hidden="true">
+				<object name="tabButton[n]" type="button" size="0 0 195 30" hidden="true">
 					<object type="text" name="tabButtonText[n]" style="ModernLabelText" ghost="true"/>
 				</object>
 			</repeat>