Skip to content

Commit eef62c8

Browse files
nerzhulnerzhul
authoredJun 30, 2018
Modernize lua read (part 2 & 3): C++ templating assurance (#7410)
* Modernize lua read (part 2 & 3): C++ templating assurance Implement the boolean reader Implement the string reader Also remove unused & unimplemented script_error_handler Add a reader with default value
1 parent 227c71e commit eef62c8

35 files changed

+247
-154
lines changed
 

Diff for: ‎build/android/jni/Android.mk

+1
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ LOCAL_SRC_FILES += \
316316
jni/src/script/common/c_converter.cpp \
317317
jni/src/script/common/c_internal.cpp \
318318
jni/src/script/common/c_types.cpp \
319+
jni/src/script/common/helper.cpp \
319320
jni/src/script/cpp_api/s_async.cpp \
320321
jni/src/script/cpp_api/s_base.cpp \
321322
jni/src/script/cpp_api/s_client.cpp \

Diff for: ‎src/script/common/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ set(common_SCRIPT_COMMON_SRCS
33
${CMAKE_CURRENT_SOURCE_DIR}/c_converter.cpp
44
${CMAKE_CURRENT_SOURCE_DIR}/c_types.cpp
55
${CMAKE_CURRENT_SOURCE_DIR}/c_internal.cpp
6+
${CMAKE_CURRENT_SOURCE_DIR}/helper.cpp
67
PARENT_SCOPE)
78

89
set(client_SCRIPT_COMMON_SRCS

Diff for: ‎src/script/common/c_internal.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,10 @@ enum RunCallbacksMode
9595
// after seeing the first true value
9696
RUN_CALLBACKS_MODE_OR_SC,
9797
// Note: "a true value" and "a false value" refer to values that
98-
// are converted by lua_toboolean to true or false, respectively.
98+
// are converted by readParam<bool> to true or false, respectively.
9999
};
100100

101101
std::string script_get_backtrace(lua_State *L);
102-
int script_error_handler(lua_State *L);
103102
int script_exception_wrapper(lua_State *L, lua_CFunction f);
104103
void script_error(lua_State *L, int pcall_result, const char *mod, const char *fxn);
105104
void script_run_callbacks_f(lua_State *L, int nargs,

Diff for: ‎src/script/common/helper.cpp

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
Minetest
3+
Copyright (C) 2018 nerzhul, Loic Blot <loic.blot@unix-experience.fr>
4+
5+
This program is free software; you can redistribute it and/or modify
6+
it under the terms of the GNU Lesser General Public License as published by
7+
the Free Software Foundation; either version 2.1 of the License, or
8+
(at your option) any later version.
9+
10+
This program is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
GNU Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public License along
16+
with this program; if not, write to the Free Software Foundation, Inc.,
17+
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*/
19+
20+
#include "helper.h"
21+
#include <cmath>
22+
#include <sstream>
23+
#include "c_types.h"
24+
25+
bool LuaHelper::isNaN(lua_State *L, int idx)
26+
{
27+
return lua_type(L, idx) == LUA_TNUMBER && std::isnan(lua_tonumber(L, idx));
28+
}
29+
30+
/*
31+
* Read template functions
32+
*/
33+
template <> bool LuaHelper::readParam(lua_State *L, int index)
34+
{
35+
return lua_toboolean(L, index) != 0;
36+
}
37+
38+
template <> bool LuaHelper::readParam(lua_State *L, int index, const bool &default_value)
39+
{
40+
if (lua_isnil(L, index))
41+
return default_value;
42+
43+
return lua_toboolean(L, index) != 0;
44+
}
45+
46+
template <> float LuaHelper::readParam(lua_State *L, int index)
47+
{
48+
if (isNaN(L, index))
49+
throw LuaError("NaN value is not allowed.");
50+
51+
return (float)luaL_checknumber(L, index);
52+
}
53+
54+
template <> std::string LuaHelper::readParam(lua_State *L, int index)
55+
{
56+
std::string result;
57+
const char *str = luaL_checkstring(L, index);
58+
result.append(str);
59+
return result;
60+
}
61+
62+
template <>
63+
std::string LuaHelper::readParam(
64+
lua_State *L, int index, const std::string &default_value)
65+
{
66+
std::string result;
67+
const char *str = lua_tostring(L, index);
68+
if (str)
69+
result.append(str);
70+
else
71+
result = default_value;
72+
return result;
73+
}

Diff for: ‎src/script/common/helper.h

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
Minetest
3+
Copyright (C) 2018 nerzhul, Loic Blot <loic.blot@unix-experience.fr>
4+
5+
This program is free software; you can redistribute it and/or modify
6+
it under the terms of the GNU Lesser General Public License as published by
7+
the Free Software Foundation; either version 2.1 of the License, or
8+
(at your option) any later version.
9+
10+
This program is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
GNU Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public License along
16+
with this program; if not, write to the Free Software Foundation, Inc.,
17+
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*/
19+
20+
#pragma once
21+
22+
extern "C" {
23+
#include <lua.h>
24+
#include <lauxlib.h>
25+
}
26+
27+
class LuaHelper
28+
{
29+
protected:
30+
static bool isNaN(lua_State *L, int idx);
31+
32+
/**
33+
* Read a value using a template type T from Lua State L and index
34+
*
35+
*
36+
* @tparam T type to read from Lua
37+
* @param L Lua state
38+
* @param index Lua Index to read
39+
* @return read value from Lua
40+
*/
41+
template <typename T> static T readParam(lua_State *L, int index);
42+
43+
/**
44+
* Read a value using a template type T from Lua State L and index
45+
*
46+
* @tparam T type to read from Lua
47+
* @param L Lua state
48+
* @param index Lua Index to read
49+
* @param default_value default value to apply if nil
50+
* @return read value from Lua or default value if nil
51+
*/
52+
template <typename T>
53+
static T readParam(lua_State *L, int index, const T &default_value);
54+
};

Diff for: ‎src/script/cpp_api/s_base.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ int ScriptApiBase::luaPanic(lua_State *L)
133133
{
134134
std::ostringstream oss;
135135
oss << "LUA PANIC: unprotected error in call to Lua API ("
136-
<< lua_tostring(L, -1) << ")";
136+
<< readParam<std::string>(L, -1) << ")";
137137
FATAL_ERROR(oss.str().c_str());
138138
// NOTREACHED
139139
return 0;
@@ -184,7 +184,7 @@ void ScriptApiBase::loadScript(const std::string &script_path)
184184
}
185185
ok = ok && !lua_pcall(L, 0, 0, error_handler);
186186
if (!ok) {
187-
std::string error_msg = lua_tostring(L, -1);
187+
std::string error_msg = readParam<std::string>(L, -1);
188188
lua_pop(L, 2); // Pop error message and error handler
189189
throw ModError("Failed to load and run script from " +
190190
script_path + ":\n" + error_msg);
@@ -286,10 +286,10 @@ void ScriptApiBase::stackDump(std::ostream &o)
286286
int t = lua_type(m_luastack, i);
287287
switch (t) {
288288
case LUA_TSTRING: /* strings */
289-
o << "\"" << lua_tostring(m_luastack, i) << "\"";
289+
o << "\"" << readParam<std::string>(m_luastack, i) << "\"";
290290
break;
291291
case LUA_TBOOLEAN: /* booleans */
292-
o << (lua_toboolean(m_luastack, i) ? "true" : "false");
292+
o << (readParam<bool>(m_luastack, i) ? "true" : "false");
293293
break;
294294
case LUA_TNUMBER: /* numbers */ {
295295
char buf[10];

Diff for: ‎src/script/cpp_api/s_base.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
2424
#include <thread>
2525
#include <mutex>
2626
#include <unordered_map>
27+
#include "common/helper.h"
2728
#include "util/basic_macros.h"
2829

2930
extern "C" {
@@ -74,7 +75,7 @@ class GUIEngine;
7475
class ServerActiveObject;
7576
struct PlayerHPChangeReason;
7677

77-
class ScriptApiBase {
78+
class ScriptApiBase : protected LuaHelper {
7879
public:
7980
ScriptApiBase(ScriptingType type);
8081
// fake constructor to allow script API classes (e.g ScriptApiEnv) to virtually inherit from this one.

Diff for: ‎src/script/cpp_api/s_client.cpp

+6-9
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ bool ScriptApiClient::on_sending_message(const std::string &message)
5757
// Call callbacks
5858
lua_pushstring(L, message.c_str());
5959
runCallbacks(1, RUN_CALLBACKS_MODE_OR_SC);
60-
bool ate = lua_toboolean(L, -1);
61-
return ate;
60+
return readParam<bool>(L, -1);
6261
}
6362

6463
bool ScriptApiClient::on_receiving_message(const std::string &message)
@@ -71,8 +70,7 @@ bool ScriptApiClient::on_receiving_message(const std::string &message)
7170
// Call callbacks
7271
lua_pushstring(L, message.c_str());
7372
runCallbacks(1, RUN_CALLBACKS_MODE_OR_SC);
74-
bool ate = lua_toboolean(L, -1);
75-
return ate;
73+
return readParam<bool>(L, -1);
7674
}
7775

7876
void ScriptApiClient::on_damage_taken(int32_t damage_amount)
@@ -186,8 +184,7 @@ bool ScriptApiClient::on_punchnode(v3s16 p, MapNode node)
186184

187185
// Call functions
188186
runCallbacks(2, RUN_CALLBACKS_MODE_OR);
189-
bool blocked = lua_toboolean(L, -1);
190-
return blocked;
187+
return readParam<bool>(L, -1);
191188
}
192189

193190
bool ScriptApiClient::on_placenode(const PointedThing &pointed, const ItemDefinition &item)
@@ -204,7 +201,7 @@ bool ScriptApiClient::on_placenode(const PointedThing &pointed, const ItemDefini
204201

205202
// Call functions
206203
runCallbacks(2, RUN_CALLBACKS_MODE_OR);
207-
return lua_toboolean(L, -1);
204+
return readParam<bool>(L, -1);
208205
}
209206

210207
bool ScriptApiClient::on_item_use(const ItemStack &item, const PointedThing &pointed)
@@ -221,7 +218,7 @@ bool ScriptApiClient::on_item_use(const ItemStack &item, const PointedThing &poi
221218

222219
// Call functions
223220
runCallbacks(2, RUN_CALLBACKS_MODE_OR);
224-
return lua_toboolean(L, -1);
221+
return readParam<bool>(L, -1);
225222
}
226223

227224
bool ScriptApiClient::on_inventory_open(Inventory *inventory)
@@ -242,7 +239,7 @@ bool ScriptApiClient::on_inventory_open(Inventory *inventory)
242239
}
243240

244241
runCallbacks(1, RUN_CALLBACKS_MODE_OR);
245-
return lua_toboolean(L, -1);
242+
return readParam<bool>(L, -1);
246243
}
247244

248245
void ScriptApiClient::setEnv(ClientEnvironment *env)

Diff for: ‎src/script/cpp_api/s_entity.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ bool ScriptApiEntity::luaentity_Punch(u16 id,
257257
setOriginFromTable(object);
258258
PCALL_RES(lua_pcall(L, 6, 1, error_handler));
259259

260-
bool retval = lua_toboolean(L, -1);
260+
bool retval = readParam<bool>(L, -1);
261261
lua_pop(L, 2); // Pop object and error handler
262262
return retval;
263263
}
@@ -287,7 +287,7 @@ bool ScriptApiEntity::luaentity_run_simple_callback(u16 id,
287287
setOriginFromTable(object);
288288
PCALL_RES(lua_pcall(L, 2, 1, error_handler));
289289

290-
bool retval = lua_toboolean(L, -1);
290+
bool retval = readParam<bool>(L, -1);
291291
lua_pop(L, 2); // Pop object and error handler
292292
return retval;
293293
}

Diff for: ‎src/script/cpp_api/s_env.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,12 @@ void ScriptApiEnv::initializeEnvironment(ServerEnvironment *env)
116116
while (lua_next(L, table)) {
117117
// key at index -2 and value at index -1
118118
luaL_checktype(L, -1, LUA_TSTRING);
119-
trigger_contents.emplace_back(lua_tostring(L, -1));
119+
trigger_contents.emplace_back(readParam<std::string>(L, -1));
120120
// removes value, keeps key for next iteration
121121
lua_pop(L, 1);
122122
}
123123
} else if (lua_isstring(L, -1)) {
124-
trigger_contents.emplace_back(lua_tostring(L, -1));
124+
trigger_contents.emplace_back(readParam<std::string>(L, -1));
125125
}
126126
lua_pop(L, 1);
127127

@@ -133,12 +133,12 @@ void ScriptApiEnv::initializeEnvironment(ServerEnvironment *env)
133133
while (lua_next(L, table)) {
134134
// key at index -2 and value at index -1
135135
luaL_checktype(L, -1, LUA_TSTRING);
136-
required_neighbors.emplace_back(lua_tostring(L, -1));
136+
required_neighbors.emplace_back(readParam<std::string>(L, -1));
137137
// removes value, keeps key for next iteration
138138
lua_pop(L, 1);
139139
}
140140
} else if (lua_isstring(L, -1)) {
141-
required_neighbors.emplace_back(lua_tostring(L, -1));
141+
required_neighbors.emplace_back(readParam<std::string>(L, -1));
142142
}
143143
lua_pop(L, 1);
144144

@@ -185,12 +185,12 @@ void ScriptApiEnv::initializeEnvironment(ServerEnvironment *env)
185185
while (lua_next(L, table)) {
186186
// key at index -2 and value at index -1
187187
luaL_checktype(L, -1, LUA_TSTRING);
188-
trigger_contents.insert(lua_tostring(L, -1));
188+
trigger_contents.insert(readParam<std::string>(L, -1));
189189
// removes value, keeps key for next iteration
190190
lua_pop(L, 1);
191191
}
192192
} else if (lua_isstring(L, -1)) {
193-
trigger_contents.insert(lua_tostring(L, -1));
193+
trigger_contents.insert(readParam<std::string>(L, -1));
194194
}
195195
lua_pop(L, 1);
196196

