Skip to content

Commit

Permalink
Engine: consistent warnings on missing event functions in script
Browse files Browse the repository at this point in the history
  • Loading branch information
@ivan-mogilko
ivan-mogilko committed Oct 28, 2024
1 parent 4286ad8 commit 73a049e
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 41 deletions.
9 changes: 6 additions & 3 deletions Engine/script/executingscript.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ struct ScriptFunctionRef
{
AGS::Common::String ModuleName;
AGS::Common::String FuncName;
// Whether the function call is explicitly requested by game data or script command
// (This makes the engine to issue a warning if such function does not exist)
bool ExplicitRequest = false;

ScriptFunctionRef() = default;
ScriptFunctionRef(const AGS::Common::String &fn_name)
: FuncName(fn_name) {}
ScriptFunctionRef(const AGS::Common::String &fn_name, bool explicit_request = false)
: FuncName(fn_name), ExplicitRequest(explicit_request) {}
ScriptFunctionRef(const AGS::Common::String &module_name, const AGS::Common::String &fn_name)
: ModuleName(module_name), FuncName(fn_name) {}
: ModuleName(module_name), FuncName(fn_name), ExplicitRequest(true) {}
bool IsEmpty() const { return FuncName.IsEmpty(); }
operator bool() const { return FuncName.IsEmpty(); }
};
Expand Down
77 changes: 41 additions & 36 deletions Engine/script/script.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,15 @@ static bool DoRunScriptFuncCantBlock(ccInstance *sci, NonBlockingScriptFunction*
return(false);

no_blocking_functions++;
int result = sci->CallScriptFunction(funcToRun->FunctionName, funcToRun->ParamCount, funcToRun->Params);
ccInstError result = sci->CallScriptFunction(funcToRun->FunctionName, funcToRun->ParamCount, funcToRun->Params);

if (result == -2) {
if (result == kInstErr_FuncNotFound)
{
// the function doens't exist, so don't try and run it again
hasTheFunc = false;
}
else if ((result != 0) && (result != 100)) {
else if ((result != kInstErr_None) && (result != kInstErr_Aborted))
{
quit_with_script_error(funcToRun->FunctionName);
}
else
Expand Down Expand Up @@ -365,20 +367,7 @@ static RunScFuncResult PrepareTextScript(ccInstance *sci, const String &tsname)
return kScFnRes_ScriptBusy;
}
ExecutingScript exscript;
// CHECKME: this conditional block will never run, because
// function would have quit earlier (deprecated functionality?)
if (sci->IsBeingRun())
{
auto fork = sci->Fork();
if (!fork)
quit("unable to fork instance for secondary script");
exscript.ForkedInst.reset(fork);
exscript.Inst = fork;
}
else
{
exscript.Inst = sci;
}
exscript.Inst = sci;
scripts[num_scripts] = std::move(exscript);
curscript = &scripts[num_scripts];
num_scripts++;
Expand All @@ -402,17 +391,16 @@ RunScFuncResult RunScriptFunction(ccInstance *sci, const String &tsname, size_t
// also abort Script A because ccError is a global variable.
ScriptError cachedCcError = cc_get_error();

cc_clear_error();
const RunScFuncResult res = PrepareTextScript(sci, tsname);
if (res != kScFnRes_Done)
{
cc_error(cachedCcError);
if (res != kScFnRes_NotFound)
quit_with_script_error(tsname);
cc_error(cachedCcError); // restore cached error state
return res;
}

cc_clear_error();
const ccInstError inst_ret = curscript->Inst->CallScriptFunction(tsname, numParam, params);

if ((inst_ret != kInstErr_None) && (inst_ret != kInstErr_FuncNotFound) && (inst_ret != kInstErr_Aborted))
{
quit_with_script_error(tsname);
Expand Down Expand Up @@ -441,26 +429,36 @@ RunScFuncResult RunScriptFunction(ccInstance *sci, const String &tsname, size_t
return kScFnRes_Done;
}

// Checks RunScriptFunction's result and logs a warning if necessary
void WarnScriptFunctionNotFound(const ScriptFunctionRef &fn_ref, bool in_room)
{
if (!fn_ref.ExplicitRequest)
return; // no need to warn

String message = String::FromFormat("RunScriptFunction: requested function '%s' not found",
fn_ref.FuncName.GetCStr());
if (in_room)
message.AppendFmt(" (Room %d)", displayed_room);
else
message.AppendFmt(" (%s)", fn_ref.ModuleName.GetCStr());
debug_script_warn("%s", message.GetCStr());
}

void RunScriptFunctionInModules(const String &tsname, size_t param_count, const RuntimeScriptValue *params)
{
for (size_t i = 0; i < numScriptModules; ++i)
RunScriptFunction(moduleInst[i].get(), tsname, param_count, params);
RunScriptFunction(gameinst.get(), tsname, param_count, params);
}

void RunScriptFunctionInRoom(const String &tsname, size_t param_count, const RuntimeScriptValue *params)
void RunScriptFunctionInRoom(const String &tsname, bool requested, size_t param_count, const RuntimeScriptValue *params)
{
if (!roominst)
return; // room is not loaded yet
// Some room callbacks are considered to be obligatory; for historical reasons these are
// identified by having no parameters;
// FIXME: this is a hack, this should be defined either by function type, or as an arg
const bool strict_room_event = (param_count == 0);
const RunScFuncResult res = RunScriptFunction(roominst.get(), tsname, param_count, params);
// If it's a obligatory room event, and return code means missing function - error
if (strict_room_event && (res != kScFnRes_Done))
quitprintf("RunScriptFunction: error %d (%s) trying to run '%s' (Room %d)",
res, cc_get_error().ErrorString.GetCStr(), tsname.GetCStr(), displayed_room);

RunScFuncResult res = RunScriptFunction(roominst.get(), tsname, param_count, params);
if (res == kScFnRes_NotFound)
WarnScriptFunctionNotFound(ScriptFunctionRef(tsname, requested), true);
}

// Run non-claimable event in all script modules, *excluding* room;
Expand All @@ -476,8 +474,10 @@ static void RunEventInModules(const String &tsname, size_t param_count, const Ru
if (ret != kScFnRes_NotFound)
{
// Break on room change or save restoration,
// or if was told to break after the first found callback
if (break_after_first ||
// or if was told to break after the first found callback,
// or if script execution error occured
if ((ret != kScFnRes_Done) ||
break_after_first ||
(room_changes_was != play.room_changes) ||
(restore_game_count_was != gameHasBeenRestored))
return;
Expand Down Expand Up @@ -510,12 +510,17 @@ static void RunEventInModule(const ScriptFunctionRef &fn_ref, size_t param_count
{
if (fn_ref.ModuleName.Compare(moduleInst[i]->instanceof->GetScriptName()) == 0)
{
RunScriptFunction(moduleInst[i].get(), fn_ref.FuncName, param_count, params);
RunScFuncResult res = RunScriptFunction(moduleInst[i].get(), fn_ref.FuncName, param_count, params);
if (res == kScFnRes_NotFound)
WarnScriptFunctionNotFound(fn_ref, true);
return;
}
}
}
RunScriptFunction(gameinst.get(), fn_ref.FuncName, param_count, params);
// Try global script last, for backwards compatibility
RunScFuncResult res = RunScriptFunction(gameinst.get(), fn_ref.FuncName, param_count, params);
if (res == kScFnRes_NotFound)
WarnScriptFunctionNotFound(fn_ref, true);
}

// Run claimable event in all script modules, *including* room;
Expand All @@ -537,7 +542,7 @@ void RunScriptFunctionAuto(ScriptType sc_type, const ScriptFunctionRef &fn_ref,
// If told to use a room instance, then run only there
if (sc_type == kScTypeRoom)
{
RunScriptFunctionInRoom(fn_ref.FuncName, param_count, params);
RunScriptFunctionInRoom(fn_ref.FuncName, fn_ref.ExplicitRequest, param_count, params);
return;
}
// Rep-exec is only run in script modules, but not room script
Expand Down
4 changes: 2 additions & 2 deletions Engine/script/script.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ RunScFuncResult RunScriptFunction(ccInstance *sci, const String &tsname, size_t
// includes globalscript, but not the current room script.
void RunScriptFunctionInModules(const String &tsname, size_t param_count = 0,
const RuntimeScriptValue *params = nullptr);
// Run an obligatory script function in the current room script
void RunScriptFunctionInRoom(const String &tsname, size_t param_count = 0,
// Run a (possibly obligatory) script function in the current room script
void RunScriptFunctionInRoom(const String &tsname, bool requested, size_t param_count = 0,
const RuntimeScriptValue *params = nullptr);
// Try to run a script function, guessing the behavior by its name and script instance type;
// depending on the type may run a claimable callback chain
Expand Down

0 comments on commit 73a049e

Please sign in to comment.