PR review fixes:

- change return of `getBuildingVariantsFromModel` to `QVariantList`
- change while to do-while where it makes sense
- moved constants outside of methods
- made building and resources name non editable

mapeditor/inspector/townbuildingswidget.cpp

@@ -73,7 +73,7 @@ QStandardItem * getBuildingParentFromTreeModel(const CBuilding * building, QStan
 {
 	QStandardItem * parent = nullptr;
 	std::vector<QModelIndex> stack(1);
-	while (!parent && !stack.empty())
+	do
 	{
 		auto pindex = stack.back();
 		stack.pop_back();
@@ -89,15 +89,15 @@ QStandardItem * getBuildingParentFromTreeModel(const CBuilding * building, QStan
 			if (model.hasChildren(index))
 				stack.push_back(index);
 		}
-	}
+	} while(!parent && !stack.empty());
 	return parent;
 }
 
-std::list<QVariant> getBuildingVariantsFromModel(QStandardItemModel & model, int modelColumn, Qt::CheckState checkState)
+QVariantList getBuildingVariantsFromModel(QStandardItemModel & model, int modelColumn, Qt::CheckState checkState)
 {
-	std::list<QVariant> result;
+	QVariantList result;
 	std::vector<QModelIndex> stack(1);
-	while (!stack.empty())
+	do
 	{
 		auto pindex = stack.back();
 		stack.pop_back();
@@ -105,14 +105,14 @@ std::list<QVariant> getBuildingVariantsFromModel(QStandardItemModel & model, int
 		for (int i = 0; i < rowCount; ++i)
 		{
 			QModelIndex index = model.index(i, modelColumn, pindex);
-			if (auto * item = model.itemFromIndex(index))
+			auto * item = model.itemFromIndex(index);
-				if (item->checkState() == checkState)
+			if(item && item->checkState() == checkState)
-					result.push_back(item->data(MapEditorRoles::BuildingIDRole));
+				result.push_back(item->data(MapEditorRoles::BuildingIDRole));
 			index = model.index(i, 0, pindex);
 			if (model.hasChildren(index))
 				stack.push_back(index);
 		}
-	}
+	} while(!stack.empty());
 
 	return result;
 }
@@ -263,7 +263,7 @@ void TownBuildingsWidget::setRowColumnCheckState(QStandardItem * item, Column co
 void TownBuildingsWidget::setAllRowsColumnCheckState(Column column, Qt::CheckState checkState)
 {
 	std::vector<QModelIndex> stack(1);
-	while (!stack.empty())
+	do
 	{
 		auto parentIndex = stack.back();
 		stack.pop_back();
@@ -277,7 +277,7 @@ void TownBuildingsWidget::setAllRowsColumnCheckState(Column column, Qt::CheckSta
 			if (model.hasChildren(index))
 				stack.push_back(index);
 		}
-	}
+	} while(!stack.empty());
 }
 
 void TownBuildingsWidget::onItemChanged(QStandardItem * item) {
@@ -297,7 +297,8 @@ void TownBuildingsWidget::onItemChanged(QStandardItem * item) {
 	else if (item->checkState() == Qt::Unchecked) {
 		std::vector<QStandardItem*> stack;
 		stack.push_back(nextRow);
-		while (!stack.empty()) {
+		do
+		{
 			nextRow = stack.back();
 			stack.pop_back();
 			setRowColumnCheckState(nextRow, Column(item->column()), Qt::Unchecked);
@@ -310,7 +311,7 @@ void TownBuildingsWidget::onItemChanged(QStandardItem * item) {
 				}
 			}
 			
-		}
+		} while(!stack.empty());
 	}
 	connect(&model, &QStandardItemModel::itemChanged, this, &TownBuildingsWidget::onItemChanged);
 }

mapeditor/inspector/townbuildingswidget.h

@@ -21,7 +21,7 @@ std::string defaultBuildingIdConversion(BuildingID bId);
 
 QStandardItem * getBuildingParentFromTreeModel(const CBuilding * building, QStandardItemModel & model);
 
