Fixes following reviews

mapeditor/PlayerSelectionDialog.cpp

@@ -12,26 +12,24 @@
 #include "mainwindow.h"
 #include "../lib/mapping/CMap.h"
 
-PlayerSelectionDialog::PlayerSelectionDialog(QWidget * parent)
-	: QDialog(parent), selectedPlayer(PlayerColor::NEUTRAL)
+PlayerSelectionDialog::PlayerSelectionDialog(MainWindow * mainWindow)
+	: QDialog(mainWindow), selectedPlayer(PlayerColor::NEUTRAL)
 {
-	auto main = qobject_cast<MainWindow *>(parent);
-	assert(main);
+	assert(mainWindow);
 
 	setupDialogComponents();
 
 	int maxPlayers = 0;
-	if(main && main->controller.map())
-		maxPlayers = main->controller.map()->players.size();
-	
-	bool defaultIsChecked = false;
+	if(mainWindow && mainWindow->controller.map())
+		maxPlayers = mainWindow->controller.map()->players.size();
 
 	for(int i = 0; i < maxPlayers; ++i)
 	{
 		PlayerColor player(i);
-		QAction * action = main->getActionPlayer(player);
+		QAction * action = mainWindow->getActionPlayer(player);
+		bool isEnabled = mainWindow->controller.map()->players.at(i).canAnyonePlay();
 
-		addCheckbox(action, player, main->controller.map()->players.at(i).canAnyonePlay(), & defaultIsChecked);
+		addCheckbox(action, player, isEnabled);
 	}
 }
 
@@ -53,9 +51,8 @@ void PlayerSelectionDialog::onCheckboxToggled(bool checked)
 		}
 		else
 		{
-			cb->blockSignals(true);
+			QSignalBlocker blocker(cb);
 			cb->setChecked(false);
-			cb->blockSignals(false);
 		}
 	}
 }
@@ -101,7 +98,7 @@ void PlayerSelectionDialog::setupDialogComponents()
 	setLayout(& mainLayout);
 }
 
