code review

config/gameConfig.json

@@ -519,10 +519,10 @@
 			/// Strength of generic creature specialties ( "creature" : "creatureName" ) per level
 			"specialtyCreatureGrowth" : 5,
 
-			/// Amount of advanced or expert skills selectable (if any)
-			"skillSelectionAmountUpgr" : 1,
-			/// Amount of basic skills selectable (if fewer advanced or expert skills than expected they will also filled up with basic skills)
-			"skillSelectionAmount" : 1
+			/// Maximum amount of skills that can be offered to hero on levelup
+			"levelupTotalSkillsAmount" : 2,
+			/// Maximum amount of advanced or expert skills that can be offered to hero on levelup
+			"levelupUpgradedSkillsAmount" : 1
 		},
 
 		"towns":

config/schemas/gameSettings.json

@@ -52,8 +52,8 @@
 				"specialtyCreatureGrowth" :       { "type" : "number" },
 				"specialtySecondarySkillGrowth" : { "type" : "number" },
 				"baseScoutingRange" :             { "type" : "number" },
-				"skillSelectionAmountUpgr" :      { "type" : "number" },
-				"skillSelectionAmount" :          { "type" : "number" }
+				"levelupTotalSkillsAmount" :      { "type" : "number" },
+				"levelupUpgradedSkillsAmount" :   { "type" : "number" }
 			}
 		},
 		"towns" : {

lib/GameSettings.cpp

@@ -86,8 +86,8 @@ const std::vector<GameSettings::SettingOption> GameSettings::settingProperties =
 		{EGameSettings::HEROES_SKILL_PER_HERO,                            "heroes",    "skillPerHero"                         },
 		{EGameSettings::HEROES_SPECIALTY_CREATURE_GROWTH,                 "heroes",    "specialtyCreatureGrowth"              },
 		{EGameSettings::HEROES_SPECIALTY_SECONDARY_SKILL_GROWTH,          "heroes",    "specialtySecondarySkillGrowth"        },
-		{EGameSettings::HEROES_SKILL_SELECTION_AMOUNT_UPGR,               "heroes",    "skillSelectionAmountUpgr"             },
-		{EGameSettings::HEROES_SKILL_SELECTION_AMOUNT,                    "heroes",    "skillSelectionAmount"                 },
+		{EGameSettings::LEVEL_UP_TOTAL_SKILLS_AMOUNT,                     "heroes",    "levelupTotalSkillsAmount"             },
+		{EGameSettings::LEVEL_UP_UPGRADED_SKILLS_AMOUNT,                  "heroes",    "levelupUpgradedSkillsAmount"          },
 		{EGameSettings::MAP_FORMAT_ARMAGEDDONS_BLADE,                     "mapFormat", "armageddonsBlade"                     },
 		{EGameSettings::MAP_FORMAT_CHRONICLES,                            "mapFormat", "chronicles"                           },
 		{EGameSettings::MAP_FORMAT_HORN_OF_THE_ABYSS,                     "mapFormat", "hornOfTheAbyss"                       },

lib/IGameSettings.h

@@ -59,8 +59,8 @@ enum class EGameSettings
 	HEROES_SKILL_PER_HERO,
 	HEROES_SPECIALTY_CREATURE_GROWTH,
 	HEROES_SPECIALTY_SECONDARY_SKILL_GROWTH,
-	HEROES_SKILL_SELECTION_AMOUNT_UPGR,
-	HEROES_SKILL_SELECTION_AMOUNT,
+	LEVEL_UP_TOTAL_SKILLS_AMOUNT,
+	LEVEL_UP_UPGRADED_SKILLS_AMOUNT,
 	INTERFACE_PLAYER_COLORED_BACKGROUND,
 	MAP_FORMAT_ARMAGEDDONS_BLADE,
 	MAP_FORMAT_CHRONICLES,

lib/mapObjects/CGHeroInstance.cpp

@@ -1413,27 +1413,18 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelupSkillCandidates(IGameRando
 		none.erase(elem.first);
 	}
 	
-	for(;;)
+	int maxUpgradedSkills = cb->getSettings().getInteger(EGameSettings::LEVEL_UP_UPGRADED_SKILLS_AMOUNT);
+	while (skills.size() < maxUpgradedSkills && !basicAndAdv.empty())
 	{
-		if(skills.size() >= cb->getSettings().getInteger(EGameSettings::HEROES_SKILL_SELECTION_AMOUNT_UPGR) || basicAndAdv.empty())
-			break;
-
-		if (!basicAndAdv.empty())
-		{
-			skills.push_back(gameRandomizer.rollSecondarySkillForLevelup(this, basicAndAdv));
-			basicAndAdv.erase(skills.back());
-		}
+		skills.push_back(gameRandomizer.rollSecondarySkillForLevelup(this, basicAndAdv));
+		basicAndAdv.erase(skills.back());
 	}
-	for(;;)
-	{
-		if(skills.size() >= cb->getSettings().getInteger(EGameSettings::HEROES_SKILL_SELECTION_AMOUNT) + cb->getSettings().getInteger(EGameSettings::HEROES_SKILL_SELECTION_AMOUNT_UPGR) || none.empty())
-			break;
 
-		if (!none.empty())
-		{
-			skills.push_back(gameRandomizer.rollSecondarySkillForLevelup(this, none));
-			none.erase(skills.back());
-		}		
+	int maxTotalSkills = cb->getSettings().getInteger(EGameSettings::LEVEL_UP_TOTAL_SKILLS_AMOUNT);
+	while (skills.size() < maxTotalSkills && !none.empty())
+	{
+		skills.push_back(gameRandomizer.rollSecondarySkillForLevelup(this, none));
+		none.erase(skills.back());
 	}
 
 	return skills;