Port to python3 and add functionality #11
Conversation
Two Python files did not have executable permissions. Shouldn't cause any symptom, but was irritating. Signed-off-by: James Cameron <[email protected]>
- camel case the game class name, - rename source file to match lowercase class name, - rename instance variable to match Sugargame test activity Signed-off-by: James Cameron <[email protected]>
Sugargame test activity does not do this. Was also found in other activities, such as Bridge and Jumble, it changes the order of creation of the X11 windows. The earlier port of thie activity to Sugargame 1.3 has not properly followed the new API and test activity. Signed-off-by: James Cameron <[email protected]>
EPERM is reported when the activity is run under Sugar when the source code is in /usr/share/sugar/activities
- name the run method the same as the example program, - use a running flag rather than a crashed flag; crashed could be misunderstood as referring to the cars, and running is how the example program is phrased, - change initialisation of the display, - properly exit the event loop if the stop button is pressed, - handle all events in the message queue in one place. Tested on Ubuntu 20.04 with Sugar. Mostly works. Signed-off-by: James Cameron <[email protected]>
Dimi20cen
changed the title
There was a problem hiding this comment.
If these changes are from Sourabha's PR then that PR contains 13 commits which it looks like you've merged all to one without retaining the commit authors.
If you intend to continue the work on that PR then fetch the PR and then continue on top of the latest commit.
|
Tested 27d2652, arrow keys no longer work and only mouse buttons work. |
|
In my environments it works just fine, seems to be the same issue I faced with flappy's toolbar not showing |
|
Tested 27d265, Arrow keys are working for me |
Should I do that now? I think it's possible
I can't figure out how to approach flake8 for python2, can you help me? I did an attempt #13 |
|
I can't tell if this set of changes is based on the earlier pull requests by sourabhaa or ayushnawal, or if it discards all that work we(-all) did in favour of doing it over again. Can you explain if you used the earlier work and just forgot to use "git commit --author", or if you just didn't notice it? Looking at the flake8 fixes for python2 seems unhelpful unless you plan to rebase your branch on top of them. Is that what you plan to do? |
The changes in my first commit were based on the sourabhaa's repo, I honestly didn't know "git commit --author" was a thing. That's why in my commit message I wrote, "Using the already ported to Python3 fork from Sourabhaa's repo.".
No, I don't think I want to rebase my branch on top of them. I thought it might be helpful for some other reason, since it was suggested. |
We usually use git to attribute developer work, and the format used by git for author ensures that the developer is identified properly. Sourabhaa is a GitHub username, and that isn't stored in git. Perhaps you could rebase your branch on their branch? That will ensure their commits are present.
I expect Ibiam did this to keep flake8 work separate from more substantial changes. It is much harder to review a pull request or especially a commit where there is obvious confusion about what the commit is for; and mixing flake8 work with substantial change is an example of that. In our guide to reviewers in the contributing guide, we have summarised this as a style issue;
Perhaps you need to do some reviews yourself, to better understand the process. Have you done any? |
Aim is to improve readability mainly by - Use more descriptive naming for variables and functions - Breaking up large functions - Adding global variables Other changes - Incorporate score_path inside scorescreen - Fill space that was unused by pygame with black color
Make the following adjust to screen size - Background size - Squares' and circles' size - Cars' size - Font size - Perceived speed of cars
- Add home button image - Use the same code that's used for restart button
When pause button is clicked or space button is pressed game is paused and screen darkens Changes - Add pause button image - Display button at the top left - Add handling for when pause is clicked - Add handling for when space key is press - Add method for when pause button or space key is pressed
|
Thanks. Reviewed to 988afb2. |
This change also fixes the following Traceback that was caused when using the activity with newer pygame versions
- Traceback (most recent call last):
File "/home/dim/Activities/2-cars-activity/sugargame/event.py", line 128, in _keydown_cb
return self._keyevent(widget, event, pygame.KEYDOWN)
File "/home/dim/Activities/2-cars-activity/sugargame/event.py", line 169, in _keyevent
mod = self._keymods()
File "/home/dim/Activities/2-cars-activity/sugargame/event.py", line 145, in _keymods
mod |= self.__keystate[key_val] and mod_val
IndexError: list index out of range
|
Arrow keys should be working with the updated version of sugargame. @chimosky |
| if (pause_rect.collidepoint(mouse_x, mouse_y) and | ||
| event.type == pygame.MOUSEBUTTONUP and | ||
| event.button == 1): |
There was a problem hiding this comment.
No need for the outer parentheses.
There was a problem hiding this comment.
Without the outer parentheses, I would get SyntaxError: invalid syntax
There was a problem hiding this comment.
Use \ to indicate that the line doesn't end there.
The refactor in ca712d3 isn't also needed here as it's not related to this PR, another PR for that would be helpful. I read that in the guide, but in this specific case, I fixed flakes and refactored the code and then on top of those I added new features. So my features were build on top of the flake fixes and the refactoring. Wouldn't the only other choice be to make a PR for the flakes, then a PR on top of it for the refactoring and then another PR on top of them for the added features? Wouldn't that create too much complexity? I also had in mind that in the case of this small project that would be to confusing. |
Mainly - Add empty lines above comments - Rename self.soon_to_draw to pending_element - Set line to execute when `if event.type == pygame.QUIT:` to `self.running = False`
My bad, I should've explained earlier, this PR should be for Port to Python 3, any new functionality should be in another PR, same goes for any other change that's not a Port to Python 3. |
@chimosky @sourabhaa