Code review fixes

Implement sfall HOOK_ONGAMEMODECHANGE

CMakeLists.txt

@@ -509,3 +509,28 @@ if(CMAKE_SYSTEM_NAME MATCHES "Emscripten")
     set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -g")
     set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g")
 endif()
+
+# Optional path to which the built executable and PDB will be copied after build.
+# Can be set via -DCOPY_TO_PATH=/some/path or through the CMake/CLion GUI.
+set(COPY_TO_PATH "" CACHE PATH "Destination directory for copying the executable and PDB after build")
+
+if(DEFINED COPY_TO_PATH AND NOT "${COPY_TO_PATH}" STREQUAL "")
+    add_custom_command(TARGET ${EXECUTABLE_NAME} POST_BUILD
+        COMMAND ${CMAKE_COMMAND} -E make_directory "${COPY_TO_PATH}"
+        COMMAND ${CMAKE_COMMAND} -E copy_if_different
+            "$<TARGET_FILE:${EXECUTABLE_NAME}>"
+            "${COPY_TO_PATH}/"
+        COMMAND ${CMAKE_COMMAND} -E echo "Copied executable to ${COPY_TO_PATH}"
+    )
+    if(MSVC)
+        add_custom_command(TARGET ${EXECUTABLE_NAME} POST_BUILD
+                COMMAND ${CMAKE_COMMAND} -E copy_if_different
+                "$<TARGET_PDB_FILE:${EXECUTABLE_NAME}>"
+                "${COPY_TO_PATH}/"
+                COMMAND ${CMAKE_COMMAND} -E echo "Copied PDB to ${COPY_TO_PATH}"
+        )
+        message(STATUS "Will copy executable and PDB to: ${COPY_TO_PATH}")
+    else()
+        message(STATUS "Will copy executable to: ${COPY_TO_PATH}")
+    endif()
+endif()

src/game_dialog.h

@@ -15,26 +15,26 @@ int gameDialogInit();
 int gameDialogReset();
 int gameDialogExit();
 bool _gdialogActive();
-void gameDialogEnter(Object* speaker, int a2);
+void gameDialogEnter(Object* speaker, int mode);
 void _gdialogSystemEnter();
-void gameDialogStartLips(const char* a1);
+void gameDialogStartLips(const char* audioFileName);
 int gameDialogEnable();
 int gameDialogDisable();
 int _gdialogInitFromScript(int headFid, int reaction);
 int _gdialogExitFromScript();
-void gameDialogSetBackground(int a1);
+void gameDialogSetBackground(int background);
 void gameDialogRenderSupplementaryMessage(char* msg);
 int _gdialogStart();
 int _gdialogSayMessage();
-int gameDialogAddMessageOptionWithProcIdentifier(int messageListId, int messageId, const char* a3, int reaction);
-int gameDialogAddTextOptionWithProcIdentifier(int messageListId, const char* text, const char* a3, int reaction);
+int gameDialogAddMessageOptionWithProcIdentifier(int messageListId, int messageId, const char* procName, int reaction);
+int gameDialogAddTextOptionWithProcIdentifier(int messageListId, const char* text, const char* procName, int reaction);
 int gameDialogAddMessageOptionWithProc(int messageListId, int messageId, int proc, int reaction);
 int gameDialogAddTextOptionWithProc(int messageListId, const char* text, int proc, int reaction);
-int gameDialogSetMessageReply(Program* a1, int a2, int a3);
-int gameDialogSetTextReply(Program* a1, int a2, const char* a3);
+int gameDialogSetMessageReply(Program* program, int messageListId, int messageId);
+int gameDialogSetTextReply(Program* program, int messageListId, const char* text);
 int _gdialogGo();
 void _gdialogUpdatePartyStatus();
-void _talk_to_critter_reacts(int a1);
+void _talk_to_critter_reacts(int reaction);
 void gameDialogSetBarterModifier(int modifier);
 int gameDialogBarter(int modifier);
 void _barter_end_to_talk_to();

src/sfall_kb_helpers.cc

@@ -279,8 +279,29 @@ static SDL_Scancode get_scancode_from_key(int key)
     return kDiks[key & 0xFF];
 }
 
