From 5b74f7f19e8cae614a0bf7e47559bc9d851364bc Mon Sep 17 00:00:00 2001 From: Laserlicht <13953785+Laserlicht@users.noreply.github.com> Date: Thu, 10 Apr 2025 21:04:24 +0200 Subject: [PATCH] code review (first batch) --- client/lobby/CBonusSelection.cpp | 6 ++-- lib/campaign/CampaignHandler.cpp | 26 ++++++++-------- lib/constants/EntityIdentifiers.cpp | 4 ++- lib/constants/EntityIdentifiers.h | 7 ++++- lib/gameState/CGameStateCampaign.cpp | 10 +++---- mapeditor/BitmapHandler.cpp | 2 +- mapeditor/StdInc.h | 3 -- mapeditor/campaigneditor/campaigneditor.cpp | 7 +++-- .../campaigneditor/campaignproperties.cpp | 8 +++-- .../campaigneditor/scenarioproperties.cpp | 6 ++-- mapeditor/campaigneditor/startingbonus.cpp | 30 +++++++++---------- mapeditor/mainwindow.cpp | 2 +- 12 files changed, 59 insertions(+), 52 deletions(-) diff --git a/client/lobby/CBonusSelection.cpp b/client/lobby/CBonusSelection.cpp index 7dedec842..d5a0ade19 100644 --- a/client/lobby/CBonusSelection.cpp +++ b/client/lobby/CBonusSelection.cpp @@ -271,13 +271,13 @@ void CBonusSelection::createBonusesIcons() switch(bonDescs[i].info1) { - case 0xFD: //wood + ore + case EGameResID::COMMON: //wood + ore { desc.replaceLocalString(EMetaText::GENERAL_TXT, 721); picNumber = 7; break; } - case 0xFE: //wood + ore + case EGameResID::RARE : //mercury + sulfur + crystal + gems { desc.replaceLocalString(EMetaText::GENERAL_TXT, 722); picNumber = 8; @@ -305,7 +305,7 @@ void CBonusSelection::createBonusesIcons() } case CampaignBonusType::HERO: - if(bonDescs[i].info2 == 0xFFFF) + if(bonDescs[i].info2 == HeroTypeID::RANDOM) { desc.appendLocalString(EMetaText::GENERAL_TXT, 720); // Start with random hero picNumber = -1; diff --git a/lib/campaign/CampaignHandler.cpp b/lib/campaign/CampaignHandler.cpp index 0e2a1802c..37cd0aa70 100644 --- a/lib/campaign/CampaignHandler.cpp +++ b/lib/campaign/CampaignHandler.cpp @@ -281,23 +281,21 @@ static const std::map primarySkillsMap = { }; static const std::map heroSpecialMap = { - {"strongest", 0xFFFD}, - {"generated", 0xFFFE}, - {"random", 0xFFFF} + {"strongest", HeroTypeID::STRONGEST}, + {"generated", HeroTypeID::GENERATED}, + {"random", HeroTypeID::RANDOM} }; static const std::map resourceTypeMap = { - //FD - wood+ore - //FE - mercury+sulfur+crystal+gem - {"wood", 0}, - {"mercury", 1}, - {"ore", 2}, - {"sulfur", 3}, - {"crystal", 4}, - {"gems", 5}, - {"gold", 6}, - {"common", 0xFD}, - {"rare", 0xFE} + {"wood", EGameResID::WOOD}, + {"mercury", EGameResID::MERCURY}, + {"ore", EGameResID::ORE}, + {"sulfur", EGameResID::SULFUR}, + {"crystal", EGameResID::CRYSTAL}, + {"gems", EGameResID::GEMS}, + {"gold", EGameResID::GOLD}, + {"common", EGameResID::COMMON}, + {"rare", EGameResID::RARE} }; CampaignTravel CampaignHandler::readScenarioTravelFromJson(JsonNode & reader) diff --git a/lib/constants/EntityIdentifiers.cpp b/lib/constants/EntityIdentifiers.cpp index d57571f32..5731ff261 100644 --- a/lib/constants/EntityIdentifiers.cpp +++ b/lib/constants/EntityIdentifiers.cpp @@ -53,9 +53,11 @@ const BattleID BattleID::NONE(-1); const QueryID QueryID::NONE(-1); const QueryID QueryID::CLIENT(-2); const HeroTypeID HeroTypeID::NONE(-1); -const HeroTypeID HeroTypeID::RANDOM(-2); const HeroTypeID HeroTypeID::GEM(27); const HeroTypeID HeroTypeID::SOLMYR(45); +const HeroTypeID HeroTypeID::STRONGEST(0xFFFD); +const HeroTypeID HeroTypeID::GENERATED(0xFFFE); +const HeroTypeID HeroTypeID::RANDOM(0xFFFF); const ObjectInstanceID ObjectInstanceID::NONE(-1); diff --git a/lib/constants/EntityIdentifiers.h b/lib/constants/EntityIdentifiers.h index ac89388b1..c22298ef3 100644 --- a/lib/constants/EntityIdentifiers.h +++ b/lib/constants/EntityIdentifiers.h @@ -102,10 +102,13 @@ public: const HeroType * toEntity(const Services * services) const; static const HeroTypeID NONE; - static const HeroTypeID RANDOM; static const HeroTypeID GEM; // aka Gem, Sorceress in campaign static const HeroTypeID SOLMYR; // aka Young Yog in campaigns + static const HeroTypeID STRONGEST; + static const HeroTypeID GENERATED; + static const HeroTypeID RANDOM; + bool isValid() const { return getNum() >= 0; @@ -1043,6 +1046,8 @@ public: COUNT, WOOD_AND_ORE = 127, // special case for town bonus resource + COMMON = 0xFD, // campaign bonus + RARE = 0xFE, // campaign bonus NONE = -1 }; }; diff --git a/lib/gameState/CGameStateCampaign.cpp b/lib/gameState/CGameStateCampaign.cpp index 3cc6cc9ab..21e7ac413 100644 --- a/lib/gameState/CGameStateCampaign.cpp +++ b/lib/gameState/CGameStateCampaign.cpp @@ -209,7 +209,7 @@ void CGameStateCampaign::placeCampaignHeroes() if(it != gameState->scenarioOps->playerInfos.end()) { HeroTypeID heroTypeId = HeroTypeID(campaignBonus->info2); - if(heroTypeId.getNum() == 0xffff) // random bonus hero + if(heroTypeId.getNum() == HeroTypeID::RANDOM) // random bonus hero { heroTypeId = gameState->pickUnusedHeroTypeRandomly(playerColor); } @@ -529,7 +529,7 @@ void CGameStateCampaign::generateCampaignHeroesToReplace() void CGameStateCampaign::initHeroes() { auto chosenBonus = currentBonus(); - if (chosenBonus && chosenBonus->isBonusForHero() && chosenBonus->info1 != 0xFFFE) //exclude generated heroes + if (chosenBonus && chosenBonus->isBonusForHero() && chosenBonus->info1 != HeroTypeID::GENERATED) //exclude generated heroes { //find human player PlayerColor humanPlayer=PlayerColor::NEUTRAL; @@ -545,7 +545,7 @@ void CGameStateCampaign::initHeroes() const auto & heroes = gameState->players[humanPlayer].getHeroes(); - if (chosenBonus->info1 == 0xFFFD) //most powerful + if (chosenBonus->info1 == HeroTypeID::STRONGEST) //most powerful { int maxB = -1; for (int b=0; binfo1); break; - case 0xFD: //wood+ore + case EGameResID::COMMON: //wood+ore res.push_back(GameResID(EGameResID::WOOD)); res.push_back(GameResID(EGameResID::ORE)); break; - case 0xFE: //rare + case EGameResID::RARE: //rare res.push_back(GameResID(EGameResID::MERCURY)); res.push_back(GameResID(EGameResID::SULFUR)); res.push_back(GameResID(EGameResID::CRYSTAL)); diff --git a/mapeditor/BitmapHandler.cpp b/mapeditor/BitmapHandler.cpp index dd971b4d7..8347a8c7b 100644 --- a/mapeditor/BitmapHandler.cpp +++ b/mapeditor/BitmapHandler.cpp @@ -69,7 +69,7 @@ namespace BitmapHandler it = (int)size - 256 * 3; for(int i = 0; i < 256; i++) { - unsigned char bytes[4]; + std::array bytes; bytes[0] = pcx[it++]; bytes[1] = pcx[it++]; bytes[2] = pcx[it++]; diff --git a/mapeditor/StdInc.h b/mapeditor/StdInc.h index fd30130ef..97f5147da 100644 --- a/mapeditor/StdInc.h +++ b/mapeditor/StdInc.h @@ -11,9 +11,6 @@ #include "../Global.h" -#define VCMI_EDITOR_NAME "VCMI Map Editor" -#define VCMI_CAMP_EDITOR_NAME "VCMI Campaign Editor" - #include #include #include diff --git a/mapeditor/campaigneditor/campaigneditor.cpp b/mapeditor/campaigneditor/campaigneditor.cpp index 077115419..fe9f61431 100644 --- a/mapeditor/campaigneditor/campaigneditor.cpp +++ b/mapeditor/campaigneditor/campaigneditor.cpp @@ -124,7 +124,7 @@ bool CampaignEditor::getAnswerAboutUnsavedChanges() void CampaignEditor::setTitle() { QFileInfo fileInfo(filename); - QString title = QString("%1%2 - %3 (%4)").arg(fileInfo.fileName(), unsaved ? "*" : "", VCMI_CAMP_EDITOR_NAME, GameConstants::VCMI_VERSION.c_str()); + QString title = QString("%1%2 - %3 (%4)").arg(fileInfo.fileName(), unsaved ? "*" : "", tr("VCMI Campaign Editor"), GameConstants::VCMI_VERSION.c_str()); setWindowTitle(title); } @@ -138,8 +138,9 @@ bool CampaignEditor::saveCampaign() { if(campaignState->mapPieces.size() != campaignState->campaignRegions.regions.size()) { - QMessageBox::critical(nullptr, tr("Maps missing"), tr("Not all Regions have a map. Please add them in Scenario Properties.")); - return false; + auto reply = QMessageBox::question(nullptr, tr("Maps missing"), tr("Not all regions have a map. Do you want to continue?"), QMessageBox::Yes|QMessageBox::No); + if (reply != QMessageBox::Yes) + return false; } Helper::saveCampaign(campaignState, filename); diff --git a/mapeditor/campaigneditor/campaignproperties.cpp b/mapeditor/campaigneditor/campaignproperties.cpp index a03e101dd..1f1120dde 100644 --- a/mapeditor/campaigneditor/campaignproperties.cpp +++ b/mapeditor/campaigneditor/campaignproperties.cpp @@ -15,6 +15,7 @@ #include "../../lib/texts/CGeneralTextHandler.h" #include "../../lib/campaign/CampaignState.h" #include "../../lib/constants/StringConstants.h" +#include "../../lib/json/JsonNode.h" CampaignProperties::CampaignProperties(std::shared_ptr campaignState): ui(new Ui::CampaignProperties), @@ -36,9 +37,12 @@ CampaignProperties::CampaignProperties(std::shared_ptr campaignSt ui->lineEditMusic->setText(QString::fromStdString(campaignState->music.getName())); ui->checkBoxScenarioDifficulty->setChecked(campaignState->difficultyChosenByPlayer); - for (int i = 0; i < 20; i++) + const JsonNode legacyRegionConfig(JsonPath::builtin("config/campaign_regions.json")); + int legacyRegionNumber = legacyRegionConfig["campaign_regions"].Vector().size(); + + for (int i = 0; i < legacyRegionNumber; i++) ui->comboBoxRegionPreset->insertItem(i, QString::fromStdString(LIBRARY->generaltexth->translate("core.camptext.names", i))); - ui->comboBoxRegionPreset->insertItem(20, tr("Custom")); + ui->comboBoxRegionPreset->insertItem(legacyRegionNumber, tr("Custom")); ui->comboBoxRegionPreset->setCurrentIndex(20); loadRegion(); diff --git a/mapeditor/campaigneditor/scenarioproperties.cpp b/mapeditor/campaigneditor/scenarioproperties.cpp index e61863517..314355e02 100644 --- a/mapeditor/campaigneditor/scenarioproperties.cpp +++ b/mapeditor/campaigneditor/scenarioproperties.cpp @@ -112,9 +112,9 @@ ScenarioProperties::ScenarioProperties(std::shared_ptr campaignSt { heroSelection.emplace(hero->getId().getNum(), hero->getNameTranslated()); }); - heroSelection.emplace(0xFFFD, tr("Strongest").toStdString()); - heroSelection.emplace(0xFFFE, tr("Generated").toStdString()); - heroSelection.emplace(0xFFFF, tr("Random").toStdString()); + heroSelection.emplace(HeroTypeID::STRONGEST, tr("Strongest").toStdString()); + heroSelection.emplace(HeroTypeID::GENERATED, tr("Generated").toStdString()); + heroSelection.emplace(HeroTypeID::RANDOM, tr("Random").toStdString()); reloadMapRelatedUi(); diff --git a/mapeditor/campaigneditor/startingbonus.cpp b/mapeditor/campaigneditor/startingbonus.cpp index fcbdc1a4d..a1a9ea976 100644 --- a/mapeditor/campaigneditor/startingbonus.cpp +++ b/mapeditor/campaigneditor/startingbonus.cpp @@ -55,9 +55,9 @@ void StartingBonus::initControls() for(auto & hero : map->heroesOnMap) if(hero->getOwner() == color || color == PlayerColor::CANNOT_DETERMINE) heroSelection.emplace(hero->getHeroTypeID(), hero->getNameTranslated()); - heroSelection.emplace(0xFFFD, tr("Strongest").toStdString()); - heroSelection.emplace(0xFFFE, tr("Generated").toStdString()); - heroSelection.emplace(0xFFFF, tr("Random").toStdString()); + heroSelection.emplace(HeroTypeID::STRONGEST, tr("Strongest").toStdString()); + heroSelection.emplace(HeroTypeID::GENERATED, tr("Generated").toStdString()); + heroSelection.emplace(HeroTypeID::RANDOM, tr("Random").toStdString()); for(auto & hero : heroSelection) { @@ -106,15 +106,15 @@ void StartingBonus::initControls() ui->comboBoxSecondarySkillMastery->addItem(tr("Advanced"), QVariant(1)); ui->comboBoxSecondarySkillMastery->addItem(tr("Expert"), QVariant(2)); - ui->comboBoxResourceResourceType->addItem(tr("Wood"), QVariant(0)); - ui->comboBoxResourceResourceType->addItem(tr("Mercury"), QVariant(1)); - ui->comboBoxResourceResourceType->addItem(tr("Ore"), QVariant(2)); - ui->comboBoxResourceResourceType->addItem(tr("Sulfur"), QVariant(3)); - ui->comboBoxResourceResourceType->addItem(tr("Crystal"), QVariant(4)); - ui->comboBoxResourceResourceType->addItem(tr("Gems"), QVariant(5)); - ui->comboBoxResourceResourceType->addItem(tr("Gold"), QVariant(6)); - ui->comboBoxResourceResourceType->addItem(tr("Common (Wood and Ore)"), QVariant(0xFD)); - ui->comboBoxResourceResourceType->addItem(tr("Rare (Mercury, Sulfur, Crystal, Gems)"), QVariant(0xFE)); + ui->comboBoxResourceResourceType->addItem(tr("Wood"), QVariant(EGameResID::WOOD)); + ui->comboBoxResourceResourceType->addItem(tr("Mercury"), QVariant(EGameResID::MERCURY)); + ui->comboBoxResourceResourceType->addItem(tr("Ore"), QVariant(EGameResID::ORE)); + ui->comboBoxResourceResourceType->addItem(tr("Sulfur"), QVariant(EGameResID::SULFUR)); + ui->comboBoxResourceResourceType->addItem(tr("Crystal"), QVariant(EGameResID::CRYSTAL)); + ui->comboBoxResourceResourceType->addItem(tr("Gems"), QVariant(EGameResID::GEMS)); + ui->comboBoxResourceResourceType->addItem(tr("Gold"), QVariant(EGameResID::GOLD)); + ui->comboBoxResourceResourceType->addItem(tr("Common (Wood and Ore)"), QVariant(EGameResID::COMMON)); + ui->comboBoxResourceResourceType->addItem(tr("Rare (Mercury, Sulfur, Crystal, Gems)"), QVariant(EGameResID::RARE)); } void StartingBonus::loadBonus() @@ -280,11 +280,11 @@ QString StartingBonus::getBonusListTitle(CampaignBonus bonus, std::shared_ptr