Author Topic: Squirrel 3.1 and new bindings  (Read 9240 times)

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: Squirrel 3.1 and new bindings
« Reply #30 on: May 18, 2021, 09:15:14 am »
I've tried it. I don't remember seeing any problems.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: Squirrel 3.1 and new bindings
« Reply #31 on: May 18, 2021, 09:49:13 am »
The tests for upper and lower aren't good and I've failed to implement them correctly. Will fix soon.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: Squirrel 3.1 and new bindings
« Reply #32 on: May 18, 2021, 10:18:30 am »
See the attached patch:
Code
From 34494a8d7405e211f74101f3989651f340b02157 Mon Sep 17 00:00:00 2001
From: T Petrov <[email protected]>
Date: Tue, 18 May 2021 11:14:18 +0300
Subject: [PATCH] * scripting: Fix regressions in the wxString upper/lower
 bindings

> Upper/Lower were bound incorrectly and they called MakeUpper/MakeLower,
  which modified 'this'.
> MakeUpper/MakeLower returned a copy of 'this' and so further
  modifications to the string were not possible. This means that chaining
  MakeUpper and MakeLower fails in a subtle way.
---
 src/scripts/tests/wx_test.script          | 30 +++++++++++++++++++++++
 src/sdk/scripting/bindings/sc_wxtypes.cpp | 28 +++++++++++++++------
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/src/scripts/tests/wx_test.script b/src/scripts/tests/wx_test.script
index 3268702da..f6259bc1d 100644
--- a/src/scripts/tests/wx_test.script
+++ b/src/scripts/tests/wx_test.script
@@ -72,14 +72,44 @@ class wxTest extends script_test_base
         test_equal("length",string.length(),5);
         test_equal("len",string.len(),5);
         test_equal("size",string.size(),5);
+
+        // Lower case functions
+        string = _T("Hallo");
         test_string("Lower",string.Lower(),"hallo");
+        test_string("Lower after",string,"Hallo");
         string.LowerCase();
         test_string("LowerCase",string,"hallo");
+
+        string = _T("HallO");
         test_string("MakeLower",string.MakeLower(),"hallo");
+        test_string("MakeLower",string,"hallo");
+
+        // Upper case functions
+        string = _T("Hallo");
         test_string("Upper",string.Upper(),"HALLO");
+        test_string("Upper after",string,"Hallo");
         string.UpperCase();
         test_string("UpperCase",string,"HALLO");
+
+        string = _T("Hallo");
         test_string("MakeUpper",string.MakeUpper(),"HALLO");
+        test_string("MakeUpper",string,"HALLO");
+
+        // Test if the return value of MakeUpper/MakeLower is a proper reference to this.
+        // Test if the return value of Lower/Upper is a copy and not reference to this.
+        string = _T("Hallo");
+        test_string("MakeLower Chaining", string.MakeLower().Upper(),"HALLO");
+        test_string("MakeLower Chaining 2", string, "hallo");
+        string = _T("Hallo");
+        test_string("MakeLower Chaining 3", string.MakeLower().MakeUpper(),"HALLO");
+        test_string("MakeLower Chaining 4", string, "HALLO");
+        string = _T("Hallo");
+        test_string("MakeUpper Chaining", string.MakeUpper().Lower(),"hallo");
+        test_string("MakeUpper Chaining 2", string, "HALLO");
+        string = _T("Hallo");
+        test_string("MakeUpper Chaining", string.MakeUpper().MakeLower(),"hallo");
+        test_string("MakeUpper Chaining 2", string, "hallo");
+
         string = _T("Hallo");
         test_string("Mid",string.Mid(2,2),"ll");
         test_string("Remove",string.Remove(2,string.len()-2),"Ha");
diff --git a/src/sdk/scripting/bindings/sc_wxtypes.cpp b/src/sdk/scripting/bindings/sc_wxtypes.cpp
index a9fde7ac0..0c2d6c045 100644
--- a/src/sdk/scripting/bindings/sc_wxtypes.cpp
+++ b/src/sdk/scripting/bindings/sc_wxtypes.cpp
@@ -288,13 +288,25 @@ SQInteger wxString_Length(HSQUIRRELVM v)
     return 1;
 }

-using wxStringDoSomethingAndReturnNewFunc = wxString& (wxString::*)();
+using wxStringMakeFunc = wxString& (wxString::*)();

-template<wxStringDoSomethingAndReturnNewFunc func>
-SQInteger wxString_NoParamReturnWxString(HSQUIRRELVM v)
+template<wxStringMakeFunc func>
+SQInteger wxString_Make(HSQUIRRELVM v)
 {
     ExtractParams1<wxString*> extractor(v);
-    if (!extractor.Process("wxString_NoParamReturnWxString"))
+    if (!extractor.Process("wxString_Make"))
+        return extractor.ErrorMessage();
+    wxString &ref = (extractor.p0->*func)();
+    return ConstructAndReturnNonOwnedPtr(v, &ref);
+}
+
+using wxStringCaseFunc = wxString (wxString::*)() const;
+
+template<wxStringCaseFunc func>
+SQInteger wxString_Case(HSQUIRRELVM v)
+{
+    ExtractParams1<wxString*> extractor(v);
+    if (!extractor.Process("wxString_Case"))
         return extractor.ErrorMessage();

     return ConstructAndReturnInstance(v, (extractor.p0->*func)());
@@ -1010,16 +1022,16 @@ void Register_wxTypes(HSQUIRRELVM v)
         BindMethod(v, _SC("len"), wxString_Length, _SC("wxString::len"));
         BindMethod(v, _SC("size"), wxString_Length, _SC("wxString::size"));

-        BindMethod(v, _SC("Lower"), wxString_NoParamReturnWxString<&wxString::MakeLower>,
+        BindMethod(v, _SC("Lower"), wxString_Case<&wxString::Lower>,
                    _SC("wxString::Lower"));
-        BindMethod(v, _SC("MakeLower"), wxString_NoParamReturnWxString<&wxString::MakeLower>,
+        BindMethod(v, _SC("MakeLower"), wxString_Make<&wxString::MakeLower>,
                    _SC("wxString::MakeLower"));
         BindMethod(v, _SC("LowerCase"), wxString_NoParamNoReturn<&wxString::LowerCase>,
                    _SC("wxString::LowerCase"));

-        BindMethod(v, _SC("Upper"), wxString_NoParamReturnWxString<&wxString::MakeUpper>,
+        BindMethod(v, _SC("Upper"), wxString_Case<&wxString::Upper>,
                    _SC("wxString::Upper"));
-        BindMethod(v, _SC("MakeUpper"), wxString_NoParamReturnWxString<&wxString::MakeUpper>,
+        BindMethod(v, _SC("MakeUpper"), wxString_Make<&wxString::MakeUpper>,
                    _SC("wxString::MakeUpper"));
         BindMethod(v, _SC("UpperCase"), wxString_NoParamNoReturn<&wxString::UpperCase>,
                    _SC("wxString::UpperCase"));
--
2.31.1
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 2987
Re: Squirrel 3.1 and new bindings
« Reply #33 on: May 18, 2021, 11:40:30 pm »
Yep, this patch fixes the error