sugarlabs / buttons-and-scissors-activity

A puzzle game for Sugar
GNU General Public License v3.0
0 stars 6 forks source link

fix 'two click needed' bug #17

Closed yell0wfl4sh closed 6 years ago

yell0wfl4sh commented 6 years ago

Fixes #12

quozl commented 6 years ago

Reviewed. Good try, but I disagree with your change. You don't explain it, and you leave check partially used.

As far as I can see, the problem is the mix of data sources used for deciding what to do; these are;

There is an asynchronous event loop. By the time the event is handled, pygame.mouse is desynchronised, and represents reality at a different time than event.

Instead, the pos and button attributes of event should be used exclusively, avoiding use of pygame.mouse. That way decisions can be made on the basis of a single point in time.

Reference:

yell0wfl4sh commented 6 years ago

The problem that I think is the fact that the condition if event.type == pygame.MOUSEBUTTONUP: is not able to occur. As soon as help or about button is clicked, another function is executed and MOUSEBUTTONUP event is not able occur, that leaves value of check to 1. therefore, it is waiting for another MOUSEBUTTONUP event, to reset value of check to 0, and then go to next screen.

quozl commented 6 years ago

That's strange. I'll test, and print a MOUSEBUTTONUP event.

quozl commented 6 years ago

Okay, I've instrumented the code.

The if event.type == pygame.MOUSEBUTTONUP does occur.

I suggest you test more carefully.

I'm still sure I'm right on the philosophy; pygame.mouse should not be used in combination with event like that, since event has the data at the right time and pygame.mouse is a query.

quozl commented 6 years ago

Got it working for just the Help button. Existing code is very repetitive, and could be done better with arrays of the critical data. That's what you get for trying to do a button GUI in PyGame. :grin:

See http://dev.laptop.org/~quozl/z/1eSGtW.txt for the "git diff".

Here's the fragment that deals with the Help button.

            if (event.type == pygame.MOUSEMOTION):
                eve_x, eve_y = event.pos
                help_active = helps.get_rect(center=(275 + 280, 545)).collidepoint(eve_x, eve_y)  # HELP

            if help_active:
                gameDisplay.blit(pygame.transform.scale(
                    helps, (90, 90)), (230 + 280, 500))
            else:
                gameDisplay.blit(helps, (230 + 280, 500))

            if (event.type == pygame.MOUSEBUTTONDOWN or
                event.type == pygame.MOUSEBUTTONUP) and event.button == 1:
                eve_x, eve_y = event.pos
                if helps.get_rect(center=(275 + 280, 545)).collidepoint(eve_x, eve_y):  # HELP
                    if event.type == pygame.MOUSEBUTTONDOWN:
                        print >>sys.stderr, 'help press'
                        if sound:
                            s1.play()
                        gameDisplay.fill(black)
                        ru = rule()
                        ru.make(gameDisplay, sound)
                        help_active = False

Rhetorical question; see how the only data source is the event?

yell0wfl4sh commented 6 years ago
yell0wfl4sh commented 6 years ago

@quozl please review!

quozl commented 6 years ago

Thanks. Many magic constants repeat. Should these be in a dictionary or list structure?

yell0wfl4sh commented 6 years ago

what do you mean by magic constants?

quozl commented 6 years ago

The coordinates in x, y for each of the rectangles for the button areas.

yell0wfl4sh commented 6 years ago

we can do it like: cont = (0,0). Or, it can also be a dictionary, whose value is a list, but this would not be very different from the previous method. @quozl your thoughts?

quozl commented 6 years ago

Agreed, too different, let's concentrate on the issue #12 first.

Tested your branch at f63ce74. There's an odd change with respect to master you might have a quick look at; when you start the activity the "New Game" and "Back" buttons are visible above the title.

When I press "Play", they slide down into position, and when I press "Back" they slide off to the left and then are positioned above the title.

yell0wfl4sh commented 6 years ago

Not sure what the error was, but I guess it is fixed now.

quozl commented 6 years ago

Tested b93cce9.

If I keep clicking the descending "Play" and "Back" buttons enough, eventually the program enters a state where the "Play" button crawls down the screen very very slowly, and doesn't stop, disappearing off the bottom edge. However, it is even worse with master branch, the buttons disappear entirely, so not caused by your code alone.

yell0wfl4sh commented 6 years ago

Maybe open a new issue about it :-)

quozl commented 6 years ago

It's not that important to log as an issue, and there are more pressing issues, in particular #6 without which this activity will hardly be used in future, as GTK+ 2 is being removed by Linux distributions. If you want to fix the descending buttons, go right ahead and do so, you don't need an issue logged.