diff --git a/AI/Nullkiller/AIGateway.cpp b/AI/Nullkiller/AIGateway.cpp index bdb25982d..a32bee870 100644 --- a/AI/Nullkiller/AIGateway.cpp +++ b/AI/Nullkiller/AIGateway.cpp @@ -1081,7 +1081,7 @@ void AIGateway::recruitCreatures(const CGDwelling * d, const CArmedInstance * re void AIGateway::battleStart(const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed) { NET_EVENT_HANDLER; - assert(playerID > PlayerColor::PLAYER_LIMIT || status.getBattle() == UPCOMING_BATTLE); + assert(!playerID.isValidPlayer() || status.getBattle() == UPCOMING_BATTLE); status.setBattle(ONGOING_BATTLE); const CGObjectInstance * presumedEnemy = vstd::backOrNull(cb->getVisitableObjs(tile)); //may be nullptr in some very are cases -> eg. visited monolith and fighting with an enemy at the FoW covered exit battlename = boost::str(boost::format("Starting battle of %s attacking %s at %s") % (hero1 ? hero1->getNameTranslated() : "a army") % (presumedEnemy ? presumedEnemy->getObjectName() : "unknown enemy") % tile.toString()); diff --git a/AI/Nullkiller/Engine/FuzzyHelper.cpp b/AI/Nullkiller/Engine/FuzzyHelper.cpp index 2757cb35a..e550c47ba 100644 --- a/AI/Nullkiller/Engine/FuzzyHelper.cpp +++ b/AI/Nullkiller/Engine/FuzzyHelper.cpp @@ -111,7 +111,7 @@ ui64 FuzzyHelper::evaluateDanger(const CGObjectInstance * obj) { auto cb = ai->cb.get(); - if(obj->tempOwner < PlayerColor::PLAYER_LIMIT && cb->getPlayerRelations(obj->tempOwner, ai->playerID) != PlayerRelations::ENEMIES) //owned or allied objects don't pose any threat + if(obj->tempOwner.isValidPlayer() && cb->getPlayerRelations(obj->tempOwner, ai->playerID) != PlayerRelations::ENEMIES) //owned or allied objects don't pose any threat return 0; switch(obj->ID) diff --git a/AI/VCAI/FuzzyHelper.cpp b/AI/VCAI/FuzzyHelper.cpp index 78595c49c..c125e2315 100644 --- a/AI/VCAI/FuzzyHelper.cpp +++ b/AI/VCAI/FuzzyHelper.cpp @@ -282,7 +282,7 @@ ui64 FuzzyHelper::evaluateDanger(const CGObjectInstance * obj, const VCAI * ai) { auto cb = ai->myCb; - if(obj->tempOwner < PlayerColor::PLAYER_LIMIT && cb->getPlayerRelations(obj->tempOwner, ai->playerID) != PlayerRelations::ENEMIES) //owned or allied objects don't pose any threat + if(obj->tempOwner.isValidPlayer() && cb->getPlayerRelations(obj->tempOwner, ai->playerID) != PlayerRelations::ENEMIES) //owned or allied objects don't pose any threat return 0; switch(obj->ID) diff --git a/AI/VCAI/VCAI.cpp b/AI/VCAI/VCAI.cpp index e59341a6f..a66152e58 100644 --- a/AI/VCAI/VCAI.cpp +++ b/AI/VCAI/VCAI.cpp @@ -1578,7 +1578,7 @@ void VCAI::completeGoal(Goals::TSubgoal goal) void VCAI::battleStart(const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed) { NET_EVENT_HANDLER; - assert(playerID > PlayerColor::PLAYER_LIMIT || status.getBattle() == UPCOMING_BATTLE); + assert(!playerID.isValidPlayer() || status.getBattle() == UPCOMING_BATTLE); status.setBattle(ONGOING_BATTLE); const CGObjectInstance * presumedEnemy = vstd::backOrNull(cb->getVisitableObjs(tile)); //may be nullptr in some very are cases -> eg. visited monolith and fighting with an enemy at the FoW covered exit battlename = boost::str(boost::format("Starting battle of %s attacking %s at %s") % (hero1 ? hero1->getNameTranslated() : "a army") % (presumedEnemy ? presumedEnemy->getObjectName() : "unknown enemy") % tile.toString()); diff --git a/client/Client.cpp b/client/Client.cpp index c650d3f4e..8094d1491 100644 --- a/client/Client.cpp +++ b/client/Client.cpp @@ -570,7 +570,7 @@ void CClient::battleStarted(const BattleInfo * info) for(auto & battleCb : battleCallbacks) { if(vstd::contains_if(info->sides, [&](const SideInBattle& side) {return side.color == battleCb.first; }) - || battleCb.first >= PlayerColor::PLAYER_LIMIT) + || !battleCb.first.isValidPlayer()) { battleCb.second->setBattle(info); } diff --git a/client/NetPacksClient.cpp b/client/NetPacksClient.cpp index ac94b9150..b36d8c81a 100644 --- a/client/NetPacksClient.cpp +++ b/client/NetPacksClient.cpp @@ -570,7 +570,7 @@ void ApplyClientNetPackVisitor::visitSetHeroesInTown(SetHeroesInTown & pack) //inform all players that see this object for(auto i = cl.playerint.cbegin(); i != cl.playerint.cend(); ++i) { - if(i->first >= PlayerColor::PLAYER_LIMIT) + if(!i->first.isValidPlayer()) continue; if(gs.isVisible(t, i->first) || diff --git a/client/adventureMap/CMinimap.cpp b/client/adventureMap/CMinimap.cpp index 9dc5da7a7..7165b9e6e 100644 --- a/client/adventureMap/CMinimap.cpp +++ b/client/adventureMap/CMinimap.cpp @@ -46,7 +46,7 @@ ColorRGBA CMinimapInstance::getTileColor(const int3 & pos) const if(player == PlayerColor::NEUTRAL) return graphics->neutralColor; - if (player < PlayerColor::PLAYER_LIMIT) + if (player.isValidPlayer()) return graphics->playerColors[player.getNum()]; } diff --git a/client/lobby/SelectionTab.cpp b/client/lobby/SelectionTab.cpp index 5b69ff3dc..7399c79dd 100644 --- a/client/lobby/SelectionTab.cpp +++ b/client/lobby/SelectionTab.cpp @@ -886,7 +886,7 @@ Canvas SelectionTab::CMapInfoTooltipBox::createMinimapForLayer(std::unique_ptrneutralColor; break; } - if (player < PlayerColor::PLAYER_LIMIT) + if (player.isValidPlayer()) { color = graphics->playerColors[player.getNum()]; break; diff --git a/client/render/Graphics.cpp b/client/render/Graphics.cpp index 6b40e7edc..ea9b55122 100644 --- a/client/render/Graphics.cpp +++ b/client/render/Graphics.cpp @@ -139,7 +139,7 @@ void Graphics::blueToPlayersAdv(SDL_Surface * sur, PlayerColor player) if(sur->format->palette) { SDL_Color palette[32]; - if(player < PlayerColor::PLAYER_LIMIT) + if(player.isValidPlayer()) { for(int i=0; i<32; ++i) palette[i] = CSDL_Ext::toSDL(playerColorPalette[player][i]); diff --git a/client/renderSDL/SDLImage.cpp b/client/renderSDL/SDLImage.cpp index c04a1aa81..1778bbad6 100644 --- a/client/renderSDL/SDLImage.cpp +++ b/client/renderSDL/SDLImage.cpp @@ -247,7 +247,7 @@ void SDLImage::setBlitMode(EImageBlitMode mode) void SDLImage::setFlagColor(PlayerColor player) { - if(player < PlayerColor::PLAYER_LIMIT || player==PlayerColor::NEUTRAL) + if(player.isValidPlayer() || player==PlayerColor::NEUTRAL) CSDL_Ext::setPlayerColor(surf, player); } diff --git a/lib/IGameCallback.cpp b/lib/IGameCallback.cpp index 76c3c10fd..4efa55cab 100644 --- a/lib/IGameCallback.cpp +++ b/lib/IGameCallback.cpp @@ -82,7 +82,7 @@ void CPrivilegedInfoCallback::getTilesInRange(std::unordered_set & tiles, int mode, int3::EDistanceFormula distanceFormula) const { - if(!!player && *player >= PlayerColor::PLAYER_LIMIT) + if(!!player && !player->isValidPlayer()) { logGlobal->error("Illegal call to getTilesInRange!"); return; @@ -114,7 +114,7 @@ void CPrivilegedInfoCallback::getTilesInRange(std::unordered_set & tiles, void CPrivilegedInfoCallback::getAllTiles(std::unordered_set & tiles, std::optional Player, int level, MapTerrainFilterMode tileFilterMode) const { - if(!!Player && *Player >= PlayerColor::PLAYER_LIMIT) + if(!!Player && !Player->isValidPlayer()) { logGlobal->error("Illegal call to getAllTiles !"); return; diff --git a/lib/NetPacksLib.cpp b/lib/NetPacksLib.cpp index 33706619d..b4668a621 100644 --- a/lib/NetPacksLib.cpp +++ b/lib/NetPacksLib.cpp @@ -770,7 +770,7 @@ void LobbyShowMessage::visitTyped(ICPackVisitor & visitor) void SetResources::applyGs(CGameState * gs) const { - assert(player < PlayerColor::PLAYER_LIMIT); + assert(player.isValidPlayer()); if(abs) gs->getPlayerState(player)->resources = res; else @@ -2018,7 +2018,7 @@ void NewTurn::applyGs(CGameState *gs) for(const auto & re : res) { - assert(re.first < PlayerColor::PLAYER_LIMIT); + assert(re.first.isValidPlayer()); gs->getPlayerState(re.first)->resources = re.second; } @@ -2058,7 +2058,7 @@ void SetObjectProperty::applyGs(CGameState * gs) const if(state->towns.empty()) *state->daysWithoutCastle = 0; } - if(val < PlayerColor::PLAYER_LIMIT_I) + if(PlayerColor(val).isValidPlayer()) { PlayerState * p = gs->getPlayerState(PlayerColor(val)); p->towns.emplace_back(t); diff --git a/lib/StartInfo.cpp b/lib/StartInfo.cpp index cc68d451a..5625807ff 100644 --- a/lib/StartInfo.cpp +++ b/lib/StartInfo.cpp @@ -203,7 +203,7 @@ PlayerInfo & LobbyInfo::getPlayerInfo(int color) TeamID LobbyInfo::getPlayerTeamId(const PlayerColor & color) { - if(color < PlayerColor::PLAYER_LIMIT) + if(color.isValidPlayer()) return getPlayerInfo(color.getNum()).team; else return TeamID::NO_TEAM; diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp index eb6cd3627..d5a714dcb 100644 --- a/lib/battle/BattleInfo.cpp +++ b/lib/battle/BattleInfo.cpp @@ -60,8 +60,7 @@ void BattleInfo::calculateCasualties(std::map * casualties) const CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position) { PlayerColor owner = sides[side].color; - assert((owner >= PlayerColor::PLAYER_LIMIT) || - (base.armyObj && base.armyObj->tempOwner == owner)); + assert(!owner.isValidPlayer() || (base.armyObj && base.armyObj->tempOwner == owner)); auto * ret = new CStack(&base, owner, id, side, slot); ret->initialPosition = getAvaliableHex(base.getCreatureID(), side, position); //TODO: what if no free tile on battlefield was found? diff --git a/lib/constants/EntityIdentifiers.cpp b/lib/constants/EntityIdentifiers.cpp index 37815f8ab..1f7a6b9d6 100644 --- a/lib/constants/EntityIdentifiers.cpp +++ b/lib/constants/EntityIdentifiers.cpp @@ -47,12 +47,12 @@ const SlotID SlotID::SUMMONED_SLOT_PLACEHOLDER = SlotID(-3); const SlotID SlotID::WAR_MACHINES_SLOT = SlotID(-4); const SlotID SlotID::ARROW_TOWERS_SLOT = SlotID(-5); -const PlayerColor PlayerColor::SPECTATOR = PlayerColor(252); -const PlayerColor PlayerColor::CANNOT_DETERMINE = PlayerColor(253); -const PlayerColor PlayerColor::UNFLAGGABLE = PlayerColor(254); -const PlayerColor PlayerColor::NEUTRAL = PlayerColor(255); +const PlayerColor PlayerColor::SPECTATOR = PlayerColor(-4); +const PlayerColor PlayerColor::CANNOT_DETERMINE = PlayerColor(-3); +const PlayerColor PlayerColor::UNFLAGGABLE = PlayerColor(-2); +const PlayerColor PlayerColor::NEUTRAL = PlayerColor(-1); const PlayerColor PlayerColor::PLAYER_LIMIT = PlayerColor(PLAYER_LIMIT_I); -const TeamID TeamID::NO_TEAM = TeamID(255); +const TeamID TeamID::NO_TEAM = TeamID(-1); const SpellSchool SpellSchool::ANY = -1; const SpellSchool SpellSchool::AIR = 0; @@ -194,12 +194,12 @@ std::string SpellIDBase::encode(const si32 index) bool PlayerColor::isValidPlayer() const { - return num < PLAYER_LIMIT_I; + return num >= 0 && num < PLAYER_LIMIT_I; } bool PlayerColor::isSpectator() const { - return num == 252; + return num == SPECTATOR.num; } std::string PlayerColor::getStr(bool L10n) const diff --git a/lib/gameState/CGameState.cpp b/lib/gameState/CGameState.cpp index df831c513..c81342752 100644 --- a/lib/gameState/CGameState.cpp +++ b/lib/gameState/CGameState.cpp @@ -184,9 +184,9 @@ std::pair CGameState::pickObject (CGObjectInstance *obj) { PlayerColor align = (dynamic_cast(obj))->alignmentToPlayer; si32 f; // can be negative (for random) - if(align >= PlayerColor::PLAYER_LIMIT) //same as owner / random + if(!align.isValidPlayer()) //same as owner / random { - if(obj->tempOwner >= PlayerColor::PLAYER_LIMIT) + if(!obj->tempOwner.isValidPlayer()) f = -1; //random else f = scenarioOps->getIthPlayersSettings(obj->tempOwner).castle; @@ -1699,7 +1699,7 @@ PlayerColor CGameState::checkForStandardWin() const TeamID winnerTeam = TeamID::NO_TEAM; for(const auto & elem : players) { - if(elem.second.status == EPlayerStatus::INGAME && elem.first < PlayerColor::PLAYER_LIMIT) + if(elem.second.status == EPlayerStatus::INGAME && elem.first.isValidPlayer()) { if(supposedWinner == PlayerColor::NEUTRAL) { diff --git a/lib/mapObjects/CArmedInstance.cpp b/lib/mapObjects/CArmedInstance.cpp index fc54bb44a..37f7fd950 100644 --- a/lib/mapObjects/CArmedInstance.cpp +++ b/lib/mapObjects/CArmedInstance.cpp @@ -143,7 +143,7 @@ void CArmedInstance::armyChanged() CBonusSystemNode & CArmedInstance::whereShouldBeAttached(CGameState * gs) { - if(tempOwner < PlayerColor::PLAYER_LIMIT) + if(tempOwner.isValidPlayer()) if(auto * where = gs->getPlayerState(tempOwner)) return *where; diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index cf2373dfb..9363a29cb 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/lib/mapObjects/CGTownInstance.cpp @@ -489,7 +489,7 @@ void CGTownInstance::newTurn(CRandomGenerator & rand) const //give resources if there's a Mystic Pond if (hasBuilt(BuildingSubID::MYSTIC_POND) && cb->getDate(Date::DAY) != 1 - && (tempOwner < PlayerColor::PLAYER_LIMIT) + && (tempOwner.isValidPlayer()) ) { int resID = rand.nextInt(2, 5); //bonus to random rare resource diff --git a/lib/mapObjects/MiscObjects.cpp b/lib/mapObjects/MiscObjects.cpp index 2779765d9..24c32a4e4 100644 --- a/lib/mapObjects/MiscObjects.cpp +++ b/lib/mapObjects/MiscObjects.cpp @@ -1568,7 +1568,7 @@ void CGLighthouse::onHeroVisit( const CGHeroInstance * h ) const h->showInfoDialog(69); giveBonusTo(h->tempOwner); - if(oldOwner < PlayerColor::PLAYER_LIMIT) //remove bonus from old owner + if(oldOwner.isValidPlayer()) //remove bonus from old owner { RemoveBonus rb(GiveBonus::ETarget::PLAYER); rb.whoID = oldOwner.getNum(); @@ -1581,7 +1581,7 @@ void CGLighthouse::onHeroVisit( const CGHeroInstance * h ) const void CGLighthouse::initObj(CRandomGenerator & rand) { - if(tempOwner < PlayerColor::PLAYER_LIMIT) + if(tempOwner.isValidPlayer()) { // FIXME: This is dirty hack giveBonusTo(tempOwner, true); diff --git a/lib/mapping/MapReaderH3M.cpp b/lib/mapping/MapReaderH3M.cpp index 5c8a13d97..45b418737 100644 --- a/lib/mapping/MapReaderH3M.cpp +++ b/lib/mapping/MapReaderH3M.cpp @@ -147,14 +147,14 @@ TerrainId MapReaderH3M::readTerrain() RoadId MapReaderH3M::readRoad() { RoadId result(readInt8()); - assert(result.getNum() < features.roadsCount); + assert(result.getNum() <= features.roadsCount); return result; } RiverId MapReaderH3M::readRiver() { RiverId result(readInt8()); - assert(result.getNum() < features.riversCount); + assert(result.getNum() <= features.riversCount); return result; } @@ -188,17 +188,24 @@ SpellID MapReaderH3M::readSpell32() PlayerColor MapReaderH3M::readPlayer() { - PlayerColor result(readUInt8()); - assert(result < PlayerColor::PLAYER_LIMIT || result == PlayerColor::NEUTRAL); - return result; + uint8_t value = readUInt8(); + + if (value == 255) + return PlayerColor::NEUTRAL; + + assert(value < PlayerColor::PLAYER_LIMIT_I); + return PlayerColor(value); } PlayerColor MapReaderH3M::readPlayer32() { - PlayerColor result(readInt32()); + uint32_t value = readUInt32(); - assert(result < PlayerColor::PLAYER_LIMIT || result == PlayerColor::NEUTRAL); - return result; + if (value == 255) + return PlayerColor::NEUTRAL; + + assert(value < PlayerColor::PLAYER_LIMIT_I); + return PlayerColor(value); } void MapReaderH3M::readBitmaskBuildings(std::set & dest, std::optional faction) diff --git a/mapeditor/graphics.cpp b/mapeditor/graphics.cpp index d781b680f..ce797921c 100644 --- a/mapeditor/graphics.cpp +++ b/mapeditor/graphics.cpp @@ -227,7 +227,7 @@ void Graphics::blueToPlayersAdv(QImage * sur, PlayerColor player) if(sur->format() == QImage::Format_Indexed8) { auto palette = sur->colorTable(); - if(player < PlayerColor::PLAYER_LIMIT) + if(player.isValidPlayer()) { for(int i = 0; i < 32; ++i) palette[224 + i] = playerColorPalette[player.getNum() * 32 + i]; diff --git a/mapeditor/maphandler.cpp b/mapeditor/maphandler.cpp index 9d09cea0c..f5f3790ed 100644 --- a/mapeditor/maphandler.cpp +++ b/mapeditor/maphandler.cpp @@ -419,7 +419,7 @@ QRgb MapHandler::getTileColor(int x, int y, int z) if(player == PlayerColor::NEUTRAL) return graphics->neutralColor; else - if (player < PlayerColor::PLAYER_LIMIT) + if (player.isValidPlayer()) return graphics->playerColors[player.getNum()]; } diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 74f426a4d..7c4745bd0 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -804,7 +804,7 @@ void CGameHandler::onNewTurn() setPortalDwelling(t, true, (n.specialWeek == NewTurn::PLAGUE ? true : false)); //set creatures for Portal of Summoning if (!firstTurn) - if (t->hasBuilt(BuildingSubID::TREASURY) && player < PlayerColor::PLAYER_LIMIT) + if (t->hasBuilt(BuildingSubID::TREASURY) && player.isValidPlayer()) n.res[player][EGameResID::GOLD] += hadGold.at(player)/10; //give 10% of starting gold if (!vstd::contains(n.cres, t->id)) @@ -845,7 +845,7 @@ void CGameHandler::onNewTurn() } } } - if (!firstTurn && player < PlayerColor::PLAYER_LIMIT)//not the first day and town not neutral + if (!firstTurn && player.isValidPlayer())//not the first day and town not neutral { n.res[player] = n.res[player] + t->dailyIncome(); } @@ -1312,13 +1312,13 @@ void CGameHandler::setOwner(const CGObjectInstance * obj, const PlayerColor owne const CGTownInstance * town = dynamic_cast(obj); if (town) //town captured { - if (owner < PlayerColor::PLAYER_LIMIT) //new owner is real player + if (owner.isValidPlayer()) //new owner is real player { if (town->hasBuilt(BuildingSubID::PORTAL_OF_SUMMONING)) setPortalDwelling(town, true, false); } - if (oldOwner < PlayerColor::PLAYER_LIMIT) //old owner is real player + if (oldOwner.isValidPlayer()) //old owner is real player { if (getPlayerState(oldOwner)->towns.empty() && getPlayerState(oldOwner)->status != EPlayerStatus::LOSER) //previous player lost last last town { diff --git a/server/NetPacksServer.cpp b/server/NetPacksServer.cpp index b49392edf..654f225ed 100644 --- a/server/NetPacksServer.cpp +++ b/server/NetPacksServer.cpp @@ -164,10 +164,10 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack) PlayerColor player = market->tempOwner; - if(player >= PlayerColor::PLAYER_LIMIT) + if(!player.isValidPlayer()) player = gh.getTile(market->visitablePos())->visitableObjects.back()->tempOwner; - if(player >= PlayerColor::PLAYER_LIMIT) + if(!player.isValidPlayer()) gh.throwAndComplain(&pack, "No player can use this market!"); bool allyTownSkillTrade = (pack.mode == EMarketMode::RESOURCE_SKILL && gh.getPlayerRelations(player, hero->tempOwner) == PlayerRelations::ALLIES); diff --git a/server/processors/HeroPoolProcessor.cpp b/server/processors/HeroPoolProcessor.cpp index 1881f8083..b9c9ae227 100644 --- a/server/processors/HeroPoolProcessor.cpp +++ b/server/processors/HeroPoolProcessor.cpp @@ -270,7 +270,7 @@ std::vector HeroPoolProcessor::findAvailableHeroesFor(const Pl const CHeroClass * HeroPoolProcessor::pickClassFor(bool isNative, const PlayerColor & player) { - if(player >= PlayerColor::PLAYER_LIMIT) + if(!player.isValidPlayer()) { logGlobal->error("Cannot pick hero for player %d. Wrong owner!", player.getStr()); return nullptr;