From e06db2365d6be0b47892aceab9ebc07081350302 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 14 Nov 2022 17:16:49 +0200 Subject: [PATCH 1/2] Fixes #1096 - do not propose banned skills on levelup Remove possibility to get banned skill on levelup as "obligatory skill" if all other such skills have been learned before. May happen under some conditions, e.g. if hero quickly learns other magic schools - in witch hut/university, leading to case where banned skill is the only obligatory skill that can be offered on levelup --- lib/mapObjects/CGHeroInstance.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp index 4785f1eb7..be7f0efda 100644 --- a/lib/mapObjects/CGHeroInstance.cpp +++ b/lib/mapObjects/CGHeroInstance.cpp @@ -1117,7 +1117,9 @@ std::vector CGHeroInstance::getLevelUpProposedSecondarySkills() std::vector obligatorySkills; //hero is offered magic school or wisdom if possible if (!skillsInfo.wisdomCounter) { - if (cb->isAllowed(2, SecondarySkill::WISDOM) && !getSecSkillLevel(SecondarySkill::WISDOM)) + if (cb->isAllowed(2, SecondarySkill::WISDOM) && + !getSecSkillLevel(SecondarySkill::WISDOM) && + type->heroClass->secSkillProbability[SecondarySkill::WISDOM] > 0) obligatorySkills.push_back(SecondarySkill::WISDOM); } if (!skillsInfo.magicSchoolCounter) @@ -1131,7 +1133,7 @@ std::vector CGHeroInstance::getLevelUpProposedSecondarySkills() for (auto skill : ss) { - if (cb->isAllowed(2, skill) && !getSecSkillLevel(skill)) //only schools hero doesn't know yet + if (cb->isAllowed(2, skill) && !getSecSkillLevel(skill) && type->heroClass->secSkillProbability[skill] > 0) //only schools hero doesn't know yet { obligatorySkills.push_back(skill); break; //only one @@ -1143,7 +1145,7 @@ std::vector CGHeroInstance::getLevelUpProposedSecondarySkills() //picking sec. skills for choice std::set basicAndAdv, expert, none; for(int i = 0; i < VLC->skillh->size(); i++) - if (cb->isAllowed(2,i)) + if (cb->isAllowed(2,i) && type->heroClass->secSkillProbability[i] > 0) none.insert(SecondarySkill(i)); for(auto & elem : secSkills) From 729357824bf3bbcfc72af7ca2eda44884aff083d Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 14 Nov 2022 19:08:49 +0200 Subject: [PATCH 2/2] Added common method for secondary skill availability checks --- client/windows/GUIClasses.cpp | 4 +--- lib/mapObjects/CGHeroInstance.cpp | 25 ++++++++++++++++++++----- lib/mapObjects/CGHeroInstance.h | 1 + server/CGameHandler.cpp | 2 +- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/client/windows/GUIClasses.cpp b/client/windows/GUIClasses.cpp index 4bce0f2d0..9d7afa516 100644 --- a/client/windows/GUIClasses.cpp +++ b/client/windows/GUIClasses.cpp @@ -1700,9 +1700,7 @@ int CUniversityWindow::CItem::state() { if(parent->hero->getSecSkillLevel(SecondarySkill(ID)))//hero know this skill return 1; - if(!parent->hero->canLearnSkill())//can't learn more skills - return 0; - if(parent->hero->type->heroClass->secSkillProbability[ID]==0)//can't learn this skill (like necromancy for most of non-necros) + if(!parent->hero->canLearnSkill(SecondarySkill(ID)))//can't learn more skills return 0; return 2; } diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp index be7f0efda..b14473ddd 100644 --- a/lib/mapObjects/CGHeroInstance.cpp +++ b/lib/mapObjects/CGHeroInstance.cpp @@ -195,6 +195,23 @@ bool CGHeroInstance::canLearnSkill() const return secSkills.size() < GameConstants::SKILL_PER_HERO; } +bool CGHeroInstance::canLearnSkill(SecondarySkill which) const +{ + if ( !canLearnSkill()) + return false; + + if (!cb->isAllowed(2, which)) + return false; + + if (getSecSkillLevel(which) > 0) + return false; + + if (type->heroClass->secSkillProbability[which] == 0) + return false; + + return true; +} + int CGHeroInstance::maxMovePoints(bool onLand) const { TurnInfo ti(this); @@ -1117,9 +1134,7 @@ std::vector CGHeroInstance::getLevelUpProposedSecondarySkills() std::vector obligatorySkills; //hero is offered magic school or wisdom if possible if (!skillsInfo.wisdomCounter) { - if (cb->isAllowed(2, SecondarySkill::WISDOM) && - !getSecSkillLevel(SecondarySkill::WISDOM) && - type->heroClass->secSkillProbability[SecondarySkill::WISDOM] > 0) + if (canLearnSkill(SecondarySkill::WISDOM)) obligatorySkills.push_back(SecondarySkill::WISDOM); } if (!skillsInfo.magicSchoolCounter) @@ -1133,7 +1148,7 @@ std::vector CGHeroInstance::getLevelUpProposedSecondarySkills() for (auto skill : ss) { - if (cb->isAllowed(2, skill) && !getSecSkillLevel(skill) && type->heroClass->secSkillProbability[skill] > 0) //only schools hero doesn't know yet + if (canLearnSkill(skill)) //only schools hero doesn't know yet { obligatorySkills.push_back(skill); break; //only one @@ -1145,7 +1160,7 @@ std::vector CGHeroInstance::getLevelUpProposedSecondarySkills() //picking sec. skills for choice std::set basicAndAdv, expert, none; for(int i = 0; i < VLC->skillh->size(); i++) - if (cb->isAllowed(2,i) && type->heroClass->secSkillProbability[i] > 0) + if (canLearnSkill(SecondarySkill(i))) none.insert(SecondarySkill(i)); for(auto & elem : secSkills) diff --git a/lib/mapObjects/CGHeroInstance.h b/lib/mapObjects/CGHeroInstance.h index 8c56f8798..5e05eafe5 100644 --- a/lib/mapObjects/CGHeroInstance.h +++ b/lib/mapObjects/CGHeroInstance.h @@ -187,6 +187,7 @@ public: /// Returns true if hero has free secondary skill slot. bool canLearnSkill() const; + bool canLearnSkill(SecondarySkill which) const; void setPrimarySkill(PrimarySkill::PrimarySkill primarySkill, si64 value, ui8 abs); void setSecSkillLevel(SecondarySkill which, int val, bool abs);// abs == 0 - changes by value; 1 - sets to value diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index a4f8dedad..13f977817 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -4083,7 +4083,7 @@ bool CGameHandler::buySecSkill(const IMarket *m, const CGHeroInstance *h, Second if (!h->canLearnSkill()) COMPLAIN_RET("Hero can't learn any more skills"); - if (h->type->heroClass->secSkillProbability.at(skill)==0)//can't learn this skill (like necromancy for most of non-necros) + if (!h->canLearnSkill(skill)) COMPLAIN_RET("The hero can't learn this skill!"); if (!vstd::contains(m->availableItemsIds(EMarketMode::RESOURCE_SKILL), skill))