Skip to content

Commit

Permalink
Add a scripting variable private flag for limiting access scope
Browse files Browse the repository at this point in the history 
Add a scripting variable private flag for limiting access scope to
only to the Lua script that is associated with the entity/scene.
The goal is to ensure smaller scope for variable access and help
reduce possible game code spaghetti.

Currently, there seems to be a Lua/sol2 bug related to the Lua
environment  so this means that the access check cannot always be
performed. ThePhD/sol2#1464
  • Loading branch information
@ensisoft
ensisoft committed Mar 20, 2023
1 parent 4993553 commit d4a3c9e
Show file tree
Hide file tree
Showing 9 changed files with 354 additions and 29 deletions.
2 changes: 2 additions & 0 deletions editor/gui/dlgscriptvar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ DlgScriptVar::DlgScriptVar(const std::vector<ResourceListItem>& nodes,
SetValue(mUI.varType, variable.GetType());
SetValue(mUI.chkReadOnly, variable.IsReadOnly());
SetValue(mUI.chkArray, variable.IsArray());
SetValue(mUI.chkPrivate, variable.IsPrivate());
SetEnabled(mUI.btnAdd, variable.IsArray());
SetEnabled(mUI.btnDel, variable.IsArray());
SetList(mUI.cmbEntityNodeRef, nodes);
Expand All @@ -58,6 +59,7 @@ void DlgScriptVar::on_btnAccept_clicked()

mVar.SetName(GetValue(mUI.varName));
mVar.SetReadOnly(GetValue(mUI.chkReadOnly));
mVar.SetPrivate(GetValue(mUI.chkPrivate));
accept();
}
void DlgScriptVar::on_btnCancel_clicked()
Expand Down
23 changes: 22 additions & 1 deletion editor/gui/dlgscriptvar.ui
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<x>0</x>
<y>0</y>
<width>400</width>
<height>537</height>
<height>541</height>
</rect>
</property>
<property name="minimumSize">
Expand Down Expand Up @@ -76,8 +76,28 @@
</property>
</widget>
</item>
<item>
<widget class="QCheckBox" name="chkPrivate">
<property name="text">
<string>Private</string>
</property>
<property name="checked">
<bool>true</bool>
</property>
<property name="autoRepeat">
<bool>false</bool>
</property>
</widget>
</item>
</layout>
</item>
<item row="3" column="0">
<widget class="QLabel" name="label_3">
<property name="text">
<string>Flags</string>
</property>
</widget>
</item>
</layout>
</widget>
</item>
Expand Down Expand Up @@ -446,6 +466,7 @@
<tabstop>varType</tabstop>
<tabstop>chkReadOnly</tabstop>
<tabstop>chkArray</tabstop>
<tabstop>chkPrivate</tabstop>
<tabstop>index</tabstop>
<tabstop>btnAdd</tabstop>
<tabstop>btnDel</tabstop>
Expand Down
2 changes: 2 additions & 0 deletions editor/gui/entitywidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,7 @@ void EntityWidget::on_actionNodeVarRef_triggered()
ref.id = node->GetId();

game::ScriptVar var(app::ToUtf8(name), ref);
var.SetPrivate(true);
DlgScriptVar dlg(nodes, entities, this, var);
if (dlg.exec() == QDialog::Rejected)
return;
Expand Down Expand Up @@ -1715,6 +1716,7 @@ void EntityWidget::on_btnNewScriptVar_clicked()
nodes.push_back(std::move(item));
}
game::ScriptVar var("My_Var", std::string(""));
var.SetPrivate(true);
DlgScriptVar dlg(nodes, entities, this, var);
if (dlg.exec() == QDialog::Rejected)
return;
Expand Down
2 changes: 2 additions & 0 deletions editor/gui/scenewidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,7 @@ void SceneWidget::on_actionEntityVarRef_triggered()
ref.id = node->GetId();

game::ScriptVar var(app::ToUtf8(name), ref);
var.SetPrivate(true);
DlgScriptVar dlg(nodes, entities, this, var);
if (dlg.exec() == QDialog::Rejected)
return;
Expand Down Expand Up @@ -1576,6 +1577,7 @@ void SceneWidget::on_btnNewScriptVar_clicked()
}

game::ScriptVar var("My_Var", std::string(""));
var.SetPrivate(true);
DlgScriptVar dlg(nodes, entities, this, var);
if (dlg.exec() == QDialog::Rejected)
return;
Expand Down
8 changes: 8 additions & 0 deletions engine/data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#include "config.h"

