Skip to content
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

Fixes and Release #12

Merged
merged 5 commits into from Jul 7, 2020
Merged

Fixes and Release #12

merged 5 commits into from Jul 7, 2020

Conversation

JuiP
Copy link
Member

@JuiP JuiP commented Jun 19, 2020

Fixed:

  • Pressing any key starts the Game. Now only Enter key can be used to start the game
  • Added a new key to return to Homescreen 'b' (back)
  • How to change and select the levels instruction was not clear
  • Fix: The text "Press Esc to exit" was out of screen and used hardcoded coordinates
  • Port from GObject to GLib
  • Fix Syntax Warnings
  • Change the text Color to white to improve visibility
  • Pressing any key after Game Over Restarts the game
    x/o have no meaning
    Added key y- yes and n-no after the Game Over screen

Next to work on #8
@chimosky @quozl @srevinsaju @Saumya-Mishra9129 kindly review changes till d368c01

@JuiP JuiP changed the title Dev Fixes and Release Jun 19, 2020
Copy link
Member

@Saumya-Mishra9129 Saumya-Mishra9129 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks , reviewed and tested as well. I got these syntax warning in log. You can easily fix them as well.

Screenshot from 2020-06-20 13-20-53

I am testing with 1920x1080 display size:

  • The text on screen doesn't fit well with black background , You can fix margins and make it working for all resolutions..Look here

Screenshot from 2020-06-20 13-19-40

  • I don't know exact what activity does , but when I started for first time, it shows LEVEL: 5 there. It should be LEVEL: 1.
  • The text on bottom right corner, i.e Press b for homescreen and Press ESC to exit also overlaps.

A suggestion for you- If you can change text colours here for the texts in the center, because they are not clearly visible with black background. You can try for white colour.

@JuiP
Copy link
Member Author

JuiP commented Jun 21, 2020

Thanks @Saumya-Mishra9129 😄

A suggestion for you- If you can change text colours here for the texts in the center, because they are not clearly visible with black background. You can try for white colour.

Implemented the suggestion 😉

I don't know exact what activity does , but when I started for first time, it shows LEVEL: 5 there. It should be LEVEL: 1.

It did look queer to me at the start, however I'm convinced it should start from Level 5 because the levels are to manage speed of the incoming blocks. I think it is important that we keep the default speed moderate instead of too slow (Level 1). Maybe try playing with Level 1 you will notice the speed is too slow.

The text on bottom right corner, i.e Press b for homescreen and Press ESC to exit also overlaps.

This was caused due to hardcoded coordinates, I have fixed it in d908cb1

I got these syntax warning in log. You can easily fix them as well.

Fixed them in c7c5c68

@JuiP
Copy link
Member Author

JuiP commented Jun 21, 2020

Mostly finished what I planned, do test and review my changes as of c7c5c68 and let me know of the bugs I may have skipped while testing,
My test method - Play one game for every level. The game is simple you can use the arrow keys to rotate the block that is falling and you lose when the last block touches the top edge of the black rectangle. Try clicking random keyboard keys to notice if something thats not expected happens.
@Saumya-Mishra9129 @srevinsaju @chimosky :)

I think we need to release the activity soon, there were substantial changes since the last released version

Copy link
Member

@srevinsaju srevinsaju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a code review; looks good. Will do an activity testing soon

BlockParty.py Outdated Show resolved Hide resolved
BlockParty.py Outdated Show resolved Hide resolved
Copy link
Member

@srevinsaju srevinsaju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested as of d368c01; No errors in logs. :

BlockParty.py Outdated Show resolved Hide resolved
BlockParty.py Outdated Show resolved Hide resolved
@chimosky
Copy link
Member

@JuiP said

It did look queer to me at the start, however I'm convinced it should start from Level 5 because the levels are to manage speed of the incoming blocks. I think it is important that we keep the default speed moderate instead of too slow (Level 1). Maybe try playing with Level 1 you will notice the speed is too slow.

I disagree because our focus is on kids and a kid playing a game starting from level 5 wouldn't make sense to them.
The idea of going from too slow to fast is so a kid can start with something that's comfortable and then move on to something that's more difficult.

@srevinsaju
Copy link
Member

@chimosky Level 5 is very slow already. Level 1 makes it even slower. I am not sure, but the Snake game (aka nibble), specifically Snake II starts at level 3 by default. A player can go to level 1 if necessary, The levels in that particular game starts at 1 and ends at 8.

But,

fun fact: Tetris starts at level 1 :D

@chimosky
Copy link
Member

@srevinsaju said

@chimosky Level 5 is very slow already. Level 1 makes it even slower. I am not sure, but the Snake game (aka nibble), specifically Snake II starts at level 3 by default. A player can go to level 1 if necessary, The levels in that particular game starts at 1 and ends at 8.

