From 4d3dfce93f207c8dfec5e979dc61e84e4397c18f Mon Sep 17 00:00:00 2001
From: King_DuckZ <king_duckz@gmx.com>
Date: Sun, 15 May 2022 17:01:57 +0200
Subject: [PATCH] Improve and cleanup make_foreign_class()

No need to have two versions of this.
Static assert when users specify ambiguous overloads.
Currently overloads are discriminated only by
parameter count, not types, so we need to enforce this.
---
 examples/math_vector/main.cpp                 | 39 ++++++++++++++-----
 .../wrenpp/detail/construct_foreign_class.hpp | 38 +++++++++++++++---
 2 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/examples/math_vector/main.cpp b/examples/math_vector/main.cpp
index 0cdd0e1..a7e187b 100644
--- a/examples/math_vector/main.cpp
+++ b/examples/math_vector/main.cpp
@@ -21,30 +21,38 @@
 #include "wrenpp/class_manager.hpp"
 #include <string_view>
 #include <algorithm>
-#include <ctime>
-#include <memory>
-#include <vector>
-#include <cstring>
 #include <iostream>
 
 namespace {
 	constexpr std::string_view g_math_vector_src{
 "foreign class MathVector {" R"script(
+	construct new() {}
 	construct new(value) {}
 	construct new(x, y, z) {}
 
 	foreign x
 	foreign y
 	foreign z
+
+	foreign x=(value)
+	foreign y=(value)
+	foreign z=(value)
 }
 )script"};
 
 	constexpr const char g_test_script[] =
 R"script(import "math_vector" for MathVector
-var vec1 = MathVector.new(1.0)
+var vec1 = MathVector.new(15.0)
 var vec2 = MathVector.new(10.0, 20.0, 30.0)
-System.print("vec1: <%(vec1.x), %(vec1.y), %(vec1.z)>")
-System.print("vec2: <%(vec2.x), %(vec2.y), %(vec2.z)>")
+var vec3 = MathVector.new()
+System.print("vec1 constructed from single value: <%(vec1.x), %(vec1.y), %(vec1.z)>")
+System.print("vec2 constructed from three values: <%(vec2.x), %(vec2.y), %(vec2.z)>")
+System.print("vec3 default constructed: <%(vec3.x), %(vec3.y), %(vec3.z)>")
+
+vec3.x = 3.5
+vec3.y = vec2.x / vec2.z
+vec3.z = vec1.x + vec2.y / 4.0
+System.print("vec3 modified by scripting: <%(vec3.x), %(vec3.y), %(vec3.z)>")
 )script";
 
 	class MathVector {
@@ -61,9 +69,12 @@ System.print("vec2: <%(vec2.x), %(vec2.y), %(vec2.z)>")
 		double x() const { return m_x; }
 		double y() const { return m_y; }
 		double z() const { return m_z; }
+		void set_x(double x) { m_x = x; }
+		void set_y(double y) { m_y = y; }
+		void set_z(double z) { m_z = z; }
 
 	private:
-		double m_x, m_y, m_z;
+		double m_x{}, m_y{}, m_z{};
 	};
 
 	class MyConf : public wren::DefConfiguration {
@@ -82,14 +93,24 @@ System.print("vec2: <%(vec2.x), %(vec2.y), %(vec2.z)>")
 
 int main() {
 	using wren::make_method_bindable;
+	using wren::make_foreign_class;
 
 	MyConf conf;
 	wren::VM vm(&conf, nullptr);
 	vm.callback_manager()
+		.add_callback(false, "math_vector", "MathVector", "x=(_)", make_method_bindable<&MathVector::set_x>())
+		.add_callback(false, "math_vector", "MathVector", "y=(_)", make_method_bindable<&MathVector::set_y>())
+		.add_callback(false, "math_vector", "MathVector", "z=(_)", make_method_bindable<&MathVector::set_z>())
 		.add_callback(false, "math_vector", "MathVector", "x", make_method_bindable<&MathVector::x>())
 		.add_callback(false, "math_vector", "MathVector", "y", make_method_bindable<&MathVector::y>())
 		.add_callback(false, "math_vector", "MathVector", "z", make_method_bindable<&MathVector::z>());
-	vm.class_manager().add_class_maker("math_vector", "MathVector", &wren::make_overloaded_foreign_class<MathVector, double, std::tuple<double, double, double>>);
+
+	vm.class_manager()
+		.add_class_maker("math_vector", "MathVector", &make_foreign_class<MathVector,
+			double, //single value constructor
+			std::tuple<double, double, double>, //x,y,z constructor
+			std::tuple<> //default constructor
+		>);
 
 	vm.interpret("main", g_test_script);
 
diff --git a/include/wrenpp/detail/construct_foreign_class.hpp b/include/wrenpp/detail/construct_foreign_class.hpp
index 3c547eb..616d5e6 100644
--- a/include/wrenpp/detail/construct_foreign_class.hpp
+++ b/include/wrenpp/detail/construct_foreign_class.hpp
@@ -71,6 +71,16 @@ namespace wren {
 			}
 		};
 
+		template <typename T>
+		struct ForeignParamHelper {
+			static constexpr std::size_t argument_count = 1;
+		};
+
+		template <template <typename...> class Pack, typename... PackArgs>
+		struct ForeignParamHelper<Pack<PackArgs...>> {
+			static constexpr std::size_t argument_count = sizeof...(PackArgs);
+		};
+
 		template <typename T>
 		struct ForeignClassHelper<T> {
 			static bool construct (VM& vm, int cardinality, void* memory) {
@@ -95,10 +105,31 @@ namespace wren {
 				ForeignClassHelper<T, Args...>::construct(vm, vm.slot_count() - 1, memory);
 			}
 		}
+
+		template <std::size_t Search, std::size_t... Values>
+		struct AssertIfDuplicateValues {
+			static_assert(sizeof...(Values) == 0, "Code bug?");
+		};
+
+		template <std::size_t Search, std::size_t Value, std::size_t... Values>
+		struct AssertIfDuplicateValues<Search, Value, Values...> : AssertIfDuplicateValues<Search, Values...>, AssertIfDuplicateValues<Value, Values...> {
+			static_assert(Search != Value, "Constructor overloads with the same argument counts are not currently allowed");
+			static constexpr std::size_t value = 1;
+		};
+
+		template <typename... Args>
+		constexpr void static_assert_all_packs_are_unique() {
+			if constexpr (sizeof...(Args) > 0) {
+				constexpr auto dummy = AssertIfDuplicateValues<ForeignParamHelper<Args>::argument_count...>::value;
+				static_assert(dummy == 1); //always true
+			}
+		}
 	} //namespace detail
 
 	template <typename T, typename... Args>
-	inline foreign_class_t make_overloaded_foreign_class() {
+	inline foreign_class_t make_foreign_class() {
+		detail::static_assert_all_packs_are_unique<Args...>();
+
 		foreign_class_t ret;
 		ret.allocate = [](VM& vm) {
 			void* const mem = vm.set_slot_new_foreign(0, 0, sizeof(T));
@@ -116,9 +147,4 @@ namespace wren {
 
 		return ret;
 	}
-
-	template <typename T, typename... Args>
-	inline foreign_class_t make_foreign_class() {
-		return make_overloaded_foreign_class<T, std::tuple<Args...>>();
-	}
 } //namespace wren