danicat / pacgo

A Pac Man clone written in Go (with emojis!)
MIT License
1.2k stars 141 forks source link

subtle concurrency issue in step 6 #38

Open rpoe opened 2 years ago

rpoe commented 2 years ago

In step 6 channels are introduced for processing the user input in the game loop. At the end of the step a delay with sleep is added to the game loop. This introduces the situation, that in each run through the loop the ghosts are moving and in some runs through the loop the player is moving. If before the loop both are side by side and in the loop both are moving in the opposite direction towards each other, the player can survive the hit of the ghost. The issue is the simultaneous use of a select from a channel and a sleep. The fix for this is to use a ticker for the delay and not a sleep. the ticker needs to write to the same channel as the player input. This way the for loop will process only one of this state changes of the game at a time. I find it important to mention this subtle concurrency issue in the presentation, so students learn correct use of concurrency.

rpoe commented 2 years ago

Can we setup a branch where I can address this issue? I do not have the permission to create a branch.

danicat commented 2 years ago

This is a known issue and for the sake of simplicity I left this bug unfixed (some might even call it a "feature" - if you are skilled/lucky enough you can escape a pinch :-).

The main point is that I didn't want this early chapter to become a full treaty on concurrency. I think it's worth to add it as a note and treat it in the right way in a subsequent chapter.

This new chapter could potentially talk about tooling as well, including go test -race for example. If you are interested in working in something like this what I recommend is to fork the project, make the changes to your fork and submit a PR for review.