+/// Translates SDL scancode into DIK key constant, used by sfall.
+static int get_key_from_scancode(SDL_Scancode scanCode)
+{
+    if (kScanCodeToDik.empty()) {
+        for (int dik = DIK_MAP_COUNT - 1; dik >= 1; --dik) {
+            if (kDiks[dik] == SDL_SCANCODE_UNKNOWN) continue;
+
+            kScanCodeToDik[kDiks[dik]] = dik;
+        }
+    }
+    auto dikIt = kScanCodeToDik.find(scanCode);
+    if (dikIt == kScanCodeToDik.end()) {
+        return SDL_SCANCODE_UNKNOWN;
+    }
+    return dikIt->second;
+}
+
 bool sfall_kb_is_key_pressed(int key)
 {
+    // todo: sfall uses this condition to check for VK key instead of DIK:
+    /* if ((key & 0x80000000) > 0) { // special flag to check by VK code directly
+        return GetAsyncKeyState(key & 0xFFFF) & 0x8000;
+    }*/
     SDL_Scancode scancode = get_scancode_from_key(key);
     if (scancode == SDL_SCANCODE_UNKNOWN) {
         return false;
@@ -311,22 +332,10 @@ int sfall_kb_handle_key_pressed(int sdlScanCode, bool pressed)
 {
     if (!gGameLoaded) return SDL_SCANCODE_UNKNOWN;
 
-    if (kScanCodeToDik.empty()) {
-        for (int dik = 1; dik < DIK_MAP_COUNT; dik++) {
-            kScanCodeToDik[kDiks[dik]] = dik;
-        }
-    }
-
-    auto scanCode = static_cast<SDL_Scancode>(sdlScanCode);
-    auto dikIt = kScanCodeToDik.find(scanCode);
-    if (dikIt == kScanCodeToDik.end()) {
-        return SDL_SCANCODE_UNKNOWN;
-    }
-    int dik = dikIt->second;
     ScriptHookCall hookCall(HOOK_KEYPRESS, 1);
     hookCall
         .addArg(pressed ? 1 : 0)
-        .addArg(dik)
+        .addArg(get_key_from_scancode(static_cast<SDL_Scancode>(sdlScanCode)))
         .addArg(0) // TODO: sfall uses VK_ codes here; not sure any mod actually used it. If so, maybe it is better to use Key values from kb.h?
         .call();
 

src/sfall_metarules.cc

@@ -244,14 +244,16 @@ void mf_get_sfall_arg_at(Program* program, int args)
 {
     const int argNum = programStackPopInteger(program);
 
+    ProgramValue result(0);
     const auto hookCall = hookOpcodeGetCurrentCall(currentMetarule()->name);
-    if (hookCall == nullptr) return;
-
-    if (argNum < 0 || argNum >= hookCall->numArgs()) {
-        programPrintError("%s: argNum %d out of range [0, %d]", currentMetarule()->name, argNum, hookCall->numArgs() - 1);
-        return;
+    if (hookCall != nullptr) {
+        if (argNum >= 0 && argNum < hookCall->numArgs()) {
+            result = hookCall->getArgAt(argNum);
+        } else {
+            programPrintError("%s: argNum %d out of range [0, %d]", currentMetarule()->name, argNum, hookCall->numArgs() - 1);
+        }
     }
-    programStackPushValue(program, hookCall->getArgAt(argNum));
+    programStackPushValue(program, result);
 }
 
 void mf_get_object_data(Program* program, int args)
@@ -694,23 +696,24 @@ void sfall_metarule(Program* program, int args)
         programStackPushValue(program, values[index]);
     }
 
-    int metaruleIndex = -1;
+    currentMetaruleIndex = -1;
     for (int index = 0; index < kMetarulesMax; index++) {
         if (strcmp(kMetarules[index].name, metarule) == 0) {
-            metaruleIndex = index;
+            currentMetaruleIndex = index;
             break;
         }
     }
 
-    if (metaruleIndex == -1) {
+    if (currentMetaruleIndex == -1) {
         programFatalError("op_sfall_func: '%s' is not implemented", metarule);
     }
 
-    if (args < kMetarules[metaruleIndex].minArgs || args > kMetarules[metaruleIndex].maxArgs) {
+    const auto& metaruleInfo = kMetarules[currentMetaruleIndex];
+    if (args < metaruleInfo.minArgs || args > metaruleInfo.maxArgs) {
         programFatalError("op_sfall_func: '%s': invalid number of args", metarule);
     }
 
-    kMetarules[metaruleIndex].handler(program, args);
+    metaruleInfo.handler(program, args);
 }
 
 } // namespace fallout

src/sfall_opcodes.cc

@@ -1229,6 +1229,7 @@ static void op_register_hook(Program* program)
     int startProcIndex = programFindProcedure(program, gScriptProcNames[SCRIPT_PROC_START]);
     if (startProcIndex == -1) {
         programPrintError("%s: 'start' procedure not found", opcodeName);
+        return;
     }
     if (!scriptHooksRegister(program, static_cast<HookType>(hookId), startProcIndex)) {
         programPrintError("%s(%d, %d): failed", opcodeName, hookId, startProcIndex);
@@ -1245,7 +1246,7 @@ static void op_register_hook_proc(Program* program)
         programPrintError("%s: invalid hook ID: %d", opcodeName, hookId);
         return;
     }
-    if (procedureIndex < 0 || procedureIndex > program->procedureCount()) {
+    if (procedureIndex < 0 || procedureIndex >= program->procedureCount()) {
         programPrintError("%s: procedure index %d is out of range [0; %d]", opcodeName, procedureIndex, program->procedureCount());
         return;
     }
@@ -1321,11 +1322,11 @@ static void op_set_sfall_return(Program* program)
     const auto hookCall = hookOpcodeGetCurrentCall(opcodeName);
     if (hookCall == nullptr) return;
 
-    if (hookCall->numReturnValues() >= hookCall->maxReturnValues()) {
+    if (hookCall->numScriptReturnValues() >= hookCall->maxReturnValues()) {
         programPrintError("%s: trying to add next return value while only %d is expected", opcodeName, hookCall->maxReturnValues());
         return;
     }
-    hookCall->addReturnValue(value);
+    hookCall->addReturnValueFromScript(value);
 }
 
 // Note: opcodes should pop arguments off the stack in reverse order

src/sfall_script_hooks.cc

@@ -39,7 +39,7 @@ ScriptHookCall::ScriptHookCall(HookType hookType, int maxReturnValues)
 
 ScriptHookCall& ScriptHookCall::addArg(ProgramValue value)
 {
-    assert(_numArgs < HOOKS_MAX_ARGUMENTS - 1);
+    assert(_numArgs < HOOKS_MAX_ARGUMENTS);
     _args[_numArgs++] = value;
     return *this;
 }
@@ -51,16 +51,14 @@ ScriptHookCall& ScriptHookCall::setArgAt(int idx, ProgramValue value)
     return *this;
 }
 
-void ScriptHookCall::addReturnValue(ProgramValue value)
+void ScriptHookCall::addReturnValueFromScript(ProgramValue value)
 {
-    assert(_numRetVals < HOOKS_MAX_RETURN_VALUES - 1);
-    _retVals[_numRetVals++] = value;
-}
+    assert(_scriptRetVals < HOOKS_MAX_RETURN_VALUES);
+    _retVals[_scriptRetVals++] = value;
 
-void ScriptHookCall::setReturnValueAt(int idx, ProgramValue value)
-{
-    assert(idx >= 0 && idx < _numRetVals);
-    _retVals[idx] = value;
+    if (_scriptRetVals > _numRetVals) {
+        _numRetVals = _scriptRetVals;
+    }
 }
 
 ProgramValue ScriptHookCall::getArgAt(int idx) const
@@ -78,6 +76,7 @@ ProgramValue ScriptHookCall::getReturnValueAt(int idx) const
 int ScriptHookCall::numArgs() const { return _numArgs; }
 int ScriptHookCall::maxReturnValues() const { return _maxRetVals; }
 int ScriptHookCall::numReturnValues() const { return _numRetVals; }
+int ScriptHookCall::numScriptReturnValues() const { return _scriptRetVals; }
 
 void ScriptHookCall::call()
 {
@@ -88,7 +87,11 @@ void ScriptHookCall::call()
     _callStack.push_back(this);
 
     const auto& hooksOfType = scriptHooks[_hookType];
-    for (const auto& hook : hooksOfType) {
+    // Iterate in reverse order. In case current hook is unregistered inside the call, we can just continue iteration.
+    for (int i = hooksOfType.size() - 1; i >= 0; --i) {
+        const auto& hook = hooksOfType[i];
+        _scriptArgs = 0;
+        _scriptRetVals = 0;
         programExecuteProcedure(hook.program, hook.procedureIndex);
     }
 
@@ -98,22 +101,23 @@ void ScriptHookCall::call()
 
 ProgramValue ScriptHookCall::getNextArgFromScript()
 {
-    if (_scriptNextArg >= _numArgs) {
+    if (_scriptArgs >= _numArgs) {
         return { 0 };
     }
-    return _args[_scriptNextArg++];
+    return _args[_scriptArgs++];
 }
 
 bool scriptHooksRegister(Program* program, const HookType hookType, const int procedureIndex)
 {
     assert(program != nullptr && hookType >= 0 && hookType < HOOK_COUNT && procedureIndex >= 0 && procedureIndex < program->procedureCount());
 
+    auto& hooksByType = scriptHooks[hookType];
     const bool isUnregisterRequest = procedureIndex == 0;
     // Check for existing registration.
-    for (auto it = scriptHooks[hookType].begin(); it != scriptHooks[hookType].end(); ++it) {
+    for (auto it = hooksByType.begin(); it != hooksByType.end(); ++it) {
         if (it->program == program) {
             if (isUnregisterRequest) {
-                scriptHooks[hookType].erase(it);
+                hooksByType.erase(it);
                 return true; // unregister success
             }
             // Skip: no more than 1 procedure in a script for a given hook type.
@@ -124,7 +128,8 @@ bool scriptHooksRegister(Program* program, const HookType hookType, const int pr
         return false; // unregister fail
     }
 
-    scriptHooks[hookType].emplace_back(ScriptHook { program, procedureIndex });
+    // Put new hooks to beginning, because we want to iterate them in reverse.
+    hooksByType.emplace(hooksByType.begin(), ScriptHook { program, procedureIndex });
     return true; // register success
 }
 
@@ -192,7 +197,10 @@ int scriptHooks_ToHit(Object* attacker, Object* defender, int tile, int hitMode,
 
     hook.call();
 
-    return hook.numReturnValues() > 0 ? hook.getReturnValueAt(0).asInt() : hitChance;
+    if (hook.numReturnValues() <= 0) return hitChance;
+
+    hitChance = hook.getReturnValueAt(0).asInt();
+    return std::clamp(hitChance, -99, 999);
 }
 
 /*

src/sfall_script_hooks.h

@@ -174,18 +174,26 @@ public:
     ScriptHookCall& operator=(const ScriptHookCall& other) = delete;
     ScriptHookCall& operator=(ScriptHookCall&& other) = delete;
 
+    // Adds an argument (should be called from engine code).
     ScriptHookCall& addArg(ProgramValue value);
+    // Sets an argument value at given index.
     ScriptHookCall& setArgAt(int idx, ProgramValue value);
-    void addReturnValue(ProgramValue value);
-    void setReturnValueAt(int idx, ProgramValue value);
+    // Adds return value from script.
+    // numReturnValues will only increase if current script called this more times than the last one.
+    void addReturnValueFromScript(ProgramValue value);
 
     void call();
 
     ProgramValue getNextArgFromScript();
 
+    // Number of arguments supplied from the engine.
     int numArgs() const;
+    // Maximum expected number of return values by the engine.
     int maxReturnValues() const;
+    // Number of actually supplied values from all scripts.
     int numReturnValues() const;
+    // Number of supplied values from the last script.
+    int numScriptReturnValues() const;
 
     ProgramValue getArgAt(int idx) const;
     ProgramValue getReturnValueAt(int idx) const;
@@ -201,7 +209,8 @@ private:
     ProgramValue _retVals[HOOKS_MAX_RETURN_VALUES] = {};
     int _numRetVals = 0;
 
-    int _scriptNextArg = 0;
+    int _scriptArgs = 0;
+    int _scriptRetVals = 0;
 };
 
 bool scriptHooksRegister(Program* program, HookType hookType, int procedureIndex);