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

Fix broken unicode len checking in examples/textinput.py #2567

Merged
merged 2 commits into from Oct 9, 2021

Conversation

zaphodikus
Copy link
Contributor

On Win32, the ENTER key handling completely was defeated for some reason, so chatbox entering of text does not do anything since the unicode for ENTER is '\r' and is length 1, not zero.

@Starbuck5
Copy link
Contributor

Starbuck5 commented Apr 18, 2021

The example works on pygame 2.0.0, but not on 2.0.1.

With this example script, you can see that KEYDOWN events like enter or ctrl-c have filled out unicode attributes in 1.9.6, and in 2.0.1, but not in 2.0.0.

import pygame
pygame.init()

screen = pygame.display.set_mode((400,400))

clock = pygame.time.Clock()
while True:
    clock.tick(100)

    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            pygame.quit()
            raise SystemExit

        if event.type == pygame.KEYDOWN:
            print(event)

So that's the deal with why this is broken now.

@zaphodikus Should this test for unicode field length at all?

Requesting a review from ankith26, because I think their event refactor PRs for 2.0.1 are related to this.

@Starbuck5 Starbuck5 requested a review from ankith26 April 18, 2021 21:22
@ankith26
Copy link
Contributor

I consider this change as a "fix" from my event refactor PRs, because the output is same as that in 1.9.6

As to this PR, it is looking good, but I feel that completely removing the event.unicode check might also be a good option

@robertpfeiffer robertpfeiffer self-assigned this May 20, 2021
@robertpfeiffer
Copy link
Contributor

I have made a new textinput example to supplant this one after my IME fix and IME docs are merged. I'll take a look at this soon.

@ankith26 ankith26 added this to the 2.0.2 milestone Aug 14, 2021
Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

👍 thanks.

Since it's 1.9.6 compatible, I'm ok with this.
(Probably better to remove the len(event.unicode) bit all together.)

Copy link
Contributor

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

Yeah this PR seems good to go, to me as well!
Since the original author of this PR seems inactive, should I remove the and len(event.unicode) == 1 check altogether, and then merge this?

@illume
Copy link
Member

illume commented Oct 9, 2021

Since the original author of this PR seems inactive, should I remove the and len(event.unicode) == 1 check altogether, and then merge this?

I'm ok with that (and also just merging it... up to you?)

@ankith26 ankith26 changed the title The enter key code is len = 1 Fix broken unicode len checking in examples/textinput.py Oct 9, 2021
@illume illume merged commit 553a63d into pygame:main Oct 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants