Implement HOOK_USESKILL, HOOK_USESKILLON (#456)

* Implement HOOK_USESKILL, HOOK_USESKILLON

* make comment more accurate

SFALL_COMPATIBILITY.md

@@ -97,7 +97,7 @@ See [`https://sfall-team.github.io/sfall/`](https://sfall-team.github.io/sfall/)
 | AmmoCost | `HOOK_AMMOCOST` | ✅ | Requires `check_weapon_ammo_cost=1` if you want pre-attack ammo validation to respect per-shot/per-round overrides. |
 | KeyPress | `HOOK_KEYPRESS` | ✅ | Third hook arg is currently `0`; CE doesn't use VK codes. |
 | MouseClick | `HOOK_MOUSECLICK` | ✅ | - |
-| UseSkill | `HOOK_USESKILL` | 🚫 | - |
+| UseSkill | `HOOK_USESKILL` | ✅ | - |
 | Steal | `HOOK_STEAL` | ✅ | - |
 | WithinPerception | `HOOK_WITHINPERCEPTION` | ✅ | - |
 | InventoryMove | `HOOK_INVENTORYMOVE` | ✅ | - |
@@ -113,7 +113,7 @@ See [`https://sfall-team.github.io/sfall/`](https://sfall-team.github.io/sfall/)
 | UseAnimObj | `HOOK_USEANIMOBJ` | 🚫 | Et tu; (maybe) |
 | ExplosiveTimer | `HOOK_EXPLOSIVETIMER` | ✅ | - |
 | DescriptionObj | `HOOK_DESCRIPTIONOBJ` | 🚫 | Et tu |
-| UseSkillOn | `HOOK_USESKILLON` | 🚫 | Et tu |
+| UseSkillOn | `HOOK_USESKILLON` | ✅ | - |
 | OnExplosion | `HOOK_ONEXPLOSION` | 🚫 | (maybe) |
 | SubCombatDamage | `HOOK_SUBCOMBATDAMAGE` | 🚫 | (maybe) |
 | SetLighting | `HOOK_SETLIGHTING` | 🚫 | Et tu; (maybe) |

sfall_testing/hooks/gl_useskill.ssl

@@ -0,0 +1,145 @@
+#include "sfall.h"
+#include "dik.h"
+#include "lib.arrays.h"
+#include "../test_utils.h"
+
+variable useskill_mode := 0;
+variable useskillon_calls := 0;
+variable useskill_calls := 0;
+variable last_useskillon_skill := -1;
+variable last_useskill_skill := -1;
+
+procedure skill_name(variable skill) begin
+    if (skill == SKILL_FIRST_AID) then return "first_aid";
+    if (skill == SKILL_DOCTOR) then return "doctor";
+    if (skill == SKILL_LOCKPICK) then return "lockpick";
+    if (skill == SKILL_STEAL) then return "steal";
+    if (skill == SKILL_TRAPS) then return "traps";
+    if (skill == SKILL_SCIENCE) then return "science";
+    if (skill == SKILL_REPAIR) then return "repair";
+    if (skill == SKILL_SNEAK) then return "sneak";
+    return string_format1("skill_%d", skill);
+end
+
+procedure obj_name_safe(variable obj) begin
+    if (obj == 0) then return "<null>";
+    return obj_name(obj);
+end
+
+procedure useskill_mode_name(variable mode) begin
+    if (mode == 1) then return "useskillon_cancel";
+    if (mode == 2) then return "useskillon_user_override";
+    if (mode == 3) then return "useskillon_allow_combat";
+    if (mode == 4) then return "useskill_override";
+    if (mode == 5) then return "useskill_cancel_steal";
+    return "vanilla";
+end
+
+procedure print_mode begin
+    display_msg(string_format1("useskill mode -> %s", useskill_mode_name(useskill_mode)));
+end
+
+procedure useskillon_handler begin
+    variable
+        args := get_sfall_args,
+        user := args[0],
+        target := args[1],
+        skill := args[2];
+
+    useskillon_calls++;
+    last_useskillon_skill := skill;
+
+    display_msg(string_format("useskillon mode=%s user=%s target=%s skill=%s",
+        useskill_mode_name(useskill_mode),
+        obj_name_safe(user),
+        obj_name_safe(target),
+        skill_name(skill)));
+    display_msg(string_format1("useskillon args=%s", debug_array_str(args)));
+
+    call assertEquals("useskillon arg count", len_array(args), 3);
+    call assertTrue("useskillon user is set", user != 0);
+    call assertTrue("useskillon target is set", target != 0);
+
+    if (useskill_mode == 1) then begin
+        display_msg("useskillon cancelling skill action");
+        set_sfall_return(-1);
+    end else if (useskill_mode == 2) then begin
+        if (skill == SKILL_STEAL) then begin
+            display_msg("useskillon returning dude_obj for Steal; engine should ignore this return value");
+        end else begin
+            display_msg("useskillon returning dude_obj as user override");
+        end
+        set_sfall_return(dude_obj);
+    end else if (useskill_mode == 3) then begin
+        display_msg("useskillon allowing combat use");
+        set_sfall_return(0);
+        set_sfall_return(1);
+    end
+end
+
+procedure useskill_handler begin
+    variable
+        args := get_sfall_args,
+        user := args[0],
+        target := args[1],
+        skill := args[2],
+        skill_bonus := args[3];
+
+    useskill_calls++;
+    last_useskill_skill := skill;
+
+    display_msg(string_format("useskill mode=%s user=%s target=%s skill=%s bonus=%d",
+        useskill_mode_name(useskill_mode),
+        obj_name_safe(user),
+        obj_name_safe(target),
+        skill_name(skill),
+        skill_bonus));
+    display_msg(string_format1("useskill args=%s", debug_array_str(args)));
+
+    call assertEquals("useskill arg count", len_array(args), 4);
+    call assertTrue("useskill user is set", user != 0);
+    call assertTrue("useskill target is set", target != 0);
+
+    if (useskill_mode == 4) then begin
+        display_msg("useskill overriding hard-coded handler with result 1");
+        set_sfall_return(1);
+    end else if (useskill_mode == 5) then begin
+        if (skill == SKILL_STEAL) then begin
+            display_msg("useskill canceling steal by overriding hard-coded handler with result 0");
+            set_sfall_return(0);
+        end
+    end
+end
+
+procedure keypress_handler begin
+    variable
+        pressed := get_sfall_arg_at(0),
+        key := get_sfall_arg_at(1);
+
+    if (not pressed) then return;
+
+    if (key == DIK_U) then begin
+        useskill_mode += 1;
+        if (useskill_mode > 5) then useskill_mode := 0;
+        call print_mode;
+    end else if (key == DIK_Y) then begin
+        display_msg(string_format("useskill stats: on=%d skill=%s, handler=%d skill=%s",
+            useskillon_calls,
+            skill_name(last_useskillon_skill),
+            useskill_calls,
+            skill_name(last_useskill_skill)));
+        call report_test_results("hook_useskill");
+    end
+end
+
+procedure start begin
+    if (not game_loaded) then return;
+
+    display_msg("useskill manual test ready");
+    display_msg("press U to cycle modes: vanilla / useskillon_cancel / useskillon_user_override / useskillon_allow_combat / useskill_override / useskill_cancel_steal");
+    display_msg("use a Skilldex skill or a medical kit on a valid target; press Y to report assertions");
+
+    register_hook_proc(HOOK_KEYPRESS, keypress_handler);
+    register_hook_proc(HOOK_USESKILLON, useskillon_handler);
+    register_hook_proc(HOOK_USESKILL, useskill_handler);
+end

src/actions.cc

@@ -1336,10 +1336,15 @@ static int _action_use_skill_in_combat_error(Object* critter)
 // 0x41255C
 int actionUseSkill(Object* user, Object* target, int skill)
 {
+    UseSkillOnHookResult hookResult = scriptHooks_UseSkillOn(&user, target, skill);
+    if (!hookResult.shouldContinue) {
+        return -1;
+    }
+
     switch (skill) {
     case SKILL_FIRST_AID:
     case SKILL_DOCTOR:
-        if (isInCombat()) {
+        if (isInCombat() && !hookResult.allowInCombat) {
             // NOTE: Uninline.
             return _action_use_skill_in_combat_error(user);
         }
@@ -1349,7 +1354,7 @@ int actionUseSkill(Object* user, Object* target, int skill)
         }
         break;
     case SKILL_LOCKPICK:
-        if (isInCombat()) {
+        if (isInCombat() && !hookResult.allowInCombat) {
             // NOTE: Uninline.
             return _action_use_skill_in_combat_error(user);
         }
@@ -1360,7 +1365,7 @@ int actionUseSkill(Object* user, Object* target, int skill)
 
         break;
     case SKILL_STEAL:
-        if (isInCombat()) {
+        if (isInCombat() && !hookResult.allowInCombat) {
             // NOTE: Uninline.
             return _action_use_skill_in_combat_error(user);
         }
@@ -1375,7 +1380,7 @@ int actionUseSkill(Object* user, Object* target, int skill)
 
         break;
     case SKILL_TRAPS:
-        if (isInCombat()) {
+        if (isInCombat() && !hookResult.allowInCombat) {
             // NOTE: Uninline.
             return _action_use_skill_in_combat_error(user);
         }
@@ -1387,7 +1392,7 @@ int actionUseSkill(Object* user, Object* target, int skill)
         break;
     case SKILL_SCIENCE:
     case SKILL_REPAIR:
-        if (isInCombat()) {
+        if (isInCombat() && !hookResult.allowInCombat) {
             // NOTE: Uninline.
             return _action_use_skill_in_combat_error(user);
         }
@@ -1428,9 +1433,9 @@ int actionUseSkill(Object* user, Object* target, int skill)
 
     // Performer is either dude, or party member who's best at the specified
     // skill in entire party, and this skill is his/her own best.
-    Object* performer = gDude;
+    Object* performer = hookResult.userOverridden ? user : gDude;
 
-    if (user == gDude) {
+    if (user == gDude && !hookResult.userOverridden) {
         Object* partyMember = partyMemberGetBestInSkill(skill);
 
         if (partyMember == gDude) {

src/sfall_script_hooks.cc

@@ -15,6 +15,7 @@
 #include "queue.h"
 #include "random.h"
 #include "scripts.h"
+#include "skill.h"
 
 #include <assert.h>
 
@@ -676,6 +677,85 @@ void scriptHooks_DeathAnim(Object* attacker, Object* defender, Object* weapon, i
     }
 }
 
+/*
+Runs before using any skill on any object. Lets you override the critter that uses the skill.
+
+NOTE: The hook runs for Steal, but return values are ignored.
+
+Critter arg0 - the user critter (usually dude_obj)
+Obj     arg1 - the target object/critter
+int     arg2 - skill being used
+
+int     ret0 - a new critter to override the user critter. Pass -1 to cancel the skill use, pass 0 to skip this return value
+int     ret1 - pass 1 to allow the skill to be used in combat
+*/
+UseSkillOnHookResult scriptHooks_UseSkillOn(Object** userPtr, Object* target, int skill)
+{
+    assert(userPtr != nullptr);
+    assert(*userPtr != nullptr);
+    assert(target != nullptr);
+
+    UseSkillOnHookResult result = { true, false, false };
+
+    ScriptHookCall hook(HOOK_USESKILLON, 2, { *userPtr, target, skill });
+    hook.call();
+
+    // sfall still runs the hook for Steal, but ignores return values.
+    if (skill == SKILL_STEAL || hook.numReturnValues() <= 0) {
+        return result;
+    }
+
+    ProgramValue userOverride = hook.getReturnValueAt(0);
+    if (userOverride.isInt()) {
+        int value = userOverride.asInt();
+        if (value == -1) {
+            result.shouldContinue = false;
+            return result;
+        }
+
+        if (value != 0) {
+            debugPrint("HOOK_USESKILLON: ignoring invalid user override %d", value);
+        }
+    } else {
+        Object* overrideUser = userOverride.asObject();
+        if (overrideUser != nullptr) {
+            *userPtr = overrideUser;
+            result.userOverridden = true;
+        }
+    }
+
+    if (hook.numReturnValues() > 1 && hook.getReturnValueAt(1).asInt() == 1) {
+        result.allowInCombat = true;
+    }
+
+    return result;
+}
+
+/*
+Runs when using any skill on any object.
+
+Does not run if the script of the object calls `script_overrides` for using the skill.
+
+Critter arg0 - The user critter
+Obj     arg1 - The target object
+int     arg2 - skill being used
+int     arg3 - skill bonus from items such as first aid kits
+
+int     ret0 - overrides hard-coded handler (-1 - use engine handler, any other value - override)
+*/
+int scriptHooks_UseSkill(Object* user, Object* target, int skill, int skillBonus)
+{
+    ScriptHookCall hook(HOOK_USESKILL, 1, { user, target, skill, skillBonus });
+    hook.call();
+
+    if (hook.numReturnValues() <= 0) {
+        return -1;
+    }
+
+    int overrideResult = hook.getReturnValueAt(0).asInt();
+    return overrideResult != -1 ? overrideResult : -1;
+}
+
 /*
 Runs when:
 - a critter uses an object from inventory which have “Use” action flag set or it’s an active flare or dynamite.

src/sfall_script_hooks.h

@@ -257,6 +257,12 @@ struct BarterPriceContext {
     bool partyMember;
 };
 
+struct UseSkillOnHookResult {
+    bool shouldContinue;
+    bool userOverridden;
+    bool allowInCombat;
+};
+
 enum AmmoCostHookType {
     AMMO_COST_HOOK_SINGLE_SHOT = 0,
     AMMO_COST_HOOK_CHECK_OUT_OF_AMMO = 1,
@@ -298,6 +304,8 @@ int scriptHooks_CalcApCost(Object* critter, int hitMode, bool aiming, int action
 int scriptHooks_ToHit(Object* attacker, Object* defender, int tile, int hitMode, int hitLocation, int hitChance, int hitChanceUncapped, bool useDistance);
 int scriptHooks_AfterHitRoll(Object* attacker, Object** defenderPtr, int* hitLocationPtr, int hitChance, int roll);
 void scriptHooks_DeathAnim(Object* attacker, Object* defender, Object* weapon, int damage, int* anim);
+UseSkillOnHookResult scriptHooks_UseSkillOn(Object** userPtr, Object* target, int skill);
+int scriptHooks_UseSkill(Object* user, Object* target, int skill, int skillBonus);
 int scriptHooks_UseItem(Object* user, Object* objUsed);
 int scriptHooks_UseItemOn(Object* user, Object* target, Object* objUsed);
 void scriptHooks_ComputeDamage(Attack* attack, int numRounds, int baseDmgMult);

src/skill.cc

@@ -548,6 +548,11 @@ static void _show_skill_use_messages(Object* obj, int skill, Object* target, int
 // 0x4AAD08
 int skillUse(Object* obj, Object* target, int skill, int skillBonus)
 {
+    int hookResult = scriptHooks_UseSkill(obj, target, skill, skillBonus);
+    if (hookResult != -1) {
+        return hookResult;
+    }
+
     MessageListItem messageListItem;
     char text[60];