-void PlayerSelectionDialog::addCheckbox(QAction * checkboxAction, PlayerColor player, bool isEnabled, bool * isDefault)
+void PlayerSelectionDialog::addCheckbox(QAction * checkboxAction, PlayerColor player, bool isEnabled)
 {
 	QHBoxLayout * rowLayout = new QHBoxLayout();
 	auto * checkbox = new QCheckBox(checkboxAction->text(), this);
@@ -122,16 +119,17 @@ void PlayerSelectionDialog::addCheckbox(QAction * checkboxAction, PlayerColor pl
 	rowLayout->addWidget(shortcutLabel, 1);
 
 	checkbox->setEnabled(isEnabled);
-	if(checkbox->isEnabled() && !*isDefault)
+
+	if(isEnabled && !defaultCheckedSet)
 	{
 		checkbox->setChecked(true);
 		selectedPlayer = player;
-		*isDefault = true;
+		defaultCheckedSet = true;
 	}
 
 	checkboxLayout.addLayout(rowLayout);
 
-	connect(checkbox, &QCheckBox::clicked, this, [=]()
+	connect(checkbox, &QCheckBox::clicked, this, [this, checkbox, player]()
 		{
 			selectedPlayer = player;
 
@@ -148,7 +146,7 @@ void PlayerSelectionDialog::addCheckbox(QAction * checkboxAction, PlayerColor pl
 	addAction(checkboxAction);
 
 	// Connect action trigger to simulate checkbox click
-	connect(checkboxAction, &QAction::triggered, this, [=]()
+	connect(checkboxAction, &QAction::triggered, this, [checkbox]()
 		{
 			if(checkbox->isEnabled())
 				checkbox->click();

mapeditor/PlayerSelectionDialog.h

@@ -11,10 +11,10 @@
 #pragma once
 
 #include <QDialog>
-#include <QComboBox>
-#include <QCheckBox>
-#include <QDialogButtonBox>
-#include "../source/lib/constants/EntityIdentifiers.h"
+#include "../lib/constants/EntityIdentifiers.h"
+
+class QCheckBox;
+class MainWindow;
 
 /// Dialog shown when a hero cannot be placed as NEUTRAL.
 /// Allows the user to select a valid player via checkboxes,
@@ -24,7 +24,7 @@ class PlayerSelectionDialog : public QDialog
 	Q_OBJECT
 
 public:
-	explicit PlayerSelectionDialog(QWidget * parent = nullptr);
+	explicit PlayerSelectionDialog(MainWindow * mainWindow = nullptr);
 	PlayerColor getSelectedPlayer() const;
 
 private slots:
@@ -38,7 +38,9 @@ private:
 	QVBoxLayout mainLayout;
 	QVBoxLayout checkboxLayout;
 
+	bool defaultCheckedSet = false;
+
 	void setupDialogComponents();
-	void addCheckbox(QAction * checkboxAction, PlayerColor player, bool isEnabled, bool * isDefault);
+	void addCheckbox(QAction * checkboxAction, PlayerColor player, bool isEnabled);
 
 };

mapeditor/mainwindow.cpp

@@ -609,14 +609,7 @@ void MainWindow::on_actionCampaignEditor_triggered()
 void MainWindow::on_actionNew_triggered()
 {
 	if(getAnswerAboutUnsavedChanges())
-	{
 		new WindowNewMap(this);
-		//mapSettings = new MapSettings(controller, this);
-		//connect(&controller, &MapController::requestModsUpdate,
-		//	mapSettings->getModSettings(), &ModSettings::updateModWidgetBasedOnMods);
-
-		//controller.requestModsUpdate({}, true);
-	}
 }
 
 void MainWindow::on_actionSave_triggered()
@@ -1136,9 +1129,6 @@ void MainWindow::on_inspectorWidget_itemChanged(QTableWidgetItem *item)
 
 void MainWindow::on_actionMapSettings_triggered()
 {
-	if(!mapSettings)
-		return;
-
 	if(mapSettings->isVisible())
 	{
 		mapSettings->raise();
@@ -1146,8 +1136,6 @@ void MainWindow::on_actionMapSettings_triggered()
 	}
 	else
 	{
-		mapSettings->setWindowModality(Qt::WindowModal);
-		mapSettings->setModal(true);
 		mapSettings->show();
 	}
 }
@@ -1179,14 +1167,14 @@ void MainWindow::switchDefaultPlayer(const PlayerColor & player)
 	if(controller.defaultPlayer == player)
 		return;
 
-	ui->actionNeutral->blockSignals(true);
+	QSignalBlocker blockerNeutral(ui->actionNeutral);
 	ui->actionNeutral->setChecked(PlayerColor::NEUTRAL == player);
-	ui->actionNeutral->blockSignals(false);
+
 	for(int i = 0; i < PlayerColor::PLAYER_LIMIT.getNum(); ++i)
 	{
-		getActionPlayer(PlayerColor(i))->blockSignals(true);
-		getActionPlayer(PlayerColor(i))->setChecked(PlayerColor(i) == player);
-		getActionPlayer(PlayerColor(i))->blockSignals(false);
+		QAction * playerEntry = getActionPlayer(PlayerColor(i));
+		QSignalBlocker blocker(playerEntry); 
+		playerEntry->setChecked(PlayerColor(i) == player);
 	}
 	controller.defaultPlayer = player;
 }

mapeditor/mapcontroller.cpp

@@ -588,7 +588,7 @@ bool MapController::canPlaceHero(const CGObjectInstance * heroObj, QString & err
 		return true;
 	}
 	
-	error = QObject::tr("Hero %1 cannot be created as NEUTRAL.").arg(QString::fromStdString(heroObj->instanceName));
+	error = tr("Hero %1 cannot be created as NEUTRAL.").arg(QString::fromStdString(heroObj->instanceName));
 	return false;
 }
 
@@ -603,18 +603,18 @@ bool MapController::checkRequiredMods(const CGObjectInstance * obj, QString & er
 		{
 			QString submod;
 			if(!mod.second.parent.empty())
-				submod = " (" + tr("submod of") + " " + QString::fromStdString(mod.second.parent) + ")";
+				submod = tr(" (submod of %1)").arg(QString::fromStdString(mod.second.parent));
 
 			auto reply = QMessageBox::question(main,
 				tr("Missing Required Mod"),
 				tr("This object is from the mod '%1'%2.\n\n"
 					"The mod is currently not in the map's required modifications list.\n\n"
-					"Do you want to add this mod to the required modifications ?\n")
+					"Do you want to add this mod to the required modifications?\n")
 				.arg(QString::fromStdString(LIBRARY->modh->getModInfo(mod.first).getVerificationInfo().name), submod),
 				QMessageBox::Yes | QMessageBox::No);
 
 			if(reply == QMessageBox::Yes)
-				/* emit */ requestModsUpdate(modsInfo, true); // signal for MapSettings
+				Q_EMIT requestModsUpdate(modsInfo, true); // signal for MapSettings
 			else
 			{
 				error = tr("This object's mod is mandatory for map to remain valid.");

mapeditor/mapsettings/mapsettings.cpp

@@ -26,6 +26,7 @@ MapSettings::MapSettings(MapController & ctrl, QWidget *parent) :
 {
 	ui->setupUi(this);
 
+	setWindowModality(Qt::WindowModal);
 	setWindowFlags(Qt::Dialog | Qt::WindowTitleHint | Qt::WindowCloseButtonHint);
 	
 	assert(controller.map());

mapeditor/validator.cpp

@@ -174,7 +174,7 @@ std::set<Validator::Issue> Validator::validate(const CMap * map)
 			{
 				QString submod;
 				if(!mod.second.parent.empty())
-					submod = " ("+ tr("submod of") +" "+ QString::fromStdString(mod.second.parent) + ")";
+					submod = tr(" (submod of %1").arg(QString::fromStdString(mod.second.parent));
 				
 				issues.insert({
 						tr("Map contains object(s) from mod '%1'%2, but the mod is missing from the map's required mods list."
@@ -195,30 +195,6 @@ std::set<Validator::Issue> Validator::validate(const CMap * map)
 	return issues;
 }
 
-QPixmap Validator::createGreenTickIcon()
-{
-	QPixmap pixmap(24, 24);
-	pixmap.fill(Qt::transparent);
-
-	QPainter painter(&pixmap);
-	painter.setRenderHint(QPainter::Antialiasing);
-
-	QBrush brush(QColor(50, 205, 50));
-	painter.setBrush(brush);
-	painter.setPen(Qt::NoPen);
-	painter.drawEllipse(0, 0, 24, 24);
-
-	QPen pen(Qt::white);
-	pen.setWidth(3);
-	painter.setPen(pen);
-
-	painter.drawLine(7, 12, 10, 17);  // Tick part 1
-	painter.drawLine(10, 17, 17, 7);  // Tick part 2
-
-	painter.end();
-	return pixmap;
-}
-
 void Validator::showValidationResults(const CMap * map)
 {
 	show();
@@ -235,7 +211,7 @@ void Validator::showValidationResults(const CMap * map)
 
 	if(ui->listWidget->count() == 0)
 	{
-		QPixmap greenTick = createGreenTickIcon();
+		QPixmap greenTick = QPixmap(":/icons/mod-enabled.png");
 		QString validMessage = tr("The map is valid and has no issues.");
 		auto * item = new QListWidgetItem(QIcon(greenTick), validMessage, ui->listWidget);
 		ui->listWidget->addItem(item);
@@ -276,13 +252,11 @@ void Validator::adjustWindowSize()
 	int finalWidth = qMin(contentWidth + padding, screenWidth - screenMarginHorizontal);
 	int finalHeight = qMin(contentHeight + padding, screenHeight - screenMarginVertical);
 
-	logGlobal->warn("adjustWindowSize(): %d x %d", finalWidth, finalHeight);
-
 	QWidget * parentWidget = ui->listWidget->parentWidget();
 	if(parentWidget)
 	{
-		parentWidget->setFixedWidth(finalWidth + padding);
-		parentWidget->setFixedHeight(finalHeight + padding);
+		parentWidget->setMinimumWidth(finalWidth + padding);
+		parentWidget->setMinimumHeight(finalHeight + padding);
 	}
 
 	ui->listWidget->resize(finalWidth, finalHeight);

mapeditor/validator.h

@@ -49,7 +49,6 @@ private:
 
     QRect screenGeometry;
 
-	QPixmap createGreenTickIcon();
 	void showValidationResults(const CMap * map);
     void adjustWindowSize();
 };