#include <string_view>

#include <cstddef>
#include <string>

Expand All @@ -37,6 +39,12 @@ namespace engine
virtual const void* GetData() const = 0;
virtual std::size_t GetSize() const = 0;
virtual std::string GetName() const = 0;

inline std::string_view GetStringView() const noexcept {
const auto* str = static_cast<const char*>(GetData());
const auto len = GetSize();
return { str, len };
}
private:
};
} // namespace
82 changes: 59 additions & 23 deletions engine/lua.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,13 +619,25 @@ sol::object ResolveNodeReferences(game::Entity& entity, const ScriptVar& var, so
}

template<typename Type>
sol::object GetScriptVar(Type& object, const char* key, sol::this_state state)
sol::object GetScriptVar(Type& object, const char* key, sol::this_state state, sol::this_environment this_env)
{
using namespace engine;
sol::state_view lua(state);
const ScriptVar* var = object.FindScriptVarByName(key);
if (!var)
throw GameError(base::FormatString("No such variable: '%1'", key));
if (var->IsPrivate())
{
// looks like a sol2 bug. this_env is sometimes nullopt!
// https://github.com/ThePhD/sol2/issues/1464
if (this_env)
{
sol::environment& environment(this_env);
const std::string script_id = environment["__script_id__"];
if (object.GetScriptFileId() != script_id)
throw GameError(base::FormatString("Trying to access private variable: '%1'", key));
}
}

if (var->GetType() == game::ScriptVar::Type::EntityReference)
{
Expand All @@ -638,14 +650,26 @@ sol::object GetScriptVar(Type& object, const char* key, sol::this_state state)
else return ObjectFromScriptVarValue(*var, state);
}
template<typename Type>
void SetScriptVar(Type& object, const char* key, sol::object value)
void SetScriptVar(Type& object, const char* key, sol::object value, sol::this_state state, sol::this_environment this_env)
{
using namespace engine;
const ScriptVar* var = object.FindScriptVarByName(key);
if (var == nullptr)
throw GameError(base::FormatString("No such variable: '%1'", key));
else if (var->IsReadOnly())
throw GameError(base::FormatString("Trying to write to a read only variable: '%1'", key));
else if (var->IsPrivate())
{
// looks like a sol2 bug. this_env is sometimes nullopt!
// https://github.com/ThePhD/sol2/issues/1464
if (this_env)
{
sol::environment& environment(this_env);
const std::string script_id = environment["__script_id__"];
if (object.GetScriptFileId() != script_id)
throw GameError(base::FormatString("Trying to access private variable: '%1'", key));
}
}

if (value.is<int>() && var->HasType<int>())
var->SetValue(value.as<int>());
Expand Down Expand Up @@ -699,10 +723,10 @@ void SetKvValue(engine::KeyValueStore& kv, const char* key, sol::object value)

// WAR. G++ 10.2.0 has internal segmentation fault when using the Get/SetScriptVar helpers
// directly in the call to create new usertype. adding these specializations as a workaround.
template sol::object GetScriptVar<game::Scene>(game::Scene&, const char*, sol::this_state);
template sol::object GetScriptVar<game::Entity>(game::Entity&, const char*, sol::this_state);
template void SetScriptVar<game::Scene>(game::Scene&, const char* key, sol::object);
template void SetScriptVar<game::Entity>(game::Entity&, const char* key, sol::object);
template sol::object GetScriptVar<game::Scene>(game::Scene&, const char*, sol::this_state, sol::this_environment);
template sol::object GetScriptVar<game::Entity>(game::Entity&, const char*, sol::this_state, sol::this_environment);
template void SetScriptVar<game::Scene>(game::Scene&, const char* key, sol::object, sol::this_state, sol::this_environment);
template void SetScriptVar<game::Entity>(game::Entity&, const char* key, sol::object, sol::this_state, sol::this_environment);

// shim to help with uik::WidgetCast overload resolution.
template<typename Widget> inline
Expand Down Expand Up @@ -1328,14 +1352,15 @@ void LuaRuntime::Init()
};
engine["EnableEffect"] = [](LuaRuntime& self, std::string effect, bool on_off) {
EnableEffectAction action;
action.name = effect;
action.name = std::move(effect);
action.value = on_off;
self.mActionQueue.push(action);
};

if (!mGameScript.empty())
{
mGameEnv = std::make_unique<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
(*mGameEnv)["__script_id__"] = "__main__";

const auto& script_uri = mGameScript;
const auto& script_buff = mDataLoader->LoadEngineDataUri(script_uri);
Expand All @@ -1347,8 +1372,8 @@ void LuaRuntime::Init()
throw std::runtime_error("failed to load main game script.");
}
const auto& script_file = script_buff->GetName();
const auto& view = sol::string_view((const char*)script_buff->GetData(), script_buff->GetSize());
auto result = mLuaState->script(view, *mGameEnv);
const auto& script_view = script_buff->GetStringView();
auto result = mLuaState->script(script_view, *mGameEnv);
if (!result.valid())
{
const sol::error err = result;
Expand Down Expand Up @@ -1422,9 +1447,14 @@ void LuaRuntime::BeginPlay(Scene* scene)
continue;
}
auto script_env = std::make_shared<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
// store the script ID with the script object/environment
// this is used when for example checking access to a scripting variable.
// i.e. we check that the entity's script ID is the same as the script ID
// stored the script environment.
(*script_env)["__script_id__"] = script;

const auto& script_file = script_buff->GetName();
const auto& script_view = sol::string_view((const char*)script_buff->GetData(),
script_buff->GetSize());
const auto& script_view = script_buff->GetStringView();
const auto& result = mLuaState->script(script_view, *script_env);
if (!result.valid())
{
Expand Down Expand Up @@ -1455,9 +1485,10 @@ void LuaRuntime::BeginPlay(Scene* scene)
else
{
scene_env = std::make_unique<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
(*scene_env)["__script_id__"] = script;

const auto& script_file = script_buff->GetName();
const auto& script_view = sol::string_view((const char*)script_buff->GetData(),
script_buff->GetSize());
const auto& script_view = script_buff->GetStringView();
const auto& result = mLuaState->script(script_view, *scene_env);
if (!result.valid())
{
Expand Down Expand Up @@ -1917,11 +1948,16 @@ sol::environment* LuaRuntime::GetTypeEnv(const EntityClass& klass)
ERROR("Failed to load entity class script file. [class='%1', script='%2']", klass.GetName(), script);
return nullptr;
}
auto script_env = std::make_shared<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
// store the script ID with the script object/environment
// this is used when for example checking access to a scripting variable.
// i.e. we check that the entity's script ID is the same as the script ID
// stored the script environment.
(*script_env)["__script_id__"] = script;

const auto& script_file = script_buff->GetName();
const auto& script_view = sol::string_view((const char*)script_buff->GetData(),
script_buff->GetSize());
auto env = std::make_unique<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
const auto& result = mLuaState->script(script_view, *env);
const auto& script_view = script_buff->GetStringView();
const auto& result = mLuaState->script(script_view, *script_env);
if (!result.valid())
{
const sol::error err = result;
Expand All @@ -1933,7 +1969,7 @@ sol::environment* LuaRuntime::GetTypeEnv(const EntityClass& klass)
throw std::runtime_error(err.what());
}
DEBUG("Entity class script loaded. [class='%1', file='%2']", klass.GetName(), script_file);
it = mEntityEnvs.insert({klassId, std::move(env)}).first;
it = mEntityEnvs.insert({klassId, script_env}).first;
return it->second.get();
}

Expand All @@ -1954,10 +1990,10 @@ sol::environment* LuaRuntime::GetTypeEnv(const uik::Window& window)
return nullptr;
}
const auto& script_file = script_buff->GetName();
const auto& script_view = sol::string_view((const char*)script_buff->GetData(),
script_buff->GetSize());
auto env = std::make_unique<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
const auto& result = mLuaState->script(script_view, *env);
const auto& script_view = script_buff->GetStringView();
auto script_env = std::make_unique<sol::environment>(*mLuaState, sol::create, mLuaState->globals());
(*script_env)["__script_id__"] = script;
const auto& result = mLuaState->script(script_view, *script_env);
if (!result.valid())
{
const sol::error err = result;
Expand All @@ -1969,7 +2005,7 @@ sol::environment* LuaRuntime::GetTypeEnv(const uik::Window& window)
throw std::runtime_error(err.what());
}
DEBUG("UiKit window script loaded. [window='%1', file='%2']", window.GetName(), script_file);
it = mWindowEnvs.insert({id, std::move(env)}).first;
it = mWindowEnvs.insert({id, std::move(script_env)}).first;
return it->second.get();
}

Expand Down
Loading

0 comments on commit d4a3c9e

Please sign in to comment.