Pong example in documentation gives incorrect order of operation, constructor signatures, etc

[Poikilos]

What doesn't work?
multiple exceptions during several debugging steps:

• order of operation (wrong instructions)
• constructor signatures change. Steps are hard to follow, causing either (or both separately):
  • redefinition (putting new code in again)
  • missed refactors (not putting new code in because you already have the class and think it is just "nearby" code there for context)

Code instructions that are partial are difficult to follow without multiple monitors (or printing the instructions). It is even more difficult when the context clues (explanations and/or "nearby" code) are not clear.

To me, it really seems like there was refactoring or reordering behind the scenes (being done while the tutorial was created). I really just do not see the classes and even order of operation that is in the "nearby" code after I have completed a previous step.

When I write a book I sometimes blur out code that is not meant to be typed. I also explain exactly where to put the code. In these instructions, there is only "nearby" code to go by. If you type the new code where it appears you should, it is not possible to do right without several reordering steps (and fixes to shadowing).

How To Reproduce
Follow https://pysdl2.readthedocs.io/en/0.9.13/tutorial/pong.html to the letter.

• Constructor signatures keep changing
• Object-oriented delegation of responsibility keeps changing
• Order of operation is wrong (Reading what is before [...] leads to putting code in wrong place, in multiple instances).

Redefinition example