Let me paraphrase, it's ideal to have a level 1 with a level 5 speed but not the other way around as you'll normally expect a game to start at level 1 and not level 5.

@quozl
Copy link
Contributor

quozl commented Jun 29, 2020

This activity was written by one of the original developers of Tetris. I'm very unwilling to change speed of the game. I've seen very young children stare for minutes before figuring out how the game works. During this time their brain is on high alert, learning rapidly, and making neural networks for adaption to the game. Your experience that the game seems slow at first is mostly irrelevant. You aren't that age. If you did a study with 20 young children who had never seen any games, I'd pay attention to your opinions. 😁

@quozl
Copy link
Contributor

quozl commented Jun 29, 2020

  • Pressing any key starts the Game. Now only Enter key can be used to start the game

Why? When an activity upgrade happens, everyone familiar with pressing any key will be immediately frustrated.

  • Added a new key to return to Homescreen 'b' (back)

Why?

  • How to change and select the levels instruction was not clear
  • Fix: The text "Press Esc to exit" was out of screen and used hardcoded coordinates
  • Port from GObject to GLib
  • Fix Syntax Warnings
  • Change the text Color to white to improve visibility

Okay.

  • Pressing any key after Game Over Restarts the game

Why change this?

x/o have no meaning

They have meaning. Check earlier versions. The activity was written for the XO-1 laptop. In that context, X and O are also;

  • the left and right touchpad buttons respectively,
  • special function keys on the front face above the power button.

Added key y- yes and n-no after the Game Over screen

I've yet to evaluate this one.

@srevinsaju
Copy link
Member

This activity was written by one of the original developers of Tetris. I'm very unwilling to change speed of the game. I've seen very young children stare for minutes before figuring out how the game works. During this time their brain is on high alert, learning rapidly, and making neural networks for adaption to the game. Your experience that the game seems slow at first is mostly irrelevant. You aren't that age. If you did a study with 20 young children who had never seen any games, I'd pay attention to your opinions. grin

Makes sense. Then as @chimosky suggested, we should change the level to match Level 1. 😅

@JuiP
Copy link
Member Author

JuiP commented Jun 30, 2020

@quozl

  • Pressing any key starts the Game. Now only Enter key can be used to start the game

Why? When an activity upgrade happens, everyone familiar with pressing any key will be immediately frustrated.

I changed it to enter because the first screen that appears mentions "Enter to start". I think we should either have any key start the game and remove the text "Enter to start". Or have just Enter key to start the game.

  • Added a new key to return to Homescreen 'b' (back)

Why?

This way a player will be able to change the speed by switching levels. Before this change was made, the player has to complete a game for the start screen to display. If the player starts with a level and finds the game too fast/ too slow, he has to complete the game with the same speed set to be able to change the levels. But by adding the Homescreen button, the player no longer has to complete a game in case he/she wants to switch the levels and begin a new game with the changed speed.

x/o have no meaning

They have meaning. Check earlier versions. The activity was written for the XO-1 laptop. In that context, X and O are also;

  • the left and right touchpad buttons respectively,
  • special function keys on the front face above the power button.

Added key y- yes and n-no after the Game Over screen

I've yet to evaluate this one.

Thanks. I was not aware of the x/o key functions. Probably you are the best person to evaluate which of y/n or the x/o are to be kept. Do let me know if you have an opinion already :)

@srevinsaju

Makes sense. Then as @chimosky suggested, we should change the level to match Level 1. 😅

I kinda do not agree to this. By setting the Level 5 speed to Level 1, we remove the feature of letting the child slow down the game. Instead, we could just start with Level 1 as default rather than starting with Level 5 and let the speeds be the way they are now.

@srevinsaju
Copy link
Member

I kinda do not agree to this. By setting the Level 5 speed to Level 1, we remove the feature of letting the child slow down the game. Instead, we could just start with Level 1 as default rather than starting with Level 5 and let the speeds be the way they are now.

I mean setting the first level as the slowest possible. Not changing the speed of any level as of now. The game should start at 1, at the speed of 1, not 5.

@quozl
Copy link
Contributor

quozl commented Jun 30, 2020

Makes sense. Then as @chimosky suggested, we should change the level to match Level 1. sweat_smile

The default level is 5. The possible levels are 0 through to 9. The level advances to one fifth of the lines dismissed by the player. I don't agree to changing the default level or the advancement algorithm. I'm open to discussion, but the reasons given so far don't seem sufficient.

@quozl
Copy link
Contributor

quozl commented Jun 30, 2020

  • Pressing any key starts the Game. Now only Enter key can be used to start the game

