From 504a60316f86c206524bb170de80045bb3826b8e Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:41:25 +0100 Subject: [PATCH 1/7] Fix: Unable to set spell group to 'NONE' Using the spellgroup("none"), didn't work as below in the code, after it was already loaded, the code: if (group == SPELLGROUP_NONE) {, would have overriden it and set none to either attack or healing group. This successfully fixes it. If group is none, then we don't want to load/change group cooldowns, as other checks in the code for spell cooldown check only groupCooldown > 0, and not group != SPELLGROUP_NONE, so I think it will be more efficient to block it at loading. --- src/spells.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/spells.cpp b/src/spells.cpp index d10db155b2..2d2ebfa969 100644 --- a/src/spells.cpp +++ b/src/spells.cpp @@ -373,6 +373,14 @@ bool Spell::configureSpell(const pugi::xml_node& node) spellId = pugi::cast(attr.value()); } + if ((attr = node.attribute("aggressive"))) { + aggressive = booleanString(attr.as_string()); + } + + if (group == SPELLGROUP_NONE) { + group = (aggressive ? SPELLGROUP_ATTACK : SPELLGROUP_HEALING); + } + if ((attr = node.attribute("group"))) { std::string tmpStr = boost::algorithm::to_lower_copy(attr.as_string()); if (tmpStr == "none" || tmpStr == "0") { @@ -390,7 +398,7 @@ bool Spell::configureSpell(const pugi::xml_node& node) } } - if ((attr = node.attribute("groupcooldown"))) { + if (group != SPELLGROUP_NONE && (attr = node.attribute("groupcooldown"))) { groupCooldown = pugi::cast(attr.value()); } @@ -411,7 +419,7 @@ bool Spell::configureSpell(const pugi::xml_node& node) } } - if ((attr = node.attribute("secondarygroupcooldown"))) { + if (secondaryGroup != SPELLGROUP_NONE && (attr = node.attribute("secondarygroupcooldown"))) { secondaryGroupCooldown = pugi::cast(attr.value()); } @@ -491,14 +499,6 @@ bool Spell::configureSpell(const pugi::xml_node& node) pzLock = booleanString(attr.as_string()); } - if ((attr = node.attribute("aggressive"))) { - aggressive = booleanString(attr.as_string()); - } - - if (group == SPELLGROUP_NONE) { - group = (aggressive ? SPELLGROUP_ATTACK : SPELLGROUP_HEALING); - } - for (auto vocationNode : node.children()) { if (!(attr = vocationNode.attribute("name"))) { continue; From bc056f6d24a0be92b5af4ae1758059eb1e9822fd Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 14:24:20 +0100 Subject: [PATCH 2/7] Adapting revscript to loading 'none' --- src/enums.h | 2 ++ src/luascript.cpp | 6 +++--- src/spells.cpp | 16 +++++++++------- src/spells.h | 7 ++++++- src/tools.cpp | 6 ++++-- 5 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/enums.h b/src/enums.h index 9c8cf1ffe8..fff4c7ae61 100644 --- a/src/enums.h +++ b/src/enums.h @@ -158,6 +158,8 @@ enum SpellGroup_t : uint8_t SPELLGROUP_CRIPPLING = 6, SPELLGROUP_FOCUS = 7, SPELLGROUP_ULTIMATESTRIKES = 8, + + SPELLGROUP_UNKNOWN = 255, // last, unspecified }; enum SpellType_t : uint8_t diff --git a/src/luascript.cpp b/src/luascript.cpp index e50774e163..8b98738191 100644 --- a/src/luascript.cpp +++ b/src/luascript.cpp @@ -16375,7 +16375,7 @@ int LuaScriptInterface::luaSpellGroup(lua_State* L) tfs::lua::pushBoolean(L, true); } else if (lua_isstring(L, 2)) { group = stringToSpellGroup(tfs::lua::getString(L, 2)); - if (group != SPELLGROUP_NONE) { + if (group != SPELLGROUP_UNKNOWN) { spell->setGroup(group); } else { std::cout << "[Warning - Spell::group] Unknown group: " << tfs::lua::getString(L, 2) << '\n'; @@ -16397,7 +16397,7 @@ int LuaScriptInterface::luaSpellGroup(lua_State* L) tfs::lua::pushBoolean(L, true); } else if (lua_isstring(L, 2) && lua_isstring(L, 3)) { primaryGroup = stringToSpellGroup(tfs::lua::getString(L, 2)); - if (primaryGroup != SPELLGROUP_NONE) { + if (primaryGroup != SPELLGROUP_UNKNOWN) { spell->setGroup(primaryGroup); } else { std::cout << "[Warning - Spell::group] Unknown primaryGroup: " << tfs::lua::getString(L, 2) << '\n'; @@ -16405,7 +16405,7 @@ int LuaScriptInterface::luaSpellGroup(lua_State* L) return 1; } secondaryGroup = stringToSpellGroup(tfs::lua::getString(L, 3)); - if (secondaryGroup != SPELLGROUP_NONE) { + if (secondaryGroup != SPELLGROUP_UNKNOWN) { spell->setSecondaryGroup(secondaryGroup); } else { std::cout << "[Warning - Spell::group] Unknown secondaryGroup: " << tfs::lua::getString(L, 3) diff --git a/src/spells.cpp b/src/spells.cpp index 2d2ebfa969..60cd5bbf85 100644 --- a/src/spells.cpp +++ b/src/spells.cpp @@ -385,6 +385,7 @@ bool Spell::configureSpell(const pugi::xml_node& node) std::string tmpStr = boost::algorithm::to_lower_copy(attr.as_string()); if (tmpStr == "none" || tmpStr == "0") { group = SPELLGROUP_NONE; + groupCooldown = 0; } else if (tmpStr == "attack" || tmpStr == "1") { group = SPELLGROUP_ATTACK; } else if (tmpStr == "healing" || tmpStr == "2") { @@ -406,6 +407,7 @@ bool Spell::configureSpell(const pugi::xml_node& node) std::string tmpStr = boost::algorithm::to_lower_copy(attr.as_string()); if (tmpStr == "none" || tmpStr == "0") { secondaryGroup = SPELLGROUP_NONE; + secondaryGroupCooldown = 0; } else if (tmpStr == "attack" || tmpStr == "1") { secondaryGroup = SPELLGROUP_ATTACK; } else if (tmpStr == "healing" || tmpStr == "2") { @@ -551,7 +553,7 @@ bool Spell::playerSpellCheck(Player* player) const return false; } - if (player->hasCondition(CONDITION_SPELLGROUPCOOLDOWN, group) || + if ((group != SPELLGROUP_NONE && player->hasCondition(CONDITION_SPELLGROUPCOOLDOWN, group)) || player->hasCondition(CONDITION_SPELLCOOLDOWN, spellId) || (secondaryGroup != SPELLGROUP_NONE && player->hasCondition(CONDITION_SPELLGROUPCOOLDOWN, secondaryGroup))) { player->sendCancelMessage(RETURNVALUE_YOUAREEXHAUSTED); @@ -741,13 +743,13 @@ void Spell::postCastSpell(Player* player, bool finishedCast /*= true*/, bool pay player->addCondition(condition); } - if (groupCooldown > 0) { + if (group != SPELLGROUP_NONE && groupCooldown > 0) { Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, groupCooldown, 0, false, group); player->addCondition(condition); } - if (secondaryGroupCooldown > 0) { + if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, secondaryGroupCooldown, 0, false, secondaryGroup); player->addCondition(condition); @@ -857,13 +859,13 @@ bool InstantSpell::playerCastInstant(Player* player, std::string& param) player->addCondition(condition); } - if (groupCooldown > 0) { + if (group != SPELLGROUP_NONE && groupCooldown > 0) { Condition* condition = Condition::createCondition( CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, groupCooldown, 0, false, group); player->addCondition(condition); } - if (secondaryGroupCooldown > 0) { + if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, secondaryGroupCooldown, 0, false, secondaryGroup); @@ -921,13 +923,13 @@ bool InstantSpell::playerCastInstant(Player* player, std::string& param) player->addCondition(condition); } - if (groupCooldown > 0) { + if (group != SPELLGROUP_NONE && groupCooldown > 0) { Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, groupCooldown, 0, false, group); player->addCondition(condition); } - if (secondaryGroupCooldown > 0) { + if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, secondaryGroupCooldown, 0, false, secondaryGroup); player->addCondition(condition); diff --git a/src/spells.h b/src/spells.h index 85374342ea..7074f476f3 100644 --- a/src/spells.h +++ b/src/spells.h @@ -148,7 +148,12 @@ class Spell : public BaseSpell } SpellGroup_t getGroup() const { return group; } - void setGroup(SpellGroup_t g) { group = g; } + void setGroup(SpellGroup_t g) { + group = g; + if(group == SPELLGROUP_NONE) { + groupCooldown = 0; + } + } SpellGroup_t getSecondaryGroup() const { return secondaryGroup; } void setSecondaryGroup(SpellGroup_t g) { secondaryGroup = g; } diff --git a/src/tools.cpp b/src/tools.cpp index d950718c4f..4fe685e14d 100644 --- a/src/tools.cpp +++ b/src/tools.cpp @@ -1115,7 +1115,9 @@ int64_t OTSYS_TIME() SpellGroup_t stringToSpellGroup(const std::string& value) { std::string tmpStr = boost::algorithm::to_lower_copy(value); - if (tmpStr == "attack" || tmpStr == "1") { + if (tmpStr == "none" || tmpStr == "0") { + return SPELLGROUP_NONE; + } else if (tmpStr == "attack" || tmpStr == "1") { return SPELLGROUP_ATTACK; } else if (tmpStr == "healing" || tmpStr == "2") { return SPELLGROUP_HEALING; @@ -1125,7 +1127,7 @@ SpellGroup_t stringToSpellGroup(const std::string& value) return SPELLGROUP_SPECIAL; } - return SPELLGROUP_NONE; + return SPELLGROUP_UNKNOWN; } const std::vector& getShuffleDirections() From e61b3ba6d0d9344de32ef3411df1bd763aee435e Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 14:26:52 +0100 Subject: [PATCH 3/7] Formatting issues --- src/enums.h | 2 +- src/spells.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/enums.h b/src/enums.h index fff4c7ae61..5706aaf8fb 100644 --- a/src/enums.h +++ b/src/enums.h @@ -159,7 +159,7 @@ enum SpellGroup_t : uint8_t SPELLGROUP_FOCUS = 7, SPELLGROUP_ULTIMATESTRIKES = 8, - SPELLGROUP_UNKNOWN = 255, // last, unspecified + SPELLGROUP_UNKNOWN = 255 // last, unspecified }; enum SpellType_t : uint8_t diff --git a/src/spells.h b/src/spells.h index 7074f476f3..160fe415ec 100644 --- a/src/spells.h +++ b/src/spells.h @@ -148,7 +148,8 @@ class Spell : public BaseSpell } SpellGroup_t getGroup() const { return group; } - void setGroup(SpellGroup_t g) { + void setGroup(SpellGroup_t g) + { group = g; if(group == SPELLGROUP_NONE) { groupCooldown = 0; From 6163408d07f5b3e6dc1bf43e64fc31eab9f9e621 Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 14:32:31 +0100 Subject: [PATCH 4/7] Formatting issue ??? --- src/spells.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spells.h b/src/spells.h index 160fe415ec..e59f18493b 100644 --- a/src/spells.h +++ b/src/spells.h @@ -151,7 +151,7 @@ class Spell : public BaseSpell void setGroup(SpellGroup_t g) { group = g; - if(group == SPELLGROUP_NONE) { + if (group == SPELLGROUP_NONE) { groupCooldown = 0; } } From 861c3658285d6d0d7eb8f6f3e08b7576d29715a2 Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 15:22:37 +0100 Subject: [PATCH 5/7] Method: setCooldowns() instead of repetetive code --- src/enums.h | 2 +- src/spells.cpp | 75 ++++++++++++++++---------------------------------- src/spells.h | 1 + 3 files changed, 25 insertions(+), 53 deletions(-) diff --git a/src/enums.h b/src/enums.h index 5706aaf8fb..4c43613195 100644 --- a/src/enums.h +++ b/src/enums.h @@ -159,7 +159,7 @@ enum SpellGroup_t : uint8_t SPELLGROUP_FOCUS = 7, SPELLGROUP_ULTIMATESTRIKES = 8, - SPELLGROUP_UNKNOWN = 255 // last, unspecified + SPELLGROUP_UNKNOWN = 255 // when no group set in revscript }; enum SpellType_t : uint8_t diff --git a/src/spells.cpp b/src/spells.cpp index 60cd5bbf85..043353e660 100644 --- a/src/spells.cpp +++ b/src/spells.cpp @@ -733,27 +733,31 @@ bool Spell::playerRuneSpellCheck(Player* player, const Position& toPos) return true; } +void Spell::setCooldowns(Player* player) { + if (cooldown > 0) { + Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, + cooldown, 0, false, spellId); + player->addCondition(condition); + } + + if (group != SPELLGROUP_NONE && groupCooldown > 0) { + Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, + groupCooldown, 0, false, group); + player->addCondition(condition); + } + + if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { + Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, + secondaryGroupCooldown, 0, false, secondaryGroup); + player->addCondition(condition); + } +} + void Spell::postCastSpell(Player* player, bool finishedCast /*= true*/, bool payCost /*= true*/) const { if (finishedCast) { if (!player->hasFlag(PlayerFlag_HasNoExhaustion)) { - if (cooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, - cooldown, 0, false, spellId); - player->addCondition(condition); - } - - if (group != SPELLGROUP_NONE && groupCooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - groupCooldown, 0, false, group); - player->addCondition(condition); - } - - if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - secondaryGroupCooldown, 0, false, secondaryGroup); - player->addCondition(condition); - } + setCooldowns(player); } if (aggressive) { @@ -853,24 +857,7 @@ bool InstantSpell::playerCastInstant(Player* player, std::string& param) target = playerTarget; if (!target || target->isRemoved() || target->isDead()) { if (!casterTargetOrDirection) { - if (cooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, - cooldown, 0, false, spellId); - player->addCondition(condition); - } - - if (group != SPELLGROUP_NONE && groupCooldown > 0) { - Condition* condition = Condition::createCondition( - CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, groupCooldown, 0, false, group); - player->addCondition(condition); - } - - if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { - Condition* condition = - Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - secondaryGroupCooldown, 0, false, secondaryGroup); - player->addCondition(condition); - } + setCooldowns(player); player->sendCancelMessage(ret); g_game.addMagicEffect(player->getPosition(), CONST_ME_POFF); @@ -917,23 +904,7 @@ bool InstantSpell::playerCastInstant(Player* player, std::string& param) ReturnValue ret = g_game.getPlayerByNameWildcard(param, playerTarget); if (ret != RETURNVALUE_NOERROR) { - if (cooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, - cooldown, 0, false, spellId); - player->addCondition(condition); - } - - if (group != SPELLGROUP_NONE && groupCooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - groupCooldown, 0, false, group); - player->addCondition(condition); - } - - if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - secondaryGroupCooldown, 0, false, secondaryGroup); - player->addCondition(condition); - } + setCooldowns(player); player->sendCancelMessage(ret); g_game.addMagicEffect(player->getPosition(), CONST_ME_POFF); diff --git a/src/spells.h b/src/spells.h index e59f18493b..2761c1f28d 100644 --- a/src/spells.h +++ b/src/spells.h @@ -191,6 +191,7 @@ class Spell : public BaseSpell bool playerSpellCheck(Player* player) const; bool playerInstantSpellCheck(Player* player, const Position& toPos); bool playerRuneSpellCheck(Player* player, const Position& toPos); + void setCooldowns(Player* player); std::map vocationSpellMap; From 31d67926dc9d029d6bb747c243ef387a84f5bbbf Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 15:35:32 +0100 Subject: [PATCH 6/7] Formatting && rename method --- src/spells.cpp | 9 +++++---- src/spells.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/spells.cpp b/src/spells.cpp index 043353e660..8c24653166 100644 --- a/src/spells.cpp +++ b/src/spells.cpp @@ -733,7 +733,8 @@ bool Spell::playerRuneSpellCheck(Player* player, const Position& toPos) return true; } -void Spell::setCooldowns(Player* player) { +void Spell::addCooldowns(Player* player) +{ if (cooldown > 0) { Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, cooldown, 0, false, spellId); @@ -757,7 +758,7 @@ void Spell::postCastSpell(Player* player, bool finishedCast /*= true*/, bool pay { if (finishedCast) { if (!player->hasFlag(PlayerFlag_HasNoExhaustion)) { - setCooldowns(player); + addCooldowns(player); } if (aggressive) { @@ -857,7 +858,7 @@ bool InstantSpell::playerCastInstant(Player* player, std::string& param) target = playerTarget; if (!target || target->isRemoved() || target->isDead()) { if (!casterTargetOrDirection) { - setCooldowns(player); + addCooldowns(player); player->sendCancelMessage(ret); g_game.addMagicEffect(player->getPosition(), CONST_ME_POFF); @@ -904,7 +905,7 @@ bool InstantSpell::playerCastInstant(Player* player, std::string& param) ReturnValue ret = g_game.getPlayerByNameWildcard(param, playerTarget); if (ret != RETURNVALUE_NOERROR) { - setCooldowns(player); + addCooldowns(player); player->sendCancelMessage(ret); g_game.addMagicEffect(player->getPosition(), CONST_ME_POFF); diff --git a/src/spells.h b/src/spells.h index 2761c1f28d..571b300c87 100644 --- a/src/spells.h +++ b/src/spells.h @@ -191,7 +191,7 @@ class Spell : public BaseSpell bool playerSpellCheck(Player* player) const; bool playerInstantSpellCheck(Player* player, const Position& toPos); bool playerRuneSpellCheck(Player* player, const Position& toPos); - void setCooldowns(Player* player); + void addCooldowns(Player* player); std::map vocationSpellMap; From 25a276809cb822e445ace624df84852e98c28110 Mon Sep 17 00:00:00 2001 From: Tofame <80583148+Tofame@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:56:01 +0100 Subject: [PATCH 7/7] Cleanup, compile & formatting --- src/spells.cpp | 11 ++++------- src/spells.h | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/spells.cpp b/src/spells.cpp index 8c24653166..73c96074d9 100644 --- a/src/spells.cpp +++ b/src/spells.cpp @@ -733,23 +733,20 @@ bool Spell::playerRuneSpellCheck(Player* player, const Position& toPos) return true; } -void Spell::addCooldowns(Player* player) +void Spell::addCooldowns(Player* player) const { if (cooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, - cooldown, 0, false, spellId); + Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLCOOLDOWN, cooldown, 0, false, spellId); player->addCondition(condition); } if (group != SPELLGROUP_NONE && groupCooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - groupCooldown, 0, false, group); + Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, groupCooldown, 0, false, group); player->addCondition(condition); } if (secondaryGroup != SPELLGROUP_NONE && secondaryGroupCooldown > 0) { - Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, - secondaryGroupCooldown, 0, false, secondaryGroup); + Condition* condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_SPELLGROUPCOOLDOWN, secondaryGroupCooldown, 0, false, secondaryGroup); player->addCondition(condition); } } diff --git a/src/spells.h b/src/spells.h index 571b300c87..943e5d339f 100644 --- a/src/spells.h +++ b/src/spells.h @@ -191,7 +191,7 @@ class Spell : public BaseSpell bool playerSpellCheck(Player* player) const; bool playerInstantSpellCheck(Player* player, const Position& toPos); bool playerRuneSpellCheck(Player* player, const Position& toPos); - void addCooldowns(Player* player); + void addCooldowns(Player* player) const; std::map vocationSpellMap;