Merge pull request #3078 from pygame-community/ankith26-scale-by-arg

Fix `(F)Rect.scale_by(_ip)` handling of the `scale_by` parameter
pygame-community · Oct 5, 2024 · 295a033 · 295a033
2 parents 1a28571 + 0299629
commit 295a033
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 23 deletions.
diff --git a/docs/reST/ref/rect.rst b/docs/reST/ref/rect.rst
@@ -175,6 +175,8 @@
 
       .. versionadded:: 2.3.1
 
+      .. versionchanged:: 2.5.2 the argument ``scale_by`` can now be passed as a positional argument
+
       .. ## Rect.scale_by ##
 
    .. method:: scale_by_ip
@@ -187,6 +189,8 @@
 
       .. versionadded:: 2.3.1
 
+      .. versionchanged:: 2.5.2 the argument ``scale_by`` can now be passed as a positional argument
+
       .. ## Rect.scale_by_ip ##
 
    .. method:: update

diff --git a/src_c/rect_impl.h b/src_c/rect_impl.h
@@ -1084,31 +1084,40 @@ static PyObject *
 RectExport_scalebyIp(RectObject *self, PyObject *args, PyObject *kwargs)
 {
     float factor_x, factor_y = 0;
-
-    static char *keywords[] = {"x", "y", NULL};
-
-    if (kwargs) {
-        PyObject *scale_by = PyDict_GetItemString(kwargs, "scale_by");
-        float temp_x, temp_y = 0;
-
-        if (scale_by) {
-            if (PyDict_Size(kwargs) > 1) {
-                return RAISE(PyExc_TypeError,
-                             "The 'scale_by' keyword cannot be combined with "
-                             "other arguments.");
+    PyObject *scale_by =
+        kwargs ? PyDict_GetItemString(kwargs, "scale_by") : NULL;
+    if (scale_by) {
+        if (PyDict_Size(kwargs) > 1) {
+            return RAISE(PyExc_TypeError,
+                         "The 'scale_by' keyword cannot be combined with "
+                         "other arguments.");
+        }
+        if (!pg_TwoFloatsFromObj(scale_by, &factor_x, &factor_y)) {
+            return RAISE(PyExc_TypeError,
+                         "The 'scale_by' argument must be a number pair");
+        }
+    }
+    else {
+        static char *keywords[] = {"x", "y", NULL};
+        PyObject *arg_x, *arg_y = NULL;
+        if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|O", keywords, &arg_x,
+                                         &arg_y)) {
+            return NULL;
+        }
+        if (!pg_TwoFloatsFromObj(arg_x, &factor_x, &factor_y)) {
+            /* Not a sequence, so try handling int separately */
+            if (!pg_FloatFromObj(arg_x, &factor_x)) {
+                return RAISE(PyExc_TypeError, "Argument 'x' must be a number");
             }
-            if (!pg_TwoFloatsFromObj(scale_by, &temp_x, &temp_y)) {
-                return RAISE(PyExc_TypeError, "number pair expected");
+            if (arg_y && !pg_FloatFromObj(arg_y, &factor_y)) {
+                return RAISE(PyExc_TypeError, "Argument 'y' must be a number");
             }
-            PyDict_SetItemString(kwargs, "x", PyFloat_FromDouble(temp_x));
-            PyDict_SetItemString(kwargs, "y", PyFloat_FromDouble(temp_y));
-            PyDict_DelItemString(kwargs, "scale_by");
         }
-    }
-
-    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "f|f", keywords, &factor_x,
-                                     &factor_y)) {
-        return RAISE(PyExc_TypeError, "Float values expected.");
+        else if (arg_y) {
+            return RAISE(
+                PyExc_TypeError,
+                "Cannot pass argument 'y' after passing a sequence scale");
+        }
     }
 
     factor_x = factor_x < 0 ? -factor_x : factor_x;
@@ -1130,7 +1139,11 @@ RectExport_scaleby(RectObject *self, PyObject *args, PyObject *kwargs)
 {
     RectObject *returnRect = (RectObject *)RectExport_subtypeNew4(
         Py_TYPE(self), self->r.x, self->r.y, self->r.w, self->r.h);
-    RectExport_scalebyIp(returnRect, args, kwargs);
+    PyObject *tmp = RectExport_scalebyIp(returnRect, args, kwargs);
+    if (!tmp) {
+        return NULL;
+    }
+    Py_DECREF(tmp);
     return (PyObject *)returnRect;
 }
 

diff --git a/test/rect_test.py b/test/rect_test.py
@@ -893,6 +893,8 @@ def test_scale_by__larger_kwargs_scale_by(self):
         r = Rect(2, 4, 6, 8)
         # act
         r2 = r.scale_by(scale_by=(2, 4))
+        r3 = r.scale_by((2, 4))
+        self.assertEqual(r2, r3)
         # assert
         self.assertEqual(r.center, r2.center)
         self.assertEqual(r.left - 3, r2.left)
@@ -3091,6 +3093,8 @@ def test_scale_by__larger_kwargs_scale_by(self):
         r = FRect(2.1, 4, 6, 8.9)
         # act
         r2 = r.scale_by(scale_by=(2, 4))
+        r3 = r.scale_by((2, 4))
+        self.assertEqual(r2, r3)
         # assert
         self.assertSeqAlmostEqual5(r.center, r2.center)
         self.assertAlmostEqual5(r.left - 3, r2.left)