Why? When an activity upgrade happens, everyone familiar with pressing any key will be immediately frustrated.

I changed it to enter because the first screen that appears mentions "Enter to start". I think we should either have any key start the game and remove the text "Enter to start". Or have just Enter key to start the game.

Thanks. I don't think that is sufficient reason. As long as the key that is in the text does start the game, that is sufficient.

  • Added a new key to return to Homescreen 'b' (back)

Why?

This way a player will be able to change the speed by switching levels. Before this change was made, the player has to complete a game for the start screen to display. If the player starts with a level and finds the game too fast/ too slow, he has to complete the game with the same speed set to be able to change the levels. But by adding the Homescreen button, the player no longer has to complete a game in case he/she wants to switch the levels and begin a new game with the changed speed.

Thanks. I don't think that is sufficient reason to add a way to escape from a bad block fall. The advancement algorithm is the primary constraint on level. The game will get as fast as it needs to for the performance of the player averaged over five dismissed lines.

x/o have no meaning

They have meaning. Check earlier versions. The activity was written for the XO-1 laptop. In that context, X and O are also;

Thanks. I was not aware of the x/o key functions. Probably you are the best person to evaluate which of y/n or the x/o are to be kept. Do let me know if you have an opinion already :)

x/o should not be kept in the message; at the moment any key is accepted. I suggest asking for enter to start, again.

@srevinsaju
Copy link
Member

srevinsaju commented Jun 30, 2020

Makes sense. Then as @chimosky suggested, we should change the level to match Level 1. sweat_smile

The default level is 5. The possible levels are 0 through to 9. The level advances to one fifth of the lines dismissed by the player. I don't agree to changing the default level or the advancement algorithm. I'm open to discussion, but the reasons given so far don't seem sufficient.

  • @chimosky wanted level 1 to be the first level
  • @quozl did not want to change the current / default level
  • me wanted to [keep level 5 as default] or [increase the speed of level 1 and make level 1 as default. ]

I prefer level 5 to be the default level. that's the reason I gave an example on #12 (comment) about how the Nibble game defaults to level 3, so I was mentioning how its not necessary to start at level 1.

@JuiP
Copy link
Member Author

JuiP commented Jun 30, 2020

@srevinsaju I agree. Like I said in this comment, I think we should leave the default level to 5 with no changes in speed.

So from the discussion so far, the work needed on this PR includes:

  • Revert changes made to Enter key (suggested by @quozl )

  • Remove the back button feature added (suggested by @quozl )

  • Make changes to y/n : Using Enter instead to start again. (suggested by @quozl )

  • use a Gdk.RGBA to parse colors instead of (0,0,0) (suggested by @chimosky )

  • merged to form single elif statement; (Suggested by @srevinsaju )

JuiP added 5 commits July 3, 2020 11:48
- How to change and select the levels instruction was not clear
- Fix: The text "Press Esc to exit" was out of screen.
- Removed hardcoded coordinates and used scaling factor
- Changed text color to white (Suggested by Saumya Mishra <saumyamishra9129853131@gmail.com>)
- Fix out of proportion text
Enter Key in text for game restart and use enter key to restart the game after "GAME OVER"
@JuiP
Copy link
Member Author

JuiP commented Jul 3, 2020

@quozl @chimosky @srevinsaju I have made the suggested changes :)
Do tell me if there are more changes required. I didn't get any errors when I tested so I'm assuming this PR is ready to merge.
Please test, review and merge :)

Copy link
Member

@srevinsaju srevinsaju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect 💯

@chimosky
Copy link
Member

chimosky commented Jul 6, 2020

@quozl said

They have meaning. Check earlier versions. The activity was written for the XO-1 laptop. In that context, X and O are also;

  • the left and right touchpad buttons respectively,
  • special function keys on the front face above the power button.

Yes they have meaning but I don't think they're needed anymore as this PR is in preparation for a release and that release contains a port to python3, they're still included in the last python2 version.

@chimosky
Copy link
Member

chimosky commented Jul 6, 2020

Tested, thanks.

Waiting for @quozl to test as he's involved.

@quozl
Copy link
Contributor

quozl commented Jul 7, 2020

Re: meaning. I was replying to the opening comment. No change required.

Thanks. Tested. Approved.

@chimosky
Copy link
Member

chimosky commented Jul 7, 2020

@quozl said

Re: meaning. I was replying to the opening comment. No change required.

Oh okay, thanks.

@chimosky chimosky merged commit a7ac4da into sugarlabs:master Jul 7, 2020
@JuiP JuiP mentioned this pull request Jul 21, 2020
@JuiP JuiP deleted the dev branch August 8, 2020 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants