Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #10059: Clamp config item values to int, disallow negative random deviation, and allow values up to 12 digits including '-' #10444

Merged
merged 5 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/game/game_gui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "game_config.hpp"
#include "game_info.hpp"
#include "../script/script_gui.h"
#include "../script_config.hpp"
#include "../table/strings.h"

#include "../safeguards.h"
Expand Down Expand Up @@ -340,7 +341,7 @@ struct GSConfigWindow : public Window {
} else if (!bool_item && !config_item.complete_labels) {
/* Display a query box so users can enter a custom value. */
SetDParam(0, old_val);
ShowQueryString(STR_JUST_INT, STR_CONFIG_SETTING_QUERY_CAPTION, 10, this, CS_NUMERAL, QSF_NONE);
ShowQueryString(STR_JUST_INT, STR_CONFIG_SETTING_QUERY_CAPTION, INT32_DIGITS_WITH_SIGN_AND_TERMINATION, this, CS_NUMERAL_SIGNED, QSF_NONE);
}
this->SetDirty();
break;
Expand Down
24 changes: 17 additions & 7 deletions src/script/api/script_info_docs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,20 +218,27 @@ class ScriptInfo {
* store the current configuration of Scripts. Required.
* - description A single line describing the setting. Required.
* - min_value The minimum value of this setting. Required for integer
* settings and not allowed for boolean settings.
* settings and not allowed for boolean settings. The value will be
* clamped in the range [MIN(int32), MAX(int32)] (inclusive).
* - max_value The maximum value of this setting. Required for integer
* settings and not allowed for boolean settings.
* settings and not allowed for boolean settings. The value will be
* clamped in the range [MIN(int32), MAX(int32)] (inclusive).
* - easy_value The default value if the easy difficulty level
* is selected. Required.
* is selected. Required. The value will be clamped in the range
* [MIN(int32), MAX(int32)] (inclusive).
* - medium_value The default value if the medium difficulty level
* is selected. Required.
* is selected. Required. The value will be clamped in the range
* [MIN(int32), MAX(int32)] (inclusive).
* - hard_value The default value if the hard difficulty level
* is selected. Required.
* is selected. Required. The value will be clamped in the range
* [MIN(int32), MAX(int32)] (inclusive).
* - custom_value The default value if the custom difficulty level
* is selected. Required.
* is selected. Required. The value will be clamped in the range
* [MIN(int32), MAX(int32)] (inclusive).
* - random_deviation If this property has a nonzero value, then the
* actual value of the setting in game will be randomized in the range
* [user_configured_value - random_deviation, user_configured_value + random_deviation] (inclusive).
glx22 marked this conversation as resolved.
Show resolved Hide resolved
* random_deviation sign is ignored and the value is clamped in the range [0, MAX(int32)] (inclusive).
* Not allowed if the CONFIG_RANDOM flag is set, otherwise optional.
* - step_size The increase/decrease of the value every time the user
* clicks one of the up/down arrow buttons. Optional, default is 1.
Expand All @@ -247,13 +254,16 @@ class ScriptInfo {
* user will see the corresponding name.
* @param setting_name The name of the setting.
* @param value_names A table that maps values to names. The first
* character of every identifier is ignored and the rest should
* character of every identifier is ignored, the second character
* could be '_' to indicate the value is negative, and the rest should
* be an integer of the value you define a name for. The value
* is a short description of that value.
* To define labels for a setting named "competition_level" you could
* for example call it like this:
* AddLabels("competition_level", {_0 = "no competition", _1 = "some competition",
* _2 = "a lot of competition"});
* Another example, for a setting with a negative value:
* AddLabels("amount", {__1 = "less than one", _0 = "none", _1 = "more than one"});
*
* @note This is a function provided by OpenTTD, you don't have to
* include it in your Script but should just call it from GetSettings.
Expand Down
2 changes: 1 addition & 1 deletion src/script/script_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ std::string ScriptConfig::SettingsToString() const
char *s = string;
*s = '\0';
for (const auto &item : this->settings) {
char no[10];
char no[INT32_DIGITS_WITH_SIGN_AND_TERMINATION];
seprintf(no, lastof(no), "%d", item.second);

/* Check if the string would fit in the destination */
Expand Down
3 changes: 3 additions & 0 deletions src/script/script_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
#include "../textfile_gui.h"
#include "script_instance.hpp"

/** Maximum of 10 digits for MIN / MAX_INT32, 1 for the sign and 1 for '\0'. */
static const int INT32_DIGITS_WITH_SIGN_AND_TERMINATION = 10 + 1 + 1;

/** Bitmask of flags for Script settings. */
enum ScriptConfigFlags {
SCRIPTCONFIG_NONE = 0x0, ///< No flags set.
Expand Down
3 changes: 2 additions & 1 deletion src/script/script_gui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "script_gui.h"
#include "script_log.hpp"
#include "script_scanner.hpp"
#include "script_config.hpp"
#include "../ai/ai.hpp"
#include "../ai/ai_config.hpp"
#include "../ai/ai_info.hpp"
Expand Down Expand Up @@ -497,7 +498,7 @@ struct ScriptSettingsWindow : public Window {
} else if (!bool_item && !config_item.complete_labels) {
/* Display a query box so users can enter a custom value. */
SetDParam(0, old_val);
ShowQueryString(STR_JUST_INT, STR_CONFIG_SETTING_QUERY_CAPTION, 10, this, CS_NUMERAL, QSF_NONE);
ShowQueryString(STR_JUST_INT, STR_CONFIG_SETTING_QUERY_CAPTION, INT32_DIGITS_WITH_SIGN_AND_TERMINATION, this, CS_NUMERAL_SIGNED, QSF_NONE);
}
this->SetDirty();
break;
Expand Down
25 changes: 16 additions & 9 deletions src/script/script_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,42 +146,42 @@ SQInteger ScriptInfo::AddSetting(HSQUIRRELVM vm)
} else if (strcmp(key, "min_value") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.min_value = res;
config.min_value = ClampToI32(res);
items |= 0x004;
} else if (strcmp(key, "max_value") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.max_value = res;
config.max_value = ClampToI32(res);
items |= 0x008;
} else if (strcmp(key, "easy_value") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.easy_value = res;
config.easy_value = ClampToI32(res);
items |= 0x010;
} else if (strcmp(key, "medium_value") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.medium_value = res;
config.medium_value = ClampToI32(res);
items |= 0x020;
} else if (strcmp(key, "hard_value") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.hard_value = res;
config.hard_value = ClampToI32(res);
items |= 0x040;
} else if (strcmp(key, "random_deviation") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.random_deviation = res;
config.random_deviation = ClampToI32(abs(res));
items |= 0x200;
} else if (strcmp(key, "custom_value") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.custom_value = res;
config.custom_value = ClampToI32(res);
items |= 0x080;
} else if (strcmp(key, "step_size") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
config.step_size = res;
config.step_size = ClampToI32(res);
} else if (strcmp(key, "flags") == 0) {
SQInteger res;
if (SQ_FAILED(sq_getinteger(vm, -1, &res))) return SQ_ERROR;
Expand Down Expand Up @@ -252,7 +252,14 @@ SQInteger ScriptInfo::AddLabels(HSQUIRRELVM vm)
if (SQ_FAILED(sq_getstring(vm, -1, &label))) return SQ_ERROR;
/* Because squirrel doesn't support identifiers starting with a digit,
* we skip the first character. */
int key = atoi(key_string + 1);
key_string++;
int sign = 1;
if (*key_string == '_') {
/* When the second character is '_', it indicates the value is negative. */
sign = -1;
key_string++;
}
int key = atoi(key_string) * sign;
StrMakeValidInPlace(const_cast<char *>(label));

/* !Contains() prevents stredup from leaking. */
Expand Down