(has a regression in middle step, so I'd say the instructions there are illogical in the sense of order of explanation)

A.

class Player(sdl2.ext.Entity):
    def __init__(self, world, sprite, posx=0, posy=0):
        self.sprite = sprite
        self.sprite.position = posx, posy

changes to
B.

class Player(sdl2.ext.Entity):
    def __init__(self, world, posx=0, posy=0):
        [...]
        self.velocity = Velocity()

then to
C.

class Player(sdl2.ext.Entity):
    def __init__(self, world, sprite, posx=0, posy=0, ai=False):
        self.sprite = sprite
        self.sprite.position = posx, posy
        self.velocity = Velocity()
        self.playerdata = PlayerData()
        self.playerdata.ai = ai

Therefore there is for certain no way to actually follow the tutorial as stated (there is a regression in "B" to Player above--sprite goes away). This also demonstrates a telescoping constructor anti-pattern. Changing sequential arguments, especially in Python without type hinting, causes obscure errors that are difficult to debug (often an exception is raised in a deeper library call instead of a TypeError being raised earlier like it should be). Even for a professional, the person who knows what the values should be may not work there anymore and may not have documented it. This requires extensive reading of code, usage, specifications, and tribal knowledge [guy y knows what guy x knew about the hardware] in some cases. Sorry if this is wordy but I feel the overall issue is important because as you may tell this is a true story 😢.

The bug is either by omission or redefinition when trying to follow context clues, as described earlier.

If there is redefinition such as by accidentally pasting a class more than once (instructions have several versions of several classes, I have been doing Python for 14 years and didn't catch it).

• Maybe person is beginner and would never figure it out and give up.
• Maybe person is experienced doesn't have linter working (Ruff is not working on my install of VS Code on Linux Mint 22.1 right now) and won't see class redefinition issue.
• If person redefines more than one, which is easy to do in these instructions, fixing the issue is very difficult.

Incorrect code

I know it is just a preview of a suggested implementation, but it is not even correct for Python-like pseudocode:

StaticRepeatingSprite(Entity):
    ...
    self.positions = Positions((400, 0), (400, 60), (400, 120), ...)
    ...

I can only guess it should be something like:

class StaticRepeatingSprite(sdl2.ext.Entity):
    ...
    def __init__(self, positions):
        self.positions = positions
    ...

repeatingsprite = StaticRepeatingSprite(
    Positions((400, 0), (400, 60), (400, 120), ...)
)

Context clue ambiguity example

• I never added collision.ball = ball because it looked like just "nearby" code, and another object also manages "ball"
  • aicontroller.ball = ball is missing from the "nearby" code, so it doesn't look like anything should be added there. The context clues in here are not clear here:

    [...]
    collision.ball = ball

    running = True

looks alot like the existing code

    aicontroller.ball = ball

    running = True

so maybe just change it to:

    aicontroller.ball = ball
    collision.ball = ball

    running = True

Platform (if relevant):

• OS: Linux Mint 22.1
• Python Version: 3.12.3
• SDL2 Version: N/A
• Using pysdl2-dll: No

Additional context
Add any other context about the problem here.

I have attached the bad code I have at the end of the tutorial:
example-pysdl2-pong-nonworking.py.zip

1. One bug not included was a failure to read instructions carefully, so I didn't include it in the "nonworking" version, but I want to mention it for demonstration of the issue: I accidentally pasted Player's __init__ under PlayerData, causing another missing arguments error.
2. There was also redefinition of Player that messed things up, but I didn't include that because I already explained it.

Proposed solutions

In the attached code, the only bug left is the missing collision.ball = ball as explained above. However, I'd say the following are important to review--I'll try to distill all of the above into concrete changes you can make, so that my report is the most helpful:

• Make context clues clearer is important to review, especially regarding logic (Redefinition example)
• Make sure all code actually matches a working program at each step (Context clue ambiguity example).
• Fix "Incorrect code" example as suggested
• Mitigate telescoping constructor anti-pattern. At minimum, do not change the meaning of the third argument between Sprite and int.
• Consider improving delegation of responsibility:
  • 2 classes must have ball manually added--not too bad but maybe a missed opportunity for refactor (or arg should be removed):
    • world is unused in process method of CollisionSystem, TrackingAIController, Player
    • player2 is unused in run
• Consider better use of snake case (use undercores to separate words): This is especially helpful for people with English as a second language or for Code Spell Check to work.

Working version with fixed missing line, and snake case--All Code Spell Check issues are fixed other than library calls, including componenttypes and playerdata required by sdl2:

example-pysdl2-pong-working.py.zip

I also do not understand why a library that is a wrapper for sdl2 requires such a specific attribute as "playerdata". If I change it to player_data I get:

Traceback (most recent call last):
  File "/home/redacted/git/python-sdl3ui/.venv/lib/python3.12/site-packages/sdl2/ext/ebs.py", line 53, in __getattr__
    ctype = self._world._componenttypes[name]
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
KeyError: 'player_data'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/redacted/git/python-sdl3ui/examples/example-pysdl2-pong.py", line 220, in <module>
    sys.exit(run())
             ^^^^^
  File "/home/redacted/git/python-sdl3ui/examples/example-pysdl2-pong.py", line 191, in run
    player1 = Player(world, sp_paddle1, 0, 250)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/redacted/git/python-sdl3ui/examples/example-pysdl2-pong.py", line 64, in __init__
    self.player_data.ai = ai
    ^^^^^^^^^^^^^^^^
  File "/home/redacted/git/python-sdl3ui/.venv/lib/python3.12/site-packages/sdl2/ext/ebs.py", line 55, in __getattr__
    raise AttributeError("object '%r' has no attribute '%r'" % \
AttributeError: object ''Player'' has no attribute ''player_data''

• Fix the double traceback. Either don't require such a specific thing as playerdata, allow other attributes. Also fix the code in the exception handler to not raise another exception, and make the text of the first exception more clear (instruct developer on what is required by what).

You might argue that these problems give a beginner valuable experience debugging. I would not agree with that in this case. They are trusting you as the person knowledgeable with the domain (py-sdl2 in this case) that your instructions produce a working program at each step or at the very least, a working program at each subsection (if explained well as being part of the same feature addition). Even a Python pro trusts you to ensure that. If you do, you can be sure that the instructions are logical and factually correct.

[a-hurst]

@Poikilos Thanks so much for this detailed bug report, apologies for taking so long to get around to it!

I took over maintenance of PySDL2 from its original author in 2020, and this example and the tutorials are one of the areas I haven't had time to give an overhaul since taking over. I also don't have much personal experience with the style of component-oriented programming that this example relies on (my personal PySDL2 use case doesn't need it), so a detailed review like this is super-helpful because I don't have much expertise of my own to fall back on here.

Based on your feedback I'll do my best to revise and clean up this example for the next release! I realize I also need to update the helloworld.py example too since I've revised the script for that example a fair bit and I haven't touched the tutorial code.