Skip to content

Incorrect error message for vectors #3066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Starbuck5 opened this issue Aug 19, 2024 · 8 comments · Fixed by #3069
Closed

Incorrect error message for vectors #3066

Starbuck5 opened this issue Aug 19, 2024 · 8 comments · Fixed by #3069
Labels
bug Not working as intended math pygame.math

Comments

@Starbuck5
Copy link
Member

I was digging through the math.c source and found this:

>>> import pygame
pygame-ce 2.5.2.dev1 (SDL 2.30.6, Python 3.9.5)
>>> a = pygame.Vector2(30,40)
>>> del a.y
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Cannot delete the x attribute

I didn't try to delete the x attribute, I tried to delete the y attribute!

Implementation code (unchanged for 8 years):

static int
vector_set_component(pgVector *self, PyObject *value, int component)
{
    if (value == NULL) {
        PyErr_SetString(PyExc_TypeError, "Cannot delete the x attribute");
        return -1;
    }
    if (component >= self->dim) {
        PyErr_BadInternalCall();
        return -1;
    }

    self->coords[component] = PyFloat_AsDouble(value);
    if (PyErr_Occurred())
        return -1;
    return 0;
}
@Starbuck5 Starbuck5 added bug Not working as intended math pygame.math labels Aug 19, 2024
@damusss
Copy link
Member

damusss commented Aug 19, 2024

This is probably related to the CPython API (but finding an answer online is hard because google doesn't understand the difference between python and cpython), but why is the function for setting the component also handling deletion?

@aatle
Copy link
Contributor

aatle commented Aug 20, 2024

Hmm, so vector x, y, z, and w all call vector_set_component from their setter function, which only has one hard-coded error message.

Also I am no C or CPython expert, but why is there seemingly unused w coordinate stuff in the code?


@damusss I did a bit of research and found the relevant C-API stuff:

pygame Vector2 properties:

pygame-ce/src_c/math.c

Lines 2605 to 2609 in 3183d16

static PyGetSetDef vector2_getsets[] = {
{"x", (getter)vector_getx, (setter)vector_setx, NULL, NULL},
{"y", (getter)vector_gety, (setter)vector_sety, NULL, NULL},
{NULL, 0, NULL, NULL, NULL} /* Sentinel */
};

PyGetSetDef docs state:

setter set
Optional C function to set or delete the attribute. If NULL, the attribute is read-only.

@oddbookworm
Copy link
Member

Also I am no C or CPython expert, but why is there seemingly unused w coordinate stuff in the code?

Because it looks like someone at some point started implementing a Vector4 class, which doesn't fully exist

why is the function for setting the component also handling deletion?

https://docs.python.org/3/c-api/object.html#c.PyObject_SetAttr
Because that's a special-case for it, but it's deprecated behavior. I suggest moving to PyObject_DelAttr or PyObject_DelAttrString (which are both stable ABI as of 3.13 and both exist in all of our supported python versions)

@oddbookworm
Copy link
Member

In fact, looks like it might emit an error in the future as we have it python/cpython#106572 (comment)

@Starbuck5
Copy link
Member Author

It handles deletion because that’s how the Python C API works.

This is not about pyobject_setattr.

@oddbookworm
Copy link
Member

It handles deletion because that’s how the Python C API works.

This is not about pyobject_setattr.

It actually is. The docs and that cpython comment indicate that the behavior is deprecated and you shouldn't have PyObject_SetAttr do your deleting anymore

@oddbookworm
Copy link
Member

Hold on, I'm dumb

@damusss
Copy link
Member

damusss commented Aug 20, 2024

Thanks @aatle and @Starbuck5 for the info :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended math pygame.math
Projects
None yet
4 participants