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
EasyRPG · Oct 1, 2020 · 9e37f91 · 9e37f91
1 parent 2f80b0e
commit 9e37f91
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 54 deletions.
diff --git 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 <lcf/rpg/skill.h>
+#include <lcf/reader_util.h>
 
 #include <algorithm>
 
@@ -251,4 +257,44 @@ int CalcSelfDestructEffect(const Game_Battler& source,
 	return effect;
 }
 
+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.affect_hp || skill.affect_sp) {
+		return true;
+	}
+
+	bool affects_state = false;
+	for (int i = 0; i < static_cast<int>(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
@@ -168,6 +168,16 @@ int CalcSelfDestructEffect(const Game_Battler& source,
 		const Game_Battler& target,
 		bool apply_variance);
 
+/**
+ * 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
@@ -188,20 +188,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
@@ -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<int16_t> states = GetInflictedStates();
-	for (std::vector<int16_t>::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
@@ -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) {