Typing issues and incorrect comment

[bunny-therapist]

trafficstars

Enhancement request:

I saw this in sprite.py ( https://github.com/pythonarcade/arcade/blob/1a17591c54083a0cc16e6f351f1828157b26acb9/arcade/sprite/sprite.py#L207):

@hit_box.setter
def hit_box(self, hit_box: Union[HitBox, RotatableHitBox]):
    if type(hit_box) == HitBox:
        self._hit_box = hit_box.create_rotatable(self.angle)
    else:
        # Mypy doesn't seem to understand the type check above
        # It still thinks hit_box can be a union here
        self._hit_box = hit_box  # type: ignore

That comment blaming mypy is just wrong. The if-statement checks exact type equality. Something could be a subclass of HitBox (like indeed RotatableHitBox is) and the if statement would be False and we would go to the else statement. A subclass of a HitBox is still a HitBox, which is why mypy is right.

What should be added/changed?

The correct way to implement this function would be to swap the if and the else block and do if isinstance(hit_box, RotatableHitBox):. Then you would do one thing for RotatableHitBox (or subclasses thereof) and another thing for HitBoxes which are not RotatableHitBoxes.

What would it help with?

Correct type hinting, no type: ignore comment. Also would allow callers to subclass HitBox or RotatingHitBox and have the code work properly (type check using type() does not allow for subclasses).

[cspotcode]

What you're saying makes sense. It looks like we should even replace Union[HitBox, RotatableHitBox] with HitBox

I had initially proposed a design that did not use subclassing. https://discord.com/channels/458662222697070613/1085299745116917851/1086064696970465370

I had been imagining that AdjustableHitBox stores a reference to HitBox so that it doesn't need to duplicate the non-adjusted points array.

So Sprite internally stores 2x references, one to HitBox which it can share with Texture, and another to AdjustableHitBox which is purely internal

I had been imagining that the correspondence would be: Today's Sprite's self._hit_box_points is replaced with an instance of HitBox Today's Sprite's self._hit_box_points_cache is replaced with an instance of AdjustableHitBox

[einarf]

This was fixed 16 months ago in a typing pass 😆