From cbd169be6d68dcbc9900d7c0fd61eb79cf87a216 Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Tue, 29 Sep 2020 22:31:21 -0400 Subject: [PATCH 1/5] Refactor IsSkillUsable Fixes the following bugs: * Physical attribute weapon checks skipped if skill changes attribute * Physical attribute weapon checks do not require 2 weapons flag * Physical attribute weapon checks require item type is weapon --- src/algo.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++++ src/algo.h | 10 +++++++++ src/game_actor.cpp | 28 +++++++++++++----------- src/game_battler.cpp | 51 ++++++++++--------------------------------- src/window_skill.cpp | 2 +- 5 files changed, 89 insertions(+), 54 deletions(-) diff --git a/src/algo.cpp b/src/algo.cpp index 18ad983503..c08b0438a6 100644 --- a/src/algo.cpp +++ b/src/algo.cpp @@ -18,10 +18,16 @@ #include "game_battler.h" #include "game_actor.h" #include "game_enemy.h" +#include "game_system.h" +#include "main_data.h" +#include "game_player.h" +#include "game_targets.h" +#include "game_battle.h" #include "attribute.h" #include "player.h" #include "rand.h" #include +#include #include @@ -257,4 +263,50 @@ int CalcSkillCost(const lcf::rpg::Skill& skill, int max_sp, bool half_sp_cost) { ? max_sp * skill.sp_percent / 100 / div : (skill.sp_cost + static_cast(half_sp_cost)) / div; } + +bool IsSkillUsable(const lcf::rpg::Skill& skill, + bool require_states_persist) +{ + const auto in_battle = Game_Battle::IsBattleRunning(); + + if (skill.type == lcf::rpg::Skill::Type_escape) { + return !in_battle && Main_Data::game_system->GetAllowEscape() && Main_Data::game_targets->HasEscapeTarget() && !Main_Data::game_player->IsFlying(); + } + + if (skill.type == lcf::rpg::Skill::Type_teleport) { + return !in_battle && Main_Data::game_system->GetAllowTeleport() && Main_Data::game_targets->HasTeleportTargets() && !Main_Data::game_player->IsFlying(); + } + + if (skill.type == lcf::rpg::Skill::Type_switch) { + return in_battle ? skill.occasion_battle : skill.occasion_field; + } + + if (in_battle) { + return true; + } + + if (skill.scope == lcf::rpg::Skill::Scope_enemy + || skill.scope == lcf::rpg::Skill::Scope_enemies) { + return false; + } + + if (skill.affect_hp || skill.affect_sp) { + return true; + } + + bool affects_state = false; + for (int i = 0; i < static_cast(skill.state_effects.size()); ++i) { + const bool inflict = skill.state_effects[i]; + if (inflict) { + const auto* state = lcf::ReaderUtil::GetElement(lcf::Data::states, i + 1); + if (state && (!require_states_persist || state->type == lcf::rpg::State::Persistence_persists)) { + affects_state = true; + break; + } + } + } + + return affects_state; +} + } // namespace Algo diff --git a/src/algo.h b/src/algo.h index 821623afc9..d4c13af8fb 100644 --- a/src/algo.h +++ b/src/algo.h @@ -181,6 +181,16 @@ int CalcSelfDestructEffect(const Game_Battler& source, */ int CalcSkillCost(const lcf::rpg::Skill& skill, int max_sp, bool half_sp_cost); +/* + * Determine whether a skill is usable. + * + * @param skill the skill to check + * @param require_states_persist If we should require persistent states for non-battle. + * @return Whether the skill can be used. + */ +bool IsSkillUsable(const lcf::rpg::Skill& skill, + bool require_states_persist); + } // namespace Algo diff --git a/src/game_actor.cpp b/src/game_actor.cpp index e0cf538213..e082fcc8bf 100644 --- a/src/game_actor.cpp +++ b/src/game_actor.cpp @@ -189,20 +189,22 @@ bool Game_Actor::IsSkillUsable(int skill_id) const { return false; } - // Actor must have all attributes of the skill equipped as weapons - const lcf::rpg::Item* item = GetEquipment(lcf::rpg::Item::Type_weapon); - const lcf::rpg::Item* item2 = HasTwoWeapons() ? GetEquipment(lcf::rpg::Item::Type_weapon + 1) : nullptr; - - for (size_t i = 0; i < skill->attribute_effects.size(); ++i) { - bool required = skill->attribute_effects[i] && lcf::Data::attributes[i].type == lcf::rpg::Attribute::Type_physical; - if (required) { - if (item && i < item->attribute_set.size() && item->attribute_set[i]) { - continue; - } - if (item2 && i < item2->attribute_set.size() && item2->attribute_set[i]) { - continue; + if (!skill->affect_attr_defence) { + // Actor must have all attributes of the skill equipped as weapons + const auto* w1 = GetWeapon(); + const auto* w2 = Get2ndWeapon(); + + for (size_t i = 0; i < skill->attribute_effects.size(); ++i) { + bool required = skill->attribute_effects[i] && lcf::Data::attributes[i].type == lcf::rpg::Attribute::Type_physical; + if (required) { + if (w1 && i < w1->attribute_set.size() && w1->attribute_set[i]) { + continue; + } + if (w2 && i < w2->attribute_set.size() && w2->attribute_set[i]) { + continue; + } + return false; } - return false; } } diff --git a/src/game_battler.cpp b/src/game_battler.cpp index b8b006c565..c7b3c746e2 100644 --- a/src/game_battler.cpp +++ b/src/game_battler.cpp @@ -28,6 +28,7 @@ #include "game_battle.h" #include "game_party.h" #include "game_party_base.h" +#include "game_player.h" #include "game_switches.h" #include "game_system.h" #include "game_targets.h" @@ -129,49 +130,19 @@ bool Game_Battler::IsSkillUsable(int skill_id) const { return false; } - if (skill->type == lcf::rpg::Skill::Type_escape) { - return !Game_Battle::IsBattleRunning() && Main_Data::game_system->GetAllowEscape() && Main_Data::game_targets->HasEscapeTarget(); - } - - if (skill->type == lcf::rpg::Skill::Type_teleport) { - return !Game_Battle::IsBattleRunning() && Main_Data::game_system->GetAllowTeleport() && Main_Data::game_targets->HasTeleportTargets(); - } - - if (skill->type == lcf::rpg::Skill::Type_switch) { - if (Game_Battle::IsBattleRunning()) { - return skill->occasion_battle; - } else { - return skill->occasion_field; - } - } - - // > 10 makes any skill usable - int32_t smallest_physical_rate = 11; - int32_t smallest_magical_rate = 11; - - const std::vector states = GetInflictedStates(); - for (std::vector::const_iterator it = states.begin(); - it != states.end(); ++it) { - // States are guaranteed to be valid - const lcf::rpg::State& state = *lcf::ReaderUtil::GetElement(lcf::Data::states, (*it)); - - if (state.restrict_skill) { - smallest_physical_rate = std::min(state.restrict_skill_level, smallest_physical_rate); - } - - if (state.restrict_magic) { - smallest_magical_rate = std::min(state.restrict_magic_level, smallest_magical_rate); + for (auto state_id: GetInflictedStates()) { + const auto* state = lcf::ReaderUtil::GetElement(lcf::Data::states, state_id); + if (state) { + if (state->restrict_skill && skill->physical_rate >= state->restrict_skill_level) { + return false; + } + if (state->restrict_magic && skill->magical_rate >= state->restrict_magic_level) { + return false; + } } } - if (skill->physical_rate >= smallest_physical_rate) { - return false; - } - if (skill->magical_rate >= smallest_magical_rate) { - return false; - } - - return true; + return Algo::IsSkillUsable(*skill, true); } bool Game_Battler::UseItem(int item_id, const Game_Battler* source) { diff --git a/src/window_skill.cpp b/src/window_skill.cpp index 44f1db017e..9155577ce2 100644 --- a/src/window_skill.cpp +++ b/src/window_skill.cpp @@ -121,7 +121,7 @@ bool Window_Skill::CheckInclude(int skill_id) { bool Window_Skill::CheckEnable(int skill_id) { const Game_Actor* actor = Main_Data::game_actors->GetActor(actor_id); - return actor->IsSkillLearned(skill_id) && Main_Data::game_party->IsSkillUsable(skill_id, actor); + return actor->IsSkillLearned(skill_id) && actor->IsSkillUsable(skill_id); } void Window_Skill::SetSubsetFilter(int subset) { From a6634363d1b089ede7ac94f1e4b3583c74e0b014 Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Sun, 4 Oct 2020 20:19:44 -0400 Subject: [PATCH 2/5] IsItemUsable() - use Algo::IsSkillUsable --- src/game_party.cpp | 60 ++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/src/game_party.cpp b/src/game_party.cpp index d22da0ca9e..cbd0884b48 100644 --- a/src/game_party.cpp +++ b/src/game_party.cpp @@ -30,6 +30,7 @@ #include "game_system.h" #include #include "output.h" +#include "algo.h" Game_Party::Game_Party() { } @@ -258,45 +259,30 @@ bool Game_Party::IsItemUsable(int item_id, const Game_Actor* target) const { return false; } - switch (item->type) { - case lcf::rpg::Item::Type_weapon: - case lcf::rpg::Item::Type_shield: - case lcf::rpg::Item::Type_armor: - case lcf::rpg::Item::Type_helmet: - case lcf::rpg::Item::Type_accessory: - if (item->use_skill) { - auto* skill = lcf::ReaderUtil::GetElement(lcf::Data::skills, item->skill_id); - if (skill && ( - skill->type == lcf::rpg::Skill::Type_escape - || skill->type == lcf::rpg::Skill::Type_teleport - || skill->type == lcf::rpg::Skill::Type_switch - ) - ) { - return false; - } - return IsSkillUsable(item->skill_id, nullptr, true); - } - return false; - case lcf::rpg::Item::Type_special: - return IsSkillUsable(item->skill_id, nullptr, true); + const auto* skill = lcf::ReaderUtil::GetElement(lcf::Data::skills, item->skill_id); + const bool in_battle = Game_Battle::IsBattleRunning(); + + if (item->use_skill) { + // RPG_RT BUG: Does not check if skill is usable. + return skill && + (in_battle + || skill->scope == lcf::rpg::Skill::Scope_self + || skill->scope == lcf::rpg::Skill::Scope_ally + || skill->scope == lcf::rpg::Skill::Scope_party); } - if (Game_Battle::IsBattleRunning()) { - switch (item->type) { - case lcf::rpg::Item::Type_medicine: - return !item->occasion_field1; - case lcf::rpg::Item::Type_switch: - return item->occasion_battle; - } - } else { - switch (item->type) { - case lcf::rpg::Item::Type_medicine: - case lcf::rpg::Item::Type_material: - case lcf::rpg::Item::Type_book: - return true; - case lcf::rpg::Item::Type_switch: - return item->occasion_field2; - } + switch (item->type) { + case lcf::rpg::Item::Type_medicine: + return !in_battle || !item->occasion_field1; + case lcf::rpg::Item::Type_material: + case lcf::rpg::Item::Type_book: + return !in_battle; + case lcf::rpg::Item::Type_switch: + return in_battle ? item->occasion_battle : item->occasion_field2; + case lcf::rpg::Item::Type_special: + return skill && Algo::IsSkillUsable(*skill, false); + default: + break; } return false; From 163d2ba94e0e3e76068ffbe2fa99684811487212 Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Sun, 4 Oct 2020 22:19:39 -0400 Subject: [PATCH 3/5] Remove assert on escape/teleport skills in battle These can be triggered by equipment which invokes skills, and they do nothing --- src/game_battlealgorithm.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/game_battlealgorithm.cpp b/src/game_battlealgorithm.cpp index 65aa8d56f4..b24e32c4e5 100644 --- a/src/game_battlealgorithm.cpp +++ b/src/game_battlealgorithm.cpp @@ -1154,7 +1154,6 @@ bool Game_BattleAlgorithm::Skill::Execute() { if (!(skill.type == lcf::rpg::Skill::Type_normal || skill.type >= lcf::rpg::Skill::Type_subskill)) { - assert(false && "Unsupported skill type"); this->success = false; return this->success; } From 6f7f52f90959ee8040c320636f6f8c32d1ce7c1e Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Sun, 4 Oct 2020 23:12:45 -0400 Subject: [PATCH 4/5] Add additional party usability checks required for skill items --- src/algo.h | 12 ++++++++++++ src/game_party.cpp | 20 +++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/algo.h b/src/algo.h index d4c13af8fb..fa2997d0d9 100644 --- a/src/algo.h +++ b/src/algo.h @@ -21,6 +21,7 @@ #include #include #include +#include #include "game_battler.h" class Game_Actor; @@ -191,6 +192,17 @@ int CalcSkillCost(const lcf::rpg::Skill& skill, int max_sp, bool half_sp_cost); bool IsSkillUsable(const lcf::rpg::Skill& skill, bool require_states_persist); +/** + * Checks if the skill is a normal or subskill type. + * + * @param skill the skill to check + * @return true if a normal skill or a 2k3 subskill. + */ +inline bool IsNormalOrSubskill(const lcf::rpg::Skill& skill) { + return skill.type == lcf::rpg::Skill::Type_normal + || skill.type >= lcf::rpg::Skill::Type_subskill; +} + } // namespace Algo diff --git a/src/game_party.cpp b/src/game_party.cpp index cbd0884b48..1c60f0beaf 100644 --- a/src/game_party.cpp +++ b/src/game_party.cpp @@ -255,10 +255,6 @@ bool Game_Party::IsItemUsable(int item_id, const Game_Actor* target) const { return false; } - if (data.party.size() == 0) { - return false; - } - const auto* skill = lcf::ReaderUtil::GetElement(lcf::Data::skills, item->skill_id); const bool in_battle = Game_Battle::IsBattleRunning(); @@ -280,7 +276,21 @@ bool Game_Party::IsItemUsable(int item_id, const Game_Actor* target) const { case lcf::rpg::Item::Type_switch: return in_battle ? item->occasion_battle : item->occasion_field2; case lcf::rpg::Item::Type_special: - return skill && Algo::IsSkillUsable(*skill, false); + if (skill && Algo::IsSkillUsable(*skill, false)) { + // RPG_RT requires one actor in the party and alive who can use the item. + // But only if the item invokes a normal or subskill. This check is + // not performed for escape, teleport, or switch skills! + if (!Algo::IsNormalOrSubskill(*skill)) { + return true; + } else { + for (auto* actor: GetActors()) { + if (actor->CanAct() && actor->IsItemUsable(item_id)) { + return true; + } + } + } + } + return false; default: break; } From 3cf4bd668bc2f62032df4e6e71d27f23fd0f363d Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Sun, 4 Oct 2020 23:17:26 -0400 Subject: [PATCH 5/5] Use Algo::IsNormalOrSubskill everywhere --- src/game_battlealgorithm.cpp | 3 +-- src/game_battler.cpp | 2 +- src/game_party.cpp | 3 +-- src/scene_battle.cpp | 2 -- src/scene_skill.cpp | 3 ++- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/game_battlealgorithm.cpp b/src/game_battlealgorithm.cpp index b24e32c4e5..f5782580e8 100644 --- a/src/game_battlealgorithm.cpp +++ b/src/game_battlealgorithm.cpp @@ -1152,8 +1152,7 @@ bool Game_BattleAlgorithm::Skill::Execute() { return this->success; } - if (!(skill.type == lcf::rpg::Skill::Type_normal || - skill.type >= lcf::rpg::Skill::Type_subskill)) { + if (!Algo::IsNormalOrSubskill(skill)) { this->success = false; return this->success; } diff --git a/src/game_battler.cpp b/src/game_battler.cpp index c7b3c746e2..85e5b172ae 100644 --- a/src/game_battler.cpp +++ b/src/game_battler.cpp @@ -227,7 +227,7 @@ bool Game_Battler::UseSkill(int skill_id, const Game_Battler* source) { bool was_used = false; int revived = 0; - if (skill->type == lcf::rpg::Skill::Type_normal || skill->type >= lcf::rpg::Skill::Type_subskill) { + if (Algo::IsNormalOrSubskill(*skill)) { // Only takes care of healing skills outside of battle, // the other skill logic is in Game_BattleAlgorithm diff --git a/src/game_party.cpp b/src/game_party.cpp index 1c60f0beaf..654050917e 100644 --- a/src/game_party.cpp +++ b/src/game_party.cpp @@ -374,8 +374,7 @@ bool Game_Party::IsSkillUsable(int skill_id, const Game_Actor* target, bool from return !Game_Battle::IsBattleRunning() && Main_Data::game_system->GetAllowEscape() && Main_Data::game_targets->HasEscapeTarget() && !Main_Data::game_player->IsFlying(); } else if (skill->type == lcf::rpg::Skill::Type_teleport) { return !Game_Battle::IsBattleRunning() && Main_Data::game_system->GetAllowTeleport() && Main_Data::game_targets->HasTeleportTargets() && !Main_Data::game_player->IsFlying(); - } else if (skill->type == lcf::rpg::Skill::Type_normal || - skill->type >= lcf::rpg::Skill::Type_subskill) { + } else if (Algo::IsNormalOrSubskill(*skill)) { int scope = skill->scope; if (Game_Battle::IsBattleRunning()) { diff --git a/src/scene_battle.cpp b/src/scene_battle.cpp index 93fc99a607..8355852747 100644 --- a/src/scene_battle.cpp +++ b/src/scene_battle.cpp @@ -439,8 +439,6 @@ void Scene_Battle::AssignSkill(const lcf::rpg::Skill* skill, const lcf::rpg::Ite ActionSelectedCallback(active_actor); return; } - case lcf::rpg::Skill::Type_normal: - case lcf::rpg::Skill::Type_subskill: default: break; } diff --git a/src/scene_skill.cpp b/src/scene_skill.cpp index 8f51bfd6af..0040c38882 100644 --- a/src/scene_skill.cpp +++ b/src/scene_skill.cpp @@ -17,6 +17,7 @@ // Headers #include "scene_skill.h" +#include "algo.h" #include "game_map.h" #include "game_party.h" #include "game_player.h" @@ -65,7 +66,7 @@ void Scene_Skill::Update() { Main_Data::game_party->UseSkill(skill_id, actor, actor); Scene::PopUntil(Scene::Map); Game_Map::SetNeedRefresh(true); - } else if (skill->type == lcf::rpg::Skill::Type_normal || skill->type >= lcf::rpg::Skill::Type_subskill) { + } else if (Algo::IsNormalOrSubskill(*skill)) { Main_Data::game_system->SePlay(Main_Data::game_system->GetSystemSE(Main_Data::game_system->SFX_Decision)); Scene::Push(std::make_shared(skill_id, actor_index)); skill_index = skill_window->GetIndex();