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 DirectEntry._autoCapitalize() on Python 3.x #628

Closed
wants to merge 8 commits into from
Closed

Fix DirectEntry._autoCapitalize() on Python 3.x #628

wants to merge 8 commits into from

Conversation

jwcotejr
Copy link
Contributor

@jwcotejr jwcotejr commented Apr 18, 2019

This pull request aims to resolve the following crash with DirectEntry._autoCapitalize() under Python 3.x (Specifically Python 3.7.2 included with the current release of Panda3D 1.10.2, 64-bit, under Windows 10):

Traceback (most recent call last):
  File "_autoCapitalizeTest.py", line 8, in <module>
    base.run()
  File "C:\Panda3D-1.10.2-x64\direct\showbase\ShowBase.py", line 3109, in run
    self.taskMgr.run()
  File "C:\Panda3D-1.10.2-x64\direct\task\Task.py", line 531, in run
    self.step()
  File "C:\Panda3D-1.10.2-x64\direct\task\Task.py", line 485, in step
    self.mgr.poll()
  File "C:\Panda3D-1.10.2-x64\direct\showbase\EventManager.py", line 49, in eventLoopTask
    self.doEvents()
  File "C:\Panda3D-1.10.2-x64\direct\showbase\EventManager.py", line 43, in doEvents
    processFunc(dequeueFunc())
  File "C:\Panda3D-1.10.2-x64\direct\showbase\EventManager.py", line 99, in processEvent
    messenger.send(eventName, paramList)
  File "C:\Panda3D-1.10.2-x64\direct\showbase\Messenger.py", line 333, in send
    self.__dispatch(acceptorDict, event, sentArgs, foundWatch)
  File "C:\Panda3D-1.10.2-x64\direct\showbase\Messenger.py", line 418, in __dispatch
    result = method (*(extraArgs + sentArgs))
  File "C:\Panda3D-1.10.2-x64\direct\gui\DirectEntry.py", line 213, in _handleTyping
    self._autoCapitalize()
  File "C:\Panda3D-1.10.2-x64\direct\gui\DirectEntry.py", line 218, in _autoCapitalize
    name = self.get().decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

Two simple changes in the _autoCapitalize function were all that was needed to resolve this crash under Python 3.x, while also preserving compatibility with Python 2.7.

A simple unit test was also included to test this, and a simple test program can be found here: https://pastebin.com/P5sjhYQt

@jwcotejr jwcotejr marked this pull request as ready for review April 18, 2019 16:47
@rdb
Copy link
Member

rdb commented Apr 18, 2019

I presume the original call was there for a reason; I guess if the case that there was a str object in Python 2 that had multi-byte UTF-8 sequences in it. Maybe you should include a UTF-8 string (without u prefix) in your test.

@jwcotejr
Copy link
Contributor Author

As in, use UTF-8 characters in a string, or just use any string that just so happens to be UTF-8 encoded?

@rdb
Copy link
Member

rdb commented Apr 18, 2019

As in, use a string that has non-ASCII characters, in UTF-8 encoding. (If a string doesn't have non-ASCII characters, its UTF-8 encoded form is identical to the ASCII form, because of how UTF-8 is designed.)

@jwcotejr
Copy link
Contributor Author

@rdb Hi there, I have made the necessary changes you have requested. Sorry about the extended delay.

I am not entirely sure if this is 100% ideal, as I am setting the UTF-8 encoding within the unit test itself, nevertheless it does function as intended on both Python 2.x & Python 3.x.

Take care!

@ghost
Copy link

ghost commented Aug 6, 2019

Here I go again! Hopefully reviving @rdb on this one. A definite good push towards full Python 3 support. 👍

@rdb rdb added this to the 1.10.4 milestone Aug 7, 2019
@rdb
Copy link
Member

rdb commented Aug 7, 2019

Yeah, let's get this into the next release. I'd only additionally like to see tests with Unicode objects in Python 2, but I can make that change myself if needed.

@rdb rdb closed this in ff188c1 Aug 18, 2019
@jwcotejr jwcotejr deleted the jwcotejr-patch-1 branch December 31, 2019 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants