Fixes CGUISimpleSetting warnings and allows it to be moved.

Differential Revision: https://code.wildfiregames.com/D5071
This was SVN commit r27763.

source/gui/CGUISetting.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2021 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -20,18 +20,25 @@
 #include "CGUISetting.h"
 
 #include "gui/CGUI.h"
+#include "gui/CGUISprite.h"
 #include "gui/ObjectBases/IGUIObject.h"
+#include "gui/SettingTypes/CGUIList.h"
+#include "gui/SettingTypes/CGUISeries.h"
+#include "gui/SettingTypes/CGUISize.h"
+#include "gui/SettingTypes/CGUIString.h"
+#include "gui/SettingTypes/EAlign.h"
 #include "ps/CLogger.h"
+#include "ps/CStr.h"
 #include "scriptinterface/ScriptConversions.h"
 
-IGUISetting::IGUISetting(const CStr& name, IGUIObject* owner) : m_pObject(*owner)
+IGUISetting::IGUISetting(const CStr& name, IGUIObject* owner) : m_Object(*owner), m_Name(name)
 {
-	m_pObject.RegisterSetting(name, this);
+	m_Object.RegisterSetting(m_Name, this);
 }
 
-IGUISetting::IGUISetting(IGUISetting&& o) : m_pObject(o.m_pObject)
+IGUISetting::IGUISetting(IGUISetting&& other) : m_Object(other.m_Object), m_Name(other.m_Name)
 {
-	m_pObject.ReregisterSetting(o.GetName(), this);
+	m_Object.ReregisterSetting(m_Name, this);
 }
 
 bool IGUISetting::FromString(const CStrW& value, const bool sendMessage)
@@ -57,13 +64,13 @@ bool IGUISetting::FromJSVal(const ScriptRequest& rq, JS::HandleValue value, cons
 
 void IGUISetting::OnSettingChange(const CStr& setting, bool sendMessage)
 {
-	m_pObject.SettingChanged(setting, sendMessage);
+	m_Object.SettingChanged(setting, sendMessage);
 }
 
 template<typename T>
 bool CGUISimpleSetting<T>::DoFromString(const CStrW& value)
 {
-	return CGUI::ParseString<T>(&m_pObject.GetGUI(), value, m_Setting);
+	return CGUI::ParseString<T>(&m_Object.GetGUI(), value, m_Setting);
 };
 
 template<>
@@ -75,7 +82,7 @@ bool CGUISimpleSetting<CGUIColor>::DoFromJSVal(const ScriptRequest& rq, JS::Hand
 		if (!Script::FromJSVal(rq, value, name))
 			return false;
 
-		if (!m_Setting.ParseString(m_pObject.GetGUI(), name))
+		if (!m_Setting.ParseString(m_Object.GetGUI(), name))
 		{
 			LOGERROR("Invalid color '%s'", name.c_str());
 			return false;
@@ -108,23 +115,16 @@ TYPE(i32)
 TYPE(u32)
 TYPE(float)
 TYPE(CVector2D)
-#include "ps/CStr.h"
 TYPE(CStr)
 TYPE(CStrW)
 // TODO: make these inherit from CGUISimpleSetting directly.
-#include "gui/SettingTypes/CGUISize.h"
 TYPE(CGUISize)
 TYPE(CGUIColor)
-#include "gui/CGUISprite.h"
 TYPE(CGUISpriteInstance)
-#include "gui/SettingTypes/CGUIString.h"
 TYPE(CGUIString)
-#include "gui/SettingTypes/EAlign.h"
 TYPE(EAlign)
 TYPE(EVAlign)
-#include "gui/SettingTypes/CGUIList.h"
 TYPE(CGUIList)
-#include "gui/SettingTypes/CGUISeries.h"
 TYPE(CGUISeries)
 
 #undef TYPE

source/gui/CGUISetting.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2021 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -51,7 +51,9 @@ public:
 	virtual void ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue value) = 0;
 
 protected:
-	IGUISetting(IGUISetting&& o);
+	IGUISetting(IGUISetting&& other);
+	IGUISetting& operator=(IGUISetting&& other) = delete;
+
 	virtual ~IGUISetting() = default;
 
 	virtual bool DoFromString(const CStrW& value) = 0;
@@ -67,12 +69,18 @@ protected:
 	/**
 	 * Return the name of the setting, from JS.
 	 */
-	virtual CStr GetName() const = 0;
+	const CStr& GetName() const
+	{
+		return m_Name;
+	}
 
 	/**
 	 * The object that stores this setting.
 	 */
-	IGUIObject& m_pObject;
+	IGUIObject& m_Object;
+
+private:
+	CStr m_Name;
 };
 
 /**
@@ -85,11 +93,12 @@ class CGUISimpleSetting : public IGUISetting
 {
 public:
 	template<typename... Args>
-	CGUISimpleSetting(IGUIObject* pObject, const CStr& Name, Args&&... args)
-		: IGUISetting(Name, pObject), m_Name(Name), m_Setting(args...)
+	CGUISimpleSetting(IGUIObject* pObject, const CStr& name, Args&&... args)
+		: IGUISetting(name, pObject), m_Setting(args...)
 	{}
 	NONCOPYABLE(CGUISimpleSetting);
-	MOVABLE(CGUISimpleSetting);
+	CGUISimpleSetting(CGUISimpleSetting&&) = default;
+	CGUISimpleSetting& operator=(CGUISimpleSetting&&) = delete;
 
 	operator const T&() const { return m_Setting; }
 	const T& operator*() const { return m_Setting; }
@@ -106,20 +115,14 @@ public:
 	void Set(T value, bool sendMessage)
 	{
 		m_Setting = std::move(value);
-		OnSettingChange(m_Name, sendMessage);
+		OnSettingChange(GetName(), sendMessage);
 	}
 
 protected:
-	CStr GetName() const override
-	{
-		return m_Name;
-	}
-
 	bool DoFromString(const CStrW& value) override;
 	bool DoFromJSVal(const ScriptRequest& rq, JS::HandleValue value) override;
 	void ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue value) override;
 
-	const CStr m_Name;
 	T m_Setting;
 };
 

source/gui/ObjectTypes/CDropDown.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2021 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -51,8 +51,8 @@ CDropDown::CDropDown(CGUI& pGUI)
 	  m_Sprite2Disabled(this, "sprite2_disabled"),
 	  m_TextColorDisabled(this, "textcolor_disabled")
 	  // Add these in CList! And implement TODO
-	  //RegisterSetting("textcolor_over");
-	  //RegisterSetting("textcolor_pressed");
+	  //m_TextColorOver("textcolor_over");
+	  //m_TextColorPressed("textcolor_pressed");
 {
 	m_ScrollBar.Set(true, true);
 }

source/gui/ObjectTypes/COList.h

@@ -34,7 +34,9 @@ public:
 	{}
 	// Avoid copying the strings.
 	NONCOPYABLE(COListColumn);
-	MOVABLE(COListColumn);
+	COListColumn(COListColumn&&) = default;
+	COListColumn& operator=(COListColumn&&) = delete;
+
 	CGUIColor m_TextColor;
 	CStr m_Id;
 	float m_Width;

source/gui/SettingTypes/CGUIHotkey.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2021 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -25,18 +25,18 @@
 
 bool CGUIHotkey::DoFromString(const CStrW& value)
 {
-	m_pObject.GetGUI().UnsetObjectHotkey(&m_pObject, m_Setting);
+	m_Object.GetGUI().UnsetObjectHotkey(&m_Object, m_Setting);
 	m_Setting = value.ToUTF8();
-	m_pObject.GetGUI().SetObjectHotkey(&m_pObject, m_Setting);
+	m_Object.GetGUI().SetObjectHotkey(&m_Object, m_Setting);
 	return true;
 }
 
 bool CGUIHotkey::DoFromJSVal(const ScriptRequest& rq, JS::HandleValue value)
 {
-	m_pObject.GetGUI().UnsetObjectHotkey(&m_pObject, m_Setting);
+	m_Object.GetGUI().UnsetObjectHotkey(&m_Object, m_Setting);
 	if (!Script::FromJSVal(rq, value, m_Setting))
 		return false;
-	m_pObject.GetGUI().SetObjectHotkey(&m_pObject, m_Setting);
+	m_Object.GetGUI().SetObjectHotkey(&m_Object, m_Setting);
 	return true;
 }
 

source/gui/SettingTypes/CGUIHotkey.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2021 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -34,7 +34,8 @@ public:
 	CGUIHotkey(IGUIObject* pObject, const CStr& Name) : CGUISimpleSetting<CStr>(pObject, Name)
 	{}
 	NONCOPYABLE(CGUIHotkey);
-	MOVABLE(CGUIHotkey);
+	CGUIHotkey(CGUIHotkey&&) = default;
+	CGUIHotkey& operator=(CGUIHotkey&&) = delete;
 
 	bool DoFromString(const CStrW& value) override;
 	bool DoFromJSVal(const ScriptRequest& rq, JS::HandleValue value) override;

source/gui/SettingTypes/MouseEventMask.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2022 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -36,7 +36,7 @@ class IGUISetting;
 
 namespace
 {
-	const std::string MOUSE_EVENT_MASK = "mouse_event_mask";
+	const CStr MOUSE_EVENT_MASK = "mouse_event_mask";
 }
 
 class CGUIMouseEventMask::Impl
@@ -67,11 +67,6 @@ bool CGUIMouseEventMask::DoFromJSVal(const ScriptRequest& rq, JS::HandleValue va
 	return DoFromString(spec);
 }
 
-CStr CGUIMouseEventMask::GetName() const
-{
-	return MOUSE_EVENT_MASK;
-}
-
 bool CGUIMouseEventMask::IsMouseOver(const CVector2D& mousePos, const CRect& objectSize) const
 {
 	if (m_Impl)

source/gui/SettingTypes/MouseEventMask.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2021 Wildfire Games.
+/* Copyright (C) 2023 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -41,7 +41,11 @@ class ScriptRequest;
 class CGUIMouseEventMask : public IGUISetting
 {
 public:
+	NONCOPYABLE(CGUIMouseEventMask);
+
 	CGUIMouseEventMask(IGUIObject* owner);
+	CGUIMouseEventMask(CGUIMouseEventMask&&) = default;
+	CGUIMouseEventMask& operator=(CGUIMouseEventMask&&) = delete;
 	~CGUIMouseEventMask();
 
 	/**
@@ -60,7 +64,6 @@ public:
 protected:
 	bool DoFromString(const CStrW& value) override;
 	bool DoFromJSVal(const ScriptRequest& rq, JS::HandleValue value) override;
-	CStr GetName() const override;
 
 	std::string m_Spec;
 	std::unique_ptr<Impl> m_Impl;

source/gui/tests/test_GUISetting.h

@@ -0,0 +1,103 @@
+/* Copyright (C) 2023 Wildfire Games.
+ * This file is part of 0 A.D.
+ *
+ * 0 A.D. is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 0 A.D. is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with 0 A.D.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "lib/self_test.h"
+
+#include "graphics/FontManager.h"
+#include "graphics/FontMetrics.h"
+#include "gui/CGUI.h"
+#include "gui/CGUISetting.h"
+#include "gui/CGUIText.h"
+#include "gui/ObjectBases/IGUIObject.h"
+#include "gui/SettingTypes/CGUIString.h"
+#include "ps/CLogger.h"
+#include "ps/ConfigDB.h"
+#include "ps/Filesystem.h"
+#include "ps/ProfileViewer.h"
+#include "ps/VideoMode.h"
+#include "renderer/Renderer.h"
+#include "scriptinterface/ScriptInterface.h"
+
+#include <memory>
+#include <type_traits>
+
+class TestGUISetting : public CxxTest::TestSuite
+{
+	std::unique_ptr<CProfileViewer> m_Viewer;
+	std::unique_ptr<CRenderer> m_Renderer;
+
+public:
+	class TestGUIObject : public IGUIObject
+	{
+	public:
+		TestGUIObject(CGUI& gui) : IGUIObject(gui) {}
+
+		void Draw(CCanvas2D&) {}
+	};
+
+	void setUp()
+	{
+		g_VFS = CreateVfs();
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir() / "mods" / "_test.minimal" / "", VFS_MOUNT_MUST_EXIST));
+		TS_ASSERT_OK(g_VFS->Mount(L"cache", DataDir() / "_testcache" / "", 0, VFS_MAX_PRIORITY));
+
+		CXeromyces::Startup();
+
+		// The renderer spews messages.
+		TestLogger logger;
+
+		// We need to initialise the renderer to initialise the font manager.
+		// TODO: decouple this.
+		CConfigDB::Initialise();
+		CConfigDB::Instance()->SetValueString(CFG_SYSTEM, "rendererbackend", "dummy");
+		g_VideoMode.InitNonSDL();
+		g_VideoMode.CreateBackendDevice(false);
+		m_Viewer = std::make_unique<CProfileViewer>();
+		m_Renderer = std::make_unique<CRenderer>(g_VideoMode.GetBackendDevice());
+	}
+
+	void tearDown()
+	{
+		m_Renderer.reset();
+		m_Viewer.reset();
+		g_VideoMode.Shutdown();
+		CConfigDB::Shutdown();
+		CXeromyces::Terminate();
+		g_VFS.reset();
+		DeleteDirectory(DataDir() / "_testcache");
+	}
+
+	void test_movability()
+	{
+		CGUI gui(g_ScriptContext);
+		TestGUIObject object(gui);
+
+		static_assert(std::is_move_constructible_v<CGUISimpleSetting<CStr>>);
+		static_assert(!std::is_move_assignable_v<CGUISimpleSetting<CStr>>);
+
+		CGUISimpleSetting<CStr> settingA(&object, "A");
+		TS_ASSERT(settingA->empty());
+		TS_ASSERT(object.SettingExists("A"));
+		object.SetSettingFromString("A", L"ValueA", false);
+		TS_ASSERT_EQUALS(*settingA, "ValueA");
+
+		CGUISimpleSetting<CStr> settingB(std::move(settingA));
+		TS_ASSERT(object.SettingExists("A"));
+		object.SetSettingFromString("A", L"ValueB", false);
+		TS_ASSERT_EQUALS(*settingB, "ValueB");
+	}
+};