Addressed review feedback

Additional changes after incorporating PlayerColor in `Validator::Validate()`

mapeditor/graphics.cpp

@@ -49,9 +49,9 @@ void Graphics::loadPaletteAndColors()
 	{
 		QColor col;
 		// Cast to unsigned char to ensure values are in the range 0-255
-		col.setRed(static_cast<uint8_t>(pals[startPoint++]));
-		col.setGreen(static_cast<uint8_t>(pals[startPoint++]));
-		col.setBlue(static_cast<uint8_t>(pals[startPoint++]));
+		col.setRed(std::clamp(static_cast<int>(pals[startPoint++]), 0, 255));
+		col.setGreen(std::clamp(static_cast<int>(pals[startPoint++]), 0, 255));
+		col.setBlue(std::clamp(static_cast<int>(pals[startPoint++]), 0, 255));
 		col.setAlpha(255);
 		startPoint++;
 		playerColorPalette[i] = col.rgba();

mapeditor/validator.cpp

@@ -58,12 +58,12 @@ std::list<Validator::Issue> Validator::validate(const CMap * map)
 		//check player settings
 		int hplayers = 0;
 		int cplayers = 0;
-		std::map<int, int> amountOfCastles;
+		std::map<PlayerColor, int> amountOfTowns;
+		std::map<PlayerColor, int> amountOfHeroes;
+
 		for(int i = 0; i < map->players.size(); ++i)
 		{
 			auto & p = map->players[i];
-			if(p.canAnyonePlay())
-				amountOfCastles[i] = 0;
 			if(p.canComputerPlay)
 				++cplayers;
 			if(p.canHumanPlay)
@@ -100,16 +100,12 @@ std::list<Validator::Issue> Validator::validate(const CMap * map)
 				if(!map->players[o->getOwner().getNum()].canAnyonePlay())
 					issues.emplace_back(QString(tr("Object %1 is assigned to non-playable player %2")).arg(o->instanceName.c_str(), o->getOwner().toString().c_str()), true);
 			}
-			//checking towns
+			//count towns
 			if(auto * ins = dynamic_cast<CGTownInstance*>(o.get()))
 			{
-				bool has = amountOfCastles.count(ins->getOwner().getNum());
-				if(!has && ins->getOwner() != PlayerColor::NEUTRAL)
-					issues.emplace_back(tr("Town %1 has undefined owner %2").arg(ins->instanceName.c_str(), ins->getOwner().toString().c_str()), true);
-				if(has)
-					++amountOfCastles[ins->getOwner().getNum()];
+					++amountOfTowns[ins->getOwner()];
 			}
-			//checking heroes and prisons
+			//checking and counting heroes and prisons
 			if(auto * ins = dynamic_cast<CGHeroInstance*>(o.get()))
 			{
 				if(ins->ID == Obj::PRISON)
@@ -119,9 +115,10 @@ std::list<Validator::Issue> Validator::validate(const CMap * map)
 				}
 				else
 				{
-					bool has = amountOfCastles.count(ins->getOwner().getNum());
-					if(!has)
+					if(ins->getOwner() == PlayerColor::NEUTRAL)
 						issues.emplace_back(QString(tr("Hero %1 must have an owner")).arg(ins->instanceName.c_str()), true);
+
+					++amountOfHeroes[ins->getOwner()];
 				}
 				if(ins->type)
 				{
@@ -159,7 +156,7 @@ std::list<Validator::Issue> Validator::validate(const CMap * map)
 		}
 
 		//verification of starting towns
-		for(auto & mp : amountOfCastles)
+		for(auto & mp : amountOfTowns)
 			if(mp.second == 0)
 				issues.emplace_back(QString(tr("Player %1 doesn't have any starting town")).arg(mp.first), false);