sugarlabs / physics

a box2d playpen
GNU General Public License v3.0
7 stars 26 forks source link

Port to Python3, update collabwrapper, add imports and replace box2d … etc [GCI] #44

Closed srevinsaju closed 4 years ago

srevinsaju commented 4 years ago

This is a locally tested python3 port

Coverage report

activity.py                                                                                          519    427    18%
collabwrapper.py                                                                                     383    267    30%
helpers.py                                                                                            94     84    11%
myelements/__init__.py                                                                                 2      0   100%
myelements/add_objects.py                                                                            298    269    10%
myelements/callbacks.py                                                                               47     33    30%
myelements/camera.py                                                                                  44     31    30%
myelements/drawing.py                                                                                171    128    25%
myelements/elements.py                                                                               387    333    14%
myelements/locals.py                                                                                   8      0   100%
myelements/tools.py                                                                                   27     23    15%
myelements/tools_poly.py                                                                             172    156     9%
physics.py                                                                                           124    106    15%
sugargame/__init__.py                                                                                  1      0   100%
sugargame/canvas.py                                                                                   47     35    26%
sugargame/event.py                                                                                   157    126    20%
tools.py                                                                                             713    525    26%

Installing pybox2d

The best method to install pybox2d (so far tested by me) is the original github repository. This is because:

To build pybox2d it requires us to install swig On Ubuntu sudo apt install swig On Fedora sudo dnf install swig

quozl commented 4 years ago

Fixes https://github.com/sugarlabs/physics/issues/42.

Thanks for the information about how to resolve dependencies. Could you move it to README.md? Please see https://github.com/sugarlabs/speak and https://github.com/sugarlabs/record-activity for README.md that describe dependencies. Please see https://github.com/sugarlabs/fractionbounce for the best README.md that I've seen recently. You may also add a one line test case python3 -c 'import Box2D'. I used pip3 install . --system because Physics would normally be installed in /usr/share/sugar/activities.

Tested briefly.

A failure to resume from Journal was because exec is yet unchanged. (ImportError, because I don't have a Python 2 build of PyGame on the test system.)

Also this in logs;

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/sugar3/graphics/icon.py", line 574, in do_get_preferred_width
    surface = self._buffer.get_surface()
  File "/usr/local/lib/python3.6/dist-packages/sugar3/graphics/icon.py", line 370, in get_surface
    handle = self._load_svg(icon_info.file_name)
  File "/usr/local/lib/python3.6/dist-packages/sugar3/graphics/icon.py", line 197, in _load_svg
    return self._loader.load(file_name, entities, self.cache)
  File "/usr/local/lib/python3.6/dist-packages/sugar3/graphics/icon.py", line 126, in load
    icon = icon_file.read()
  File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 7132: ordinal not in range(128)
quozl commented 4 years ago

Thanks. https://github.com/pybox2d/pybox2d/blob/master/INSTALL.md is good documentation for installing Box2D. Can we refer to this instead of maintaining our own? :grin:

srevinsaju commented 4 years ago

Yes @quozl , we can, but i felt users might not go into it and then follow those instruction It might be like our wiki.sugarlabs where everything is interlinked, but makes sense.

quozl commented 4 years ago

Understood. Any idea on the UnicodeDecodeError?

srevinsaju commented 4 years ago

@quozl , i am not getting that error after I changed the exec statement https://github.com/sugarlabs/physics/pull/44/commits/4911cf5530031e40c24ae7caf6c3347920932636

srevinsaju commented 4 years ago

32 before and after porting for me

quozl commented 4 years ago

Thanks. Tested collaboration. Worked fine, but got this traceback;

Traceback (most recent call last):
  File "/usr/share/sugar/activities/Physics.activity/collabwrapper.py", line 696, in __notify_state_cb
    input_stream = self._get_input_stream()
  File "/usr/share/sugar/activities/Physics.activity/collabwrapper.py", line 744, in _get_input_stream
    return Gio.MemoryInputStream.new_from_data(self._blob, None)
TypeError: Item 0: Must be number, not str
srevinsaju commented 4 years ago

@quozl will test it tomorrow, seems like a bug in collabwrapper, i'm not sure though, no traces from physics.py

quozl commented 4 years ago

Traceback was only on the sharing instance. I've seen it before; yes, we might have a problem in CollabWrapper yet to be fixed. Does it happen for you?

srevinsaju commented 4 years ago

I can't do physics testing, the he VM crashes on the swig build, I guess the sugargame or collabwrapper is not updated.

srevinsaju commented 4 years ago

@quozl by editing collabwrapper, Line 744, I would be possibly able to fix that error. But editing collabwrapper is not suggested, right?

quozl commented 4 years ago

Where we identify the problem is in CollabWrapper, the fix should be in the collabwrapper.py file here, and also a pull request to the CollabWrapper repository. Especially easy if the problem can be reproduced with the CollabWrapper test activity, but if it can't and the test activity can be improved to catch the problem, that's even better.

srevinsaju commented 4 years ago

@quozl will try editing collabwrapper on the repo, and see if the tests succeed

chimosky commented 4 years ago

@srevinsaju could you apply change in 58660d4 to collabwrapper itself?

srevinsaju commented 4 years ago

@chimosky I could create a PR, but, @quozl suggested that, it should have been an error on the port of Physics activity. @chimosky did you run into more instances of such?. so I would be able to confirm its an error on the collabwrapper part. Thanks

A PR has been created @ https://github.com/sugarlabs/collabwrapper/pull/20 This error was also faced in Story activity when ported by @Kiy4h