Apply manual changes from code review

Mods/vcmi/config/vcmi/english.json

@@ -25,10 +25,9 @@
 	"vcmi.server.errors.modsIncompatibility" : "Required mods to load game:",
 	"vcmi.server.confirmReconnect"           : "Connect to the last session?",
 
-	"vcmi.settingsMainWindow.battleTab.hover" : "Battle Options", //unused
-	"vcmi.settingsMainWindow.systemTab.hover" : "System Options", //unused
-
-	"vcmi.settingsMainWindow.systemTab.help"     : "Switches to System Options tab - these settings are related to general game client behavior",
+	"vcmi.settingsMainWindow.generalTab.hover" : "General",
+	"vcmi.settingsMainWindow.generalTab.help"     : "Switches to System Options tab - these settings are related to general game client behavior",
+	"vcmi.settingsMainWindow.battleTab.hover" : "Battle",
 	"vcmi.settingsMainWindow.battleTab.help"     : "Switches to Battle Options tab - these settings allow configuring battle interface and related things",
 	"vcmi.settingsMainWindow.adventureTab.hover" : "Adventure Map",
 	"vcmi.settingsMainWindow.adventureTab.help"  : "Switches to Adventure Map Options tab - adventure map is part of the game where you can move your heroes",
@@ -76,8 +75,8 @@
 
 	"vcmi.otherOptions.availableCreaturesAsDwellingLabel.hover" : "Show available creatures in town summary",
 	"vcmi.otherOptions.availableCreaturesAsDwellingLabel.help" : "{Show available creatures in town summary}\n\n Shows creatures available to purchase instead of their growth in town summary (bottom-left corner).",
-	"vcmi.otherOptions.compactTownCreatureInfo.hover": "More compact creature info in town summary",
-	"vcmi.otherOptions.compactTownCreatureInfo.help": "{More compact creature info in town summary}\n\n Smaller town creatures information in town summary.",
+	"vcmi.otherOptions.compactTownCreatureInfo.hover": "Compact creature info in town summary",
+	"vcmi.otherOptions.compactTownCreatureInfo.help": "{Compact creature info in town summary}\n\n Smaller town creatures information in town summary.",
 
 	"vcmi.townHall.missingBase"             : "Base building %s must be built first",
 	"vcmi.townHall.noCreaturesToRecruit"    : "There are no creatures to recruit!",

client/battle/BattleInterface.cpp

@@ -102,6 +102,11 @@ BattleInterface::BattleInterface(const CCreatureSet *army1, const CCreatureSet *
 	windowObject->blockUI(true);
 	windowObject->updateQueue();
 
+	playIntroSoundAndUnlockInterface();
+}
+
+void BattleInterface::playIntroSoundAndUnlockInterface()
+{
 	if(settings["gameTweaks"]["skipBattleIntroMusic"].Bool())
 	{
 		onIntroSoundPlayed();
@@ -748,4 +753,4 @@ void BattleInterface::setBattleQueueVisibility(bool visible)
 		windowObject->showQueue();
 	else
 		windowObject->hideQueue();
-}
+}

client/battle/BattleInterface.h

@@ -112,6 +112,7 @@ class BattleInterface
 	/// defender interface, not null if attacker is human in our vcmiclient
 	std::shared_ptr<CPlayerInterface> defenderInt;
 
+	void playIntroSoundAndUnlockInterface();
 	void onIntroSoundPlayed();
 public:
 	/// copy of initial armies (for result window)

client/battle/BattleInterfaceClasses.h

@@ -14,7 +14,6 @@
 #include "../../lib/FunctionList.h"
 #include "../../lib/battle/BattleHex.h"
 #include "../windows/CWindowObject.h"
-#include "gui/InterfaceObjectConfigurable.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

client/windows/CWindowObject.cpp

@@ -59,7 +59,7 @@ CWindowObject::CWindowObject(int options_, std::string imageName):
 	options(options_),
 	background(createBg(imageName, options_ & PLAYER_COLORED))
 {
-//	assert(parent == nullptr); //Safe to remove, but windows should not have parent
+	assert(parent == nullptr); //Safe to remove, but windows should not have parent
 
 	defActions = 255-DISPOSE;
 

client/windows/settings/BattleOptionsTab.cpp

@@ -11,8 +11,9 @@
 
 #include "BattleOptionsTab.h"
 #include "CConfigHandler.h"
-#include "../../gui/CGuiHandler.h"
 
+#include "../../battle/BattleInterface.h"
+#include "../../gui/CGuiHandler.h"
 #include "../../../lib/filesystem/ResourceID.h"
 #include "../../../lib/CGeneralTextHandler.h"
 #include "../../widgets/Buttons.h"

client/windows/settings/BattleOptionsTab.h

@@ -10,7 +10,8 @@
 #pragma once
 
 #include "../../gui/InterfaceObjectConfigurable.h"
-#include "../../battle/BattleInterface.h"
+
+class BattleInterface;
 
 class BattleOptionsTab : public InterfaceObjectConfigurable
 {

client/windows/settings/GeneralOptionsTab.cpp

@@ -11,9 +11,6 @@
 
 #include "GeneralOptionsTab.h"
 
-#include <SDL_surface.h>
-#include <SDL_rect.h>
-
 #include "../../../lib/CGeneralTextHandler.h"
 #include "../../../lib/filesystem/ResourceID.h"
 #include "../../gui/CGuiHandler.h"
@@ -26,7 +23,6 @@
 #include "windows/GUIClasses.h"
 #include "CServerHandler.h"
 #include "renderSDL/SDL_Extensions.h"
-#include "CMT.h"
 
 
 static void setIntSetting(std::string group, std::string field, int value)
@@ -60,6 +56,9 @@ GeneralOptionsTab::GeneralOptionsTab()
 	addCallback("fullscreenChanged", std::bind(&GeneralOptionsTab::setFullscreenMode, this, _1));
 	addCallback("setGameResolution", std::bind(&GeneralOptionsTab::selectGameResolution, this));
 	addCallback("framerateChanged", std::bind(&setBoolSetting, "general", "showfps", _1));
+	//moved from "other" tab that is disabled for now to avoid excessible tabs with barely any content
+	addCallback("availableCreaturesAsDwellingLabelChanged", std::bind(&setBoolSetting, "gameTweaks", "availableCreaturesAsDwellingLabel", _1));
+	addCallback("compactTownCreatureInfoChanged", std::bind(&setBoolSetting, "gameTweaks", "compactTownCreatureInfo", _1));
 	build(config);
 
 	std::shared_ptr<CLabel> resolutionLabel = widget<CLabel>("resolutionLabel");
@@ -85,6 +84,12 @@ GeneralOptionsTab::GeneralOptionsTab()
 
 	std::shared_ptr<CSlider> volumeSlider = widget<CSlider>("soundVolumeSlider");
 	volumeSlider->moveTo(CCS->soundh->getVolume());
+
+	std::shared_ptr<CToggleButton> availableCreaturesAsDwellingLabelCheckbox = widget<CToggleButton>("availableCreaturesAsDwellingLabelCheckbox");
+	availableCreaturesAsDwellingLabelCheckbox->setSelected((bool)settings["gameTweaks"]["availableCreaturesAsDwellingLabel"].Bool());
+
+	std::shared_ptr<CToggleButton> compactTownCreatureInfo = widget<CToggleButton>("compactTownCreatureInfoCheckbox");
+	compactTownCreatureInfo->setSelected((bool)settings["gameTweaks"]["compactTownCreatureInfo"].Bool());
 }
 
 
@@ -152,7 +157,7 @@ void GeneralOptionsTab::setFullscreenMode(bool on)
 
 	const auto & screenRes = settings["video"]["screenRes"];
 	const Point desiredResolution(screenRes["width"].Integer(), screenRes["height"].Integer());
-	const Point currentResolution(screen->w, screen->h);
+	const Point currentResolution = GH.screenDimensions();
 
 	if (!isResolutionSupported(currentResolution, on))
 	{

client/windows/settings/GeneralOptionsTab.h

@@ -12,6 +12,7 @@
 #include "../../../lib/CConfigHandler.h"
 #include "../../gui/InterfaceObjectConfigurable.h"
 
+
 class GeneralOptionsTab : public InterfaceObjectConfigurable
 {
 private:

client/windows/settings/SettingsMainContainer.cpp

@@ -22,7 +22,9 @@
 #include "CGeneralTextHandler.h"
 #include "gui/CGuiHandler.h"
 #include "lobby/CSavingScreen.h"
+#include "widgets/Buttons.h"
 #include "widgets/Images.h"
+#include "widgets/ObjectLists.h"
 #include "CGameInfo.h"
 #include "CPlayerInterface.h"
 #include "CServerHandler.h"
@@ -64,7 +66,9 @@ SettingsMainContainer::SettingsMainContainer(BattleInterface * parentBattleUi) :
 	}
 
 	int defaultTabIndex = 0;
-	if(settings["general"]["lastSettingsTab"].isNumber())
+	if(parentBattleUi != nullptr)
+		defaultTabIndex = 2;
+	else if(settings["general"]["lastSettingsTab"].isNumber())
 		defaultTabIndex = settings["general"]["lastSettingsTab"].Integer();
 
 	parentBattleInterface = parentBattleUi;

client/windows/settings/SettingsMainContainer.h

@@ -10,11 +10,11 @@
 
 #pragma once
 
-#include "widgets/ObjectLists.h"
-#include "widgets/Buttons.h"
 #include "gui/InterfaceObjectConfigurable.h"
-#include "gui/CGuiHandler.h"
-#include "battle/BattleInterface.h"
+
+class BattleInterface;
+class CTabbedInt;
+enum class EUserEvent;
 
 class SettingsMainContainer : public InterfaceObjectConfigurable
 {

config/widgets/settings/generalOptionsTab.json

@@ -115,6 +115,44 @@
 			"help": "vcmi.systemOptions.framerateButton",
 			"position": {"x": 28, "y": 328},
 			"callback": "framerateChanged"
+		},
+
+		{
+			"name": "availableCreaturesAsDwellingLabelText",
+			"type": "label",
+			"font": "medium",
+			"alignment": "left",
+			"color": "white",
+			"text": "vcmi.otherOptions.availableCreaturesAsDwellingLabel.hover",
+			"position": {"x": 64, "y": 366}
+		},
+
+		{
+			"name": "availableCreaturesAsDwellingLabelCheckbox",
+			"type": "toggleButton",
+			"image": "sysopchk.def",
+			"help": "vcmi.otherOptions.availableCreaturesAsDwellingLabel",
+			"position": {"x": 28, "y": 364},
+			"callback": "availableCreaturesAsDwellingLabelChanged"
+		},
+
+		{
+			"name": "compactTownCreatureInfoLabel",
+			"type": "label",
+			"font": "medium",
+			"alignment": "left",
+			"color": "white",
+			"text": "vcmi.otherOptions.compactTownCreatureInfo.hover",
+			"position": {"x": 64, "y": 402}
+		},
+
+		{
+			"name": "compactTownCreatureInfoCheckbox",
+			"type": "toggleButton",
+			"image": "sysopchk.def",
+			"help": "vcmi.otherOptions.compactTownCreatureInfo",
+			"position": {"x": 28, "y": 400},
+			"callback": "compactTownCreatureInfoChanged"
 		}
 	]
 }

config/widgets/settings/settingsMainContainer.json

@@ -22,7 +22,7 @@
 					"type": "toggleButton",
 					"position": {"x": 0, "y": 0},
 					"image": "buttons/quadwide",
-					"help": "vcmi.settingsMainWindow.systemTab",
+					"help": "vcmi.settingsMainWindow.generalTab",
 					"items":
 					[
 						{
@@ -31,7 +31,7 @@
 							"font": "big",
 							"alignment": "center",
 							"color": "yellow",
-							"text": "core.genrltxt.568"
+							"text": "vcmi.settingsMainWindow.generalTab.hover"
 						}
 					]
 				},
@@ -69,29 +69,29 @@
 							"font": "big",
 							"alignment": "center",
 							"color": "yellow",
-							"text": "core.genrltxt.392"
-						}
-					]
-				},
-
-				{
-					"index": 3,
-					"type": "toggleButton",
-					"position": {"x": 450, "y": 0},
-					"image": "buttons/quadwide",
-					"help": "vcmi.settingsMainWindow.otherTab",
-					"items":
-					[
-						{
-							"name": "otherTabButtonTitle",
-							"type": "label",
-							"font": "big",
-							"alignment": "center",
-							"color": "yellow",
-							"text": "vcmi.settingsMainWindow.otherTab.hover"
+							"text": "vcmi.settingsMainWindow.battleTab.hover"
 						}
 					]
 				}
+				//disabled, due to being redundant for now
+//				{
+//					"index": 3,
+//					"type": "toggleButton",
+//					"position": {"x": 450, "y": 0},
+//					"image": "buttons/quadwide",
+//					"help": "vcmi.settingsMainWindow.otherTab",
+//					"items":
+//					[
+//						{
+//							"name": "otherTabButtonTitle",
+//							"type": "label",
+//							"font": "big",
+//							"alignment": "center",
+//							"color": "yellow",
+//							"text": "vcmi.settingsMainWindow.otherTab.hover"
+//						}
+//					]
+//				}
 			],
 			"callback": "activateSettingsTab"
 		},