-std::list<QVariant> getBuildingVariantsFromModel(QStandardItemModel & model, int modelColumn, Qt::CheckState checkState);
+QVariantList getBuildingVariantsFromModel(QStandardItemModel & model, int modelColumn, Qt::CheckState checkState);
 
 class TownBuildingsWidget : public QDialog
 {

mapeditor/inspector/towneventdialog.cpp

@@ -18,15 +18,23 @@
 #include "../../lib/constants/NumericConstants.h"
 #include "../../lib/constants/StringConstants.h"
 
+static const int FIRST_DAY_FOR_EVENT = 1;
+static const int LAST_DAY_FOR_EVENT = 999;
+static const int MAXIMUM_EVENT_REPEAT_AFTER = 999;
+
+static const int MAXIMUM_GOLD_CHANGE = 999999;
+static const int MAXIMUM_RESOURCE_CHANGE = 999;
+static const int GOLD_STEP = 100;
+static const int RESOURCE_STEP = 1;
+
+static const int MAXIMUM_CREATURES_CHANGE = 999999;
+
 TownEventDialog::TownEventDialog(CGTownInstance & t, QListWidgetItem * item, QWidget * parent) :
 	QDialog(parent),
 	ui(new Ui::TownEventDialog),
 	town(t),
 	townEventListItem(item)
 {
-	static const int FIRST_DAY_FOR_EVENT = 1;
-	static const int LAST_DAY_FOR_EVENT = 999;
-	static const int MAXIMUM_REPEAT_AFTER = 999;
 	ui->setupUi(this);
 
 	ui->buildingsTree->setModel(&buildingsModel);
@@ -34,7 +42,7 @@ TownEventDialog::TownEventDialog(CGTownInstance & t, QListWidgetItem * item, QWi
 	params = townEventListItem->data(MapEditorRoles::TownEventRole).toMap();
 	ui->eventFirstOccurrence->setMinimum(FIRST_DAY_FOR_EVENT);
 	ui->eventFirstOccurrence->setMaximum(LAST_DAY_FOR_EVENT);
-	ui->eventRepeatAfter->setMaximum(MAXIMUM_REPEAT_AFTER);
+	ui->eventRepeatAfter->setMaximum(MAXIMUM_EVENT_REPEAT_AFTER);
 	ui->eventNameText->setText(params.value("name").toString());
 	ui->eventMessageText->setPlainText(params.value("message").toString());
 	ui->eventAffectsCpu->setChecked(params.value("computerAffected").toBool());
@@ -67,21 +75,20 @@ void TownEventDialog::initPlayers()
 
 void TownEventDialog::initResources()
 {
-	static const int MAXIUMUM_GOLD_CHANGE = 999999;
-	static const int MAXIUMUM_RESOURCE_CHANGE = 999;
-	static const int GOLD_STEP = 100;
-	static const int RESOURCE_STEP = 1;
 	ui->resourcesTable->setRowCount(GameConstants::RESOURCE_QUANTITY);
 	auto resourcesMap = params.value("resources").toMap();
 	for (int i = 0; i < GameConstants::RESOURCE_QUANTITY; ++i)
 	{
 		auto name = QString::fromStdString(GameConstants::RESOURCE_NAMES[i]);
-		int val = resourcesMap.value(name).toInt();
+		auto * item = new QTableWidgetItem();
-		ui->resourcesTable->setItem(i, 0, new QTableWidgetItem(name));
+		item->setFlags(item->flags() & ~Qt::ItemIsEditable);
+		item->setText(name);
+		ui->resourcesTable->setItem(i, 0, item);
 
+		int val = resourcesMap.value(name).toInt();
 		QSpinBox * edit = new QSpinBox(ui->resourcesTable);
-		edit->setMaximum(i == GameResID::GOLD ? MAXIUMUM_GOLD_CHANGE : MAXIUMUM_RESOURCE_CHANGE);
+		edit->setMaximum(i == GameResID::GOLD ? MAXIMUM_GOLD_CHANGE : MAXIMUM_RESOURCE_CHANGE);
-		edit->setMinimum(i == GameResID::GOLD ? -MAXIUMUM_GOLD_CHANGE : -MAXIUMUM_RESOURCE_CHANGE);
+		edit->setMinimum(i == GameResID::GOLD ? -MAXIMUM_GOLD_CHANGE : -MAXIMUM_RESOURCE_CHANGE);
 		edit->setSingleStep(i == GameResID::GOLD ? GOLD_STEP : RESOURCE_STEP);
 		edit->setValue(val);
 
@@ -110,11 +117,6 @@ QStandardItem * TownEventDialog::addBuilding(const CTown& ctown, BuildingID buil
 {
 	auto bId = buildingId.num;
 	const CBuilding * building = ctown.buildings.at(buildingId);
-	if (!building)
-	{
-		remaining.erase(bId);
-		return nullptr;
-	}
 
 	QString name = QString::fromStdString(building->getNameTranslated());
 
@@ -142,12 +144,6 @@ QStandardItem * TownEventDialog::addBuilding(const CTown& ctown, BuildingID buil
 		if (!parent)
 			parent = addBuilding(ctown, building->upgrade.getNum(), remaining);
 
-		if (!parent)
-		{
-			remaining.erase(bId);
-			return nullptr;
-		}
-
 		parent->appendRow(checks);
 	}
 
@@ -157,7 +153,6 @@ QStandardItem * TownEventDialog::addBuilding(const CTown& ctown, BuildingID buil
 
 void TownEventDialog::initCreatures()
 {
-	static const int MAXIUMUM_CREATURES_CHANGE = 999999;
 	auto creatures = params.value("creatures").toList();
 	auto * ctown = town.town;
 	for (int i = 0; i < GameConstants::CREATURES_PER_TOWN; ++i)
@@ -185,7 +180,7 @@ void TownEventDialog::initCreatures()
 		auto creatureNumber = creatures.size() > i ? creatures.at(i).toInt() : 0;
 		QSpinBox* edit = new QSpinBox(ui->creaturesTable);
 		edit->setValue(creatureNumber);
-		edit->setMaximum(MAXIUMUM_CREATURES_CHANGE);
+		edit->setMaximum(MAXIMUM_CREATURES_CHANGE);
 		ui->creaturesTable->setCellWidget(i, 1, edit);
 
 	}
@@ -238,13 +233,7 @@ QVariantMap TownEventDialog::resourcesToVariant()
 
 QVariantList TownEventDialog::buildingsToVariant()
 {
-	auto buildings = getBuildingVariantsFromModel(buildingsModel, 1, Qt::Checked);
+	return getBuildingVariantsFromModel(buildingsModel, 1, Qt::Checked);
-#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
-	QVariantList buildingsList(buildings.begin(), buildings.end());
-#else
-	QVariantList buildingsList = QVariantList::fromStdList(buildings);
-#endif
-	return buildingsList;
 }
 
 QVariantList TownEventDialog::creaturesToVariant()
@@ -283,7 +272,8 @@ void TownEventDialog::onItemChanged(QStandardItem * item)
 	else if (item->checkState() == Qt::Unchecked) {
 		std::vector<QStandardItem *> stack;
 		stack.push_back(nextRow);
-		while (!stack.empty()) {
+		do
+		{
 			nextRow = stack.back();
 			stack.pop_back();
 			setRowColumnCheckState(nextRow, item->column(), Qt::Unchecked);
@@ -293,7 +283,7 @@ void TownEventDialog::onItemChanged(QStandardItem * item)
 				}
 			}
 
-		}
+		} while(!stack.empty());
 	}
 	connect(&buildingsModel, &QStandardItemModel::itemChanged, this, &TownEventDialog::onItemChanged);
 }

mapeditor/inspector/towneventdialog.ui

@@ -206,6 +206,9 @@
          <height>421</height>
         </rect>
        </property>
+       <property name="editTriggers">
+        <set>QAbstractItemView::NoEditTriggers</set>
+       </property>
        <attribute name="headerVisible">
         <bool>false</bool>
        </attribute>

mapeditor/inspector/townspellswidget.cpp

@@ -22,17 +22,8 @@ TownSpellsWidget::TownSpellsWidget(CGTownInstance & town, QWidget * parent) :
 {
 	ui->setupUi(this);
 
-	possibleSpellLists[0] = ui->possibleSpellList1;
+	possibleSpellLists = { ui->possibleSpellList1, ui->possibleSpellList2, ui->possibleSpellList3, ui->possibleSpellList4, ui->possibleSpellList5 };
-	possibleSpellLists[1] = ui->possibleSpellList2;
+	requiredSpellLists = { ui->requiredSpellList1, ui->requiredSpellList2, ui->requiredSpellList3, ui->requiredSpellList4, ui->requiredSpellList5 };
-	possibleSpellLists[2] = ui->possibleSpellList3;
-	possibleSpellLists[3] = ui->possibleSpellList4;
-	possibleSpellLists[4] = ui->possibleSpellList5;
-
-	requiredSpellLists[0] = ui->requiredSpellList1;
-	requiredSpellLists[1] = ui->requiredSpellList2;
-	requiredSpellLists[2] = ui->requiredSpellList3;
-	requiredSpellLists[3] = ui->requiredSpellList4;
-	requiredSpellLists[4] = ui->requiredSpellList5;
 
 	std::array<BuildingID, 5> mageGuilds = {BuildingID::MAGES_GUILD_1, BuildingID::MAGES_GUILD_2, BuildingID::MAGES_GUILD_3, BuildingID::MAGES_GUILD_4, BuildingID::MAGES_GUILD_5};
 	for (int i = 0; i < mageGuilds.size(); i++)
@@ -172,4 +163,4 @@ void TownSpellsDelegate::setModelData(QWidget * editor, QAbstractItemModel * mod
 	{
 		QStyledItemDelegate::setModelData(editor, model, index);
 	}
-}
+}

mapeditor/mapeditorroles.h

@@ -13,8 +13,8 @@
 
 enum MapEditorRoles
 {
-    TownEventRole = Qt::UserRole +1,
+	TownEventRole = Qt::UserRole + 1,
-    PlayerIDRole,
+	PlayerIDRole,
-    BuildingIDRole,
+	BuildingIDRole,
 	SpellIDRole
 };