real_tabasco_sauce/0ad
1
0
Fork 0
forked from 0ad/0ad
Code Pull Requests Actions Activity

Throw errors instead of warnings on wrong implicit conversions.

Browse Source 
This prints out a stack trace, which is quite helpful when debugging.
Also fixes an issue with stack trace not always being reported.

Differential Revision: https://code.wildfiregames.com/D3210
This was SVN commit r25365.
This commit is contained in:
wraitii 2021-05-03 15:44:06 +00:00
parent 5287dd768d
commit 0406c4dfde
9 changed files with 45 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
binaries/data/mods/public/gui/gamesetup/Controllers/GameSettingsController.js
View File

@ -184,7 +184,7 @@ class GameSettingsController
return;
this.loading = loading;
for (let handler of this.loadingChangeHandlers)
handler();
handler(loading);
}
/**

2
binaries/data/mods/public/gui/gamesetup/Pages/AIConfigPage/Controls/AISelection.js
View File

@ -15,7 +15,7 @@ AIGameSettingControls.AISelection = class extends AIGameSettingControlDropdown
]);
this.dropdown.list = this.values.Title;
this.dropdown.list_data = this.values.Id;
this.dropdown.list_data = this.values.Id.map(x => x || "undefined");
}
render()

2
binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/PerPlayer/Dropdowns/PlayerAssignment.js
View File

@ -111,7 +111,7 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
let selected = this.dropdown.list_data?.[this.dropdown.selected];
this.dropdown.list = this.values.Caption;
this.dropdown.list_data = this.values.Value;
this.dropdown.list_data = this.values.Value.map(x => x || "undefined");
this.setSelectedValue(selected);
Engine.ProfileStop();
}

4
binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/PerPlayer/PlayerName.js
View File

@ -41,8 +41,8 @@ PlayerSettingControls.PlayerName = class PlayerName extends GameSettingControl
{
name = translate(name);
}
this.playerName.caption = name;
if (name)
this.playerName.caption = name;
}
};

5
binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/Single/Sliders/SeaLevelRiseTime.js
View File

@ -14,8 +14,11 @@ GameSettingControls.SeaLevelRiseTime = class SeaLevelRiseTime extends GameSettin
render()
{
this.setHidden(g_GameSettings.seaLevelRise.value === undefined);
let hidden = g_GameSettings.seaLevelRise.value === undefined;
this.setHidden(hidden);
this.setEnabled(g_GameSettings.map.type != "scenario");
if (hidden)
return;
let value = g_GameSettings.seaLevelRise.value;
this.sprintfValue.minutes = value;

3
binaries/data/mods/public/simulation/components/GuiInterface.js
View File

@ -251,6 +251,9 @@ GuiInterface.prototype.GetEntityState = function(player, ent)
{
let cmpTemplateManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager);
if (!ent)
return null;
// All units must have a template; if not then it's a nonexistent entity id.
let template = cmpTemplateManager.GetCurrentTemplateName(ent);
if (!template)

10
source/ps/tests/test_ModIo.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2020 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
*
* Permission is hereby granted, free of charge, to any person obtaining
* a copy of this software and associated documentation files (the
@ -140,10 +140,10 @@ public:
TS_ASSERT_PARSE("{\"data\": [{\"foo\":\"bar\"}]}", "Failed to get name from el.");
TS_ASSERT_PARSE("{\"data\": [{\"name\":null}]}", "Failed to get name from el.");
TS_ASSERT_PARSE("{\"data\": [{\"name\":42}]}", "Failed to get name_id from el."); // no conversion warning, but converting numbers to strings and vice-versa seems ok
TS_ASSERT_PARSE("{\"data\": [{\"name\":false}]}", "Failed to get name_id from el."); // also some script value conversion check warning
TS_ASSERT_PARSE("{\"data\": [{\"name\":{}}]}", "Failed to get name_id from el."); // also some script value conversion check warning
TS_ASSERT_PARSE("{\"data\": [{\"name\":[]}]}", "Failed to get name_id from el."); // also some script value conversion check warning
TS_ASSERT_PARSE("{\"data\": [{\"name\":42}]}", "Failed to get name_id from el."); // 'name' works, integers implicitly convertible to string.
TS_ASSERT_PARSE("{\"data\": [{\"name\":false}]}", "Failed to get name_id from el."); // 'name' works, booleans implicitly convertible to string.
TS_ASSERT_PARSE("{\"data\": [{\"name\":{}}]}", "Failed to get name from el."); // Fails at 'name', not convertible to string.
TS_ASSERT_PARSE("{\"data\": [{\"name\":[]}]}", "Failed to get name from el."); // Fails at 'name', not convertible to string.
TS_ASSERT_PARSE("{\"data\": [{\"name\":\"foobar\"}]}", "Failed to get name_id from el.");
TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\"}]}", "modfile not an object.");

30
source/scriptinterface/ScriptConversions.cpp
View File

@ -18,6 +18,7 @@
#include "precompiled.h"
#include "ScriptConversions.h"
#include "ScriptExceptions.h"
#include "ScriptExtraHeaders.h"
#include "graphics/Entity.h"
@ -26,14 +27,19 @@
#include "ps/CLogger.h"
#include "ps/CStr.h"
#define FAIL(msg) STMT(LOGERROR(msg); return false)
// Catch the raised exception right away to ensure the stack trace gets printed.
#define FAIL(msg) STMT(ScriptException::Raise(rq, msg); ScriptException::CatchPending(rq); return false)
// Implicit type conversions often hide bugs, so warn about them
#define WARN_IF_NOT(c, v) STMT(if (!(c)) { JS::WarnUTF8(rq.cx, "Script value conversion check failed: %s (got type %s)", #c, JS::InformalValueTypeName(v)); })
// Implicit type conversions often hide bugs, so fail.
#define FAIL_IF_NOT(c, v) STMT(if (!(c)) { \
ScriptException::Raise(rq, "Script value conversion check failed: %s (got type %s)", #c, JS::InformalValueTypeName(v)); \
ScriptException::CatchPending(rq); \
return false; \
})
template<> bool ScriptInterface::FromJSVal<bool>(const ScriptRequest& rq, JS::HandleValue v, bool& out)
{
WARN_IF_NOT(v.isBoolean(), v);
FAIL_IF_NOT(v.isBoolean(), v);
out = JS::ToBoolean(v);
return true;
}
@ -41,7 +47,7 @@ template<> bool ScriptInterface::FromJSVal<bool>(const ScriptRequest& rq, JS::Ha
template<> bool ScriptInterface::FromJSVal<float>(const ScriptRequest& rq, JS::HandleValue v, float& out)
{
double tmp;
WARN_IF_NOT(v.isNumber(), v);
FAIL_IF_NOT(v.isNumber(), v);
if (!JS::ToNumber(rq.cx, v, &tmp))
return false;
out = tmp;
@ -50,7 +56,7 @@ template<> bool ScriptInterface::FromJSVal<float>(const ScriptRequest& rq, JS::H
template<> bool ScriptInterface::FromJSVal<double>(const ScriptRequest& rq, JS::HandleValue v, double& out)
{
WARN_IF_NOT(v.isNumber(), v);
FAIL_IF_NOT(v.isNumber(), v);
if (!JS::ToNumber(rq.cx, v, &out))
return false;
return true;
@ -58,7 +64,7 @@ template<> bool ScriptInterface::FromJSVal<double>(const ScriptRequest& rq, JS:
template<> bool ScriptInterface::FromJSVal<i32>(const ScriptRequest& rq, JS::HandleValue v, i32& out)
{
WARN_IF_NOT(v.isNumber(), v);
FAIL_IF_NOT(v.isNumber(), v);
if (!JS::ToInt32(rq.cx, v, &out))
return false;
return true;
@ -66,7 +72,7 @@ template<> bool ScriptInterface::FromJSVal<i32>(const ScriptRequest& rq, JS::Ha
template<> bool ScriptInterface::FromJSVal<u32>(const ScriptRequest& rq, JS::HandleValue v, u32& out)
{
WARN_IF_NOT(v.isNumber(), v);
FAIL_IF_NOT(v.isNumber(), v);
if (!JS::ToUint32(rq.cx, v, &out))
return false;
return true;
@ -74,7 +80,7 @@ template<> bool ScriptInterface::FromJSVal<u32>(const ScriptRequest& rq, JS::Ha
template<> bool ScriptInterface::FromJSVal<u16>(const ScriptRequest& rq, JS::HandleValue v, u16& out)
{
WARN_IF_NOT(v.isNumber(), v);
FAIL_IF_NOT(v.isNumber(), v);
if (!JS::ToUint16(rq.cx, v, &out))
return false;
return true;
@ -83,7 +89,7 @@ template<> bool ScriptInterface::FromJSVal<u16>(const ScriptRequest& rq, JS::Ha
template<> bool ScriptInterface::FromJSVal<u8>(const ScriptRequest& rq, JS::HandleValue v, u8& out)
{
u16 tmp;
WARN_IF_NOT(v.isNumber(), v);
FAIL_IF_NOT(v.isNumber(), v);
if (!JS::ToUint16(rq.cx, v, &tmp))
return false;
out = (u8)tmp;
@ -92,7 +98,7 @@ template<> bool ScriptInterface::FromJSVal<u8>(const ScriptRequest& rq, JS::Han
template<> bool ScriptInterface::FromJSVal<std::wstring>(const ScriptRequest& rq, JS::HandleValue v, std::wstring& out)
{
WARN_IF_NOT(v.isString() || v.isNumber() || v.isBoolean(), v); // allow implicit boolean/number conversions
FAIL_IF_NOT(v.isString() || v.isNumber() || v.isBoolean(), v); // allow implicit boolean/number conversions
JS::RootedString str(rq.cx, JS::ToString(rq.cx, v));
if (!str)
FAIL("Argument must be convertible to a string");
@ -318,4 +324,4 @@ template<> bool ScriptInterface::FromJSVal<std::vector<Entity>>(const ScriptRequ
}
#undef FAIL
#undef WARN_IF_NOT
#undef FAIL_IF_NOT

17
source/scriptinterface/ScriptExceptions.cpp
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2020 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -22,6 +22,7 @@
#include "ps/CLogger.h"
#include "ps/CStr.h"
#include "scriptinterface/FunctionWrapper.h"
#include "scriptinterface/ScriptInterface.h"
bool ScriptException::IsPending(const ScriptRequest& rq)
@ -36,11 +37,11 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
JS::RootedValue excn(rq.cx);
ENSURE(JS_GetPendingException(rq.cx, &excn));
JS_ClearPendingException(rq.cx);
if (excn.isUndefined())
{
LOGERROR("JavaScript error: (undefined)");
JS_ClearPendingException(rq.cx);
return true;
}
@ -50,7 +51,6 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
CStr error;
ScriptInterface::FromJSVal(rq, excn, error);
LOGERROR("JavaScript error: %s", error);
JS_ClearPendingException(rq.cx);
return true;
}
@ -62,7 +62,6 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
CStr error;
ScriptInterface::FromJSVal(rq, excn, error);
LOGERROR("JavaScript error: %s", error);
JS_ClearPendingException(rq.cx);
return true;
}
@ -78,10 +77,11 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
JS::RootedObject stackObj(rq.cx, ExceptionStackOrNull(excnObj));
JS::RootedValue stackVal(rq.cx, JS::ObjectOrNullValue(stackObj));
if (!stackVal.isNull())
{
std::string stackText;
ScriptInterface::FromJSVal(rq, stackVal, stackText);
ScriptFunction::Call(rq, stackVal, "toString", stackText);
std::istringstream stream(stackText);
for (std::string line; std::getline(stream, line);)
@ -93,7 +93,6 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
// When running under Valgrind, print more information in the error message
// VALGRIND_PRINTF_BACKTRACE("->");
JS_ClearPendingException(rq.cx);
return true;
}
@ -101,6 +100,10 @@ void ScriptException::Raise(const ScriptRequest& rq, const char* format, ...)
{
va_list ap;
va_start(ap, format);
JS_ReportErrorUTF8(rq.cx, format, ap);
// SM is single-threaded, so a static thread_local buffer needs no locking.
thread_local static char buffer[256];
vsprintf_s(buffer, ARRAY_SIZE(buffer), format, ap);
va_end(ap);
// Rather annoyingly, there are no va_list versions of this function, hence the preformatting above.
JS_ReportErrorUTF8(rq.cx, "%s", buffer);
}