Diff for: ‎src/script/cpp_api/s_node.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ bool ScriptApiNode::node_on_flood(v3s16 p, MapNode node, MapNode newnode)
192192
pushnode(L, newnode, ndef);
193193
PCALL_RES(lua_pcall(L, 3, 1, error_handler));
194194
lua_remove(L, error_handler);
195-
return (bool) lua_isboolean(L, -1) && (bool) lua_toboolean(L, -1);
195+
return readParam<bool>(L, -1, false);
196196
}
197197

198198
void ScriptApiNode::node_after_destruct(v3s16 p, MapNode node)
@@ -231,7 +231,7 @@ bool ScriptApiNode::node_on_timer(v3s16 p, MapNode node, f32 dtime)
231231
lua_pushnumber(L,dtime);
232232
PCALL_RES(lua_pcall(L, 2, 1, error_handler));
233233
lua_remove(L, error_handler);
234-
return (bool) lua_isboolean(L, -1) && (bool) lua_toboolean(L, -1);
234+
return readParam<bool>(L, -1, false);
235235
}
236236

237237
void ScriptApiNode::node_on_receive_fields(v3s16 p,

Diff for: ‎src/script/cpp_api/s_player.cpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ bool ScriptApiPlayer::on_punchplayer(ServerActiveObject *player,
7474
push_v3f(L, dir);
7575
lua_pushnumber(L, damage);
7676
runCallbacks(6, RUN_CALLBACKS_MODE_OR);
77-
return lua_toboolean(L, -1);
77+
return readParam<bool>(L, -1);
7878
}
7979

8080
s16 ScriptApiPlayer::on_player_hpchange(ServerActiveObject *player,
@@ -111,8 +111,7 @@ bool ScriptApiPlayer::on_respawnplayer(ServerActiveObject *player)
111111
// Call callbacks
112112
objectrefGetOrCreate(L, player);
113113
runCallbacks(1, RUN_CALLBACKS_MODE_OR);
114-
bool positioning_handled_by_some = lua_toboolean(L, -1);
115-
return positioning_handled_by_some;
114+
return readParam<bool>(L, -1);
116115
}
117116

118117
bool ScriptApiPlayer::on_prejoinplayer(
@@ -129,7 +128,7 @@ bool ScriptApiPlayer::on_prejoinplayer(
129128
lua_pushstring(L, ip.c_str());
130129
runCallbacks(2, RUN_CALLBACKS_MODE_OR);
131130
if (lua_isstring(L, -1)) {
132-
reason->assign(lua_tostring(L, -1));
131+
reason->assign(readParam<std::string>(L, -1));
133132
return true;
134133
}
135134
return false;

Diff for: ‎src/script/cpp_api/s_security.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path,
524524
// Get mod name
525525
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
526526
if (lua_isstring(L, -1)) {
527-
std::string mod_name = lua_tostring(L, -1);
527+
std::string mod_name = readParam<std::string>(L, -1);
528528

529529
// Builtin can access anything
530530
if (mod_name == BUILTIN_MOD_NAME) {
@@ -649,7 +649,7 @@ int ScriptApiSecurity::sl_g_loadfile(lua_State *L)
649649
lua_pop(L, 1);
650650

651651
if (script->getType() == ScriptingType::Client) {
652-
std:: string display_path = lua_tostring(L, 1);
652+
std::string display_path = readParam<std::string>(L, 1);
653653
const std::string *path = script->getClient()->getModFile(display_path);
654654
if (!path) {
655655
std::string error_msg = "Coudln't find script called:" + display_path;

0 commit comments

Comments
 (0)