sugarlabs / physics

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

Physics 35 activity crashes immediately with Pygame 2.0.1 #50

Closed aperezbios closed 1 year ago

aperezbios commented 3 years ago

Upon starting Physics 35, packaged in the Fedora 34 SoaS nightly, which uses Sugar 0.118, the following is logged, and the application exits, returning the user to the Sugar application pane. The bundled version of python3-pybox2d is 2.3.2-25


  File "/usr/lib/python3.9/site-packages/sugar3/graphics/icon.py", line 562, in do_get_preferred_height
    surface = self._buffer.get_surface()
  File "/usr/lib/python3.9/site-packages/sugar3/graphics/icon.py", line 370, in get_surface
    handle = self._load_svg(icon_info.file_name)
  File "/usr/lib/python3.9/site-packages/sugar3/graphics/icon.py", line 197, in _load_svg
    return self._loader.load(file_name, entities, self.cache)
  File "/usr/lib/python3.9/site-packages/sugar3/graphics/icon.py", line 140, in load
    return Rsvg.Handle.new_from_data(icon.encode('utf-8'))
gi.repository.GLib.Error: rsvg-error-quark: XML parse error: error code=201 (3) in (null):22:40: Namespace prefix inkscape for connector-curvature on path is not defined```
aperezbios commented 3 years ago

This appears to be related to https://github.com/sugarlabs/physics/commit/d3a7117d4854733d3dba8b6de80429e964f6b314

Fedora 34 includes librsvg2 version 2.50.3

aperezbios commented 3 years ago

I manually applied the above change/patch, and now Physics crashes with:


  File "/usr/lib64/python3.9/site-packages/gi/_propertyhelper.py", line 401, in obj_set_property
    prop.fset(self, value)
  File "/usr/lib/python3.9/site-packages/sugar3/activity/activity.py", line 612, in set_active
    self.save()
  File "/usr/lib/python3.9/site-packages/sugar3/activity/activity.py", line 978, in save
    self.write_file(file_path)
  File "/usr/share/sugar/activities/Physics.activity/activity.py", line 129, in write_file
    self.game.write_file(file_path)
  File "/usr/share/sugar/activities/Physics.activity/physics.py", line 78, in write_file
    self.world.add.remove_mouseJoint()
AttributeError: 'PhysicsGame' object has no attribute 'world'
Terminated by signal 11, pid 1546 activity_id ce9ff1229df5b45e24e2778973f91817df3a7e23```
quozl commented 3 years ago

Possibly a change to PyGObject or CPython internals. https://github.com/sugarlabs/arithmetic/pull/7#issuecomment-678500103 also found that property setters were being called earlier in the more recent releases of Python. In the traceback above, the Activity active property is being changed causing Journal write before the Physics World is set up by run.

Does this fix it?

diff --git a/physics.py b/physics.py
index 300cf44..30b9f0e 100644
--- a/physics.py
+++ b/physics.py
@@ -75,6 +75,8 @@ class PhysicsGame:

     def write_file(self, path):
         # Saving to journal
+        if not getattr(self, world):
+            return
         self.world.add.remove_mouseJoint()
         additional_data = {
             'trackinfo': self.trackinfo,
aperezbios commented 3 years ago

@quozl, with that patch applied, the activity fails to start, with a Python error NameError: name 'world' is not defined

chimosky commented 3 years ago

Might be worth testing without d3a7117 as librsvg2 2.50.3 might have not have stricter namespaces in the XML parser, the activity shouldn't exit if the above patch is applied, is there a full error output with lines where that happened?

quozl commented 3 years ago

Interesting. I'm using librsvg2 2.50.3 on Debian 11 (Bullseye) with HEAD, and there's no error. If I revert https://github.com/sugarlabs/physics/commit/d3a7117d4854733d3dba8b6de80429e964f6b314 there are three errors in logs, all expected XML parse error, and some buttons are without an icon. I also don't get what @aperezbios was getting ("AttributeError: 'PhysicsGame' object has no attribute 'world'").

But I see my patch above was in error. Quotes are missing.

diff --git a/physics.py b/physics.py
index 300cf44..ed43fab 100644
--- a/physics.py
+++ b/physics.py
@@ -75,6 +75,8 @@ class PhysicsGame:

     def write_file(self, path):
         # Saving to journal
+        if not getattr(self, 'world'):
+            return
         self.world.add.remove_mouseJoint()
         additional_data = {
             'trackinfo': self.trackinfo,
aperezbios commented 3 years ago

@quozl, even with the added quotes, I still get the same AttributeError message as before.

@chimosky, d3a7117 is definitely necessary. I have it applied.

quozl commented 3 years ago

At the same line number?

aperezbios commented 3 years ago

Here's the exact output:


  File "/usr/lib64/python3.9/site-packages/gi/_propertyhelper.py", line 401, in obj_set_property
    prop.fset(self, value)
  File "/usr/lib/python3.9/site-packages/sugar3/activity/activity.py", line 612, in set_active
    self.save()
  File "/usr/lib/python3.9/site-packages/sugar3/activity/activity.py", line 978, in save
    self.write_file(file_path)
  File "/usr/share/sugar/activities/Physics.activity/activity.py", line 129, in write_file
    self.game.write_file(file_path)
  File "/usr/share/sugar/activities/Physics.activity/physics.py", line 78, in write_file
    if not getattr(self, 'world'):
AttributeError: 'PhysicsGame' object has no attribute 'world'
Terminated by signal 11, pid 2356 activity_id d62f9a7023f3b5f07c6250bdda0cda6ae1c5322e```
aperezbios commented 3 years ago

image

quozl commented 3 years ago

Sorry, my mistake. Should be hasattr not getattr. Flying blind.

chimosky commented 3 years ago

Sorry, my mistake. Should be hasattr not getattr. Flying blind.

😆 happens sometimes, I didn't see that either.

aperezbios commented 3 years ago

@quozl, with getattr changed to hasattr, here's the log output:

  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 128, in _keydown_cb
    return self._keyevent(widget, event, pygame.KEYDOWN)
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 169, in _keyevent
    mod = self._keymods()
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 145, in _keymods
    mod |= self.__keystate[key_val] and mod_val
IndexError: list index out of range
Traceback (most recent call last):
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 140, in _keyup_cb
    return self._keyevent(widget, event, pygame.KEYUP)
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 170, in _keyevent
    self.__keystate[keycode] = type == pygame.KEYDOWN
IndexError: list assignment index out of range
Traceback (most recent call last):
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 128, in _keydown_cb
    return self._keyevent(widget, event, pygame.KEYDOWN)
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 169, in _keyevent
    mod = self._keymods()
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 145, in _keymods
    mod |= self.__keystate[key_val] and mod_val
IndexError: list index out of range
Traceback (most recent call last):
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 140, in _keyup_cb
    return self._keyevent(widget, event, pygame.KEYUP)
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 170, in _keyevent
    self.__keystate[keycode] = type == pygame.KEYDOWN
IndexError: list assignment index out of range
Traceback (most recent call last):
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 128, in _keydown_cb
    return self._keyevent(widget, event, pygame.KEYDOWN)
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 169, in _keyevent
    mod = self._keymods()
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 145, in _keymods
    mod |= self.__keystate[key_val] and mod_val
IndexError: list index out of range
Traceback (most recent call last):
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 140, in _keyup_cb
    return self._keyevent(widget, event, pygame.KEYUP)
  File "/usr/share/sugar/activities/Physics.activity/sugargame/event.py", line 170, in _keyevent
    self.__keystate[keycode] = type == pygame.KEYDOWN
IndexError: list assignment index out of range
pygame 2.0.1 (SDL 2.0.14, Python 3.9.1)
Hello from the pygame community. https://www.pygame.org/contribute.html
Normal successful completion, pid 5373 activity_id d62f9a7023f3b5f07c6250bdda0cda6ae1c5322e
quozl commented 3 years ago

Thanks. Does the activity work despite this sugargame keycode related problem?

aperezbios commented 3 years ago

It does not. I get a black window. pygame task seems hung.

quozl commented 3 years ago

Thanks. Can I see the start of the log? It appears to be truncated at the top.

aperezbios commented 3 years ago

image

aperezbios commented 3 years ago

It's not truncated. That's the entire extent of the log.

quozl commented 3 years ago

Interesting, thanks. Traceback (most recent call last): should appear above the first line of your log. If it doesn't, then something is very wrong with Python 3. If it's just a copy and paste error, no matter.

aperezbios commented 3 years ago

Upon closer inspection, it's not actually crashing, but the contents of the pygame window aren't being drawn. If I move the pygame window manually, it re-draws the contents of the window

aperezbios commented 3 years ago

VirtualBox_F34 SoaS LiveCD_17_02_2021_18_29_23

chimosky commented 3 years ago

Upon closer inspection, it's not actually crashing, but the contents of the pygame window aren't being drawn. If I move the pygame window manually, it re-draws the contents of the window

You'll not need to move the pygame window manually as it's been set as the activity canvas, can't think of the reasons why at the moment and the log doesn't seem to tell much.

quozl commented 3 years ago

My guess is Sugargame doesn't work properly with latest Pygame or GTK, in respect of linking the Pygame window into the GTK canvas for the activity. It would be helpful to reproduce with the Sugargame test activity.

chimosky commented 3 years ago

My guess is Sugargame doesn't work properly with latest Pygame or GTK, in respect of linking the Pygame window into the GTK canvas for the activity. It would be helpful to reproduce with the Sugargame test activity.

Agreed.

quozl commented 3 years ago

Reproduced. Investigating.

quozl commented 3 years ago

I ran out of time. Cause of the IndexError in _keymods is a change to Pygame constants; _pygame.KLALT was 308 (0x134), now it is 1073742050 (0x400000e2). Keys are no longer contiguous. So the method of maintaining an indexed array of key states by key number is broken and will need reworking.

I also verified physics.py and activity.py against the Sugargame test activity to make sure everything was being done right; there were a few things done wrong, but fixing them did not improve the situation; the Pygame window remains separate rather than embedded. Something not working with Gtk.Socket or SDL_WINDOWID, but don't know what.

quozl commented 3 years ago

Regarding the Pygame window being separate.

The SDL_WINDOWID feature of SDL allows the window to be reparented. The feature bypasses Pygame, using an environment variable. We use it in Sugar activities.

A change in SDL 2 removed the SDL_WINDOWID feature and replaced it with an API implementation. The different way the feature is implemented requires a solution to be made inside Pygame. It is being tracked here. We might also see progress in the SDL upstream. See here for one such issue of relevance.

This breaks all Sugargame activities, and any activities that embed Pygame inside GTK. A brief list of affected activities;

bundle_id repository
org.sugarlabs.2cars https://github.com/sugarlabs/2-cars-activity
mulawa.AcrossDown https://github.com/sugarlabs/across-and-down-activity
org.somosazucar.Adivinanzas https://github.com/sugarlabs/adivinanzas-activity
mulawa.AppelHaken https://github.com/sugarlabs/appel-haken-activity
org.sugarlabs.Ayni https://github.com/sugarlabs/ayni-activity
org.sugarlabs.BallAndBrick https://github.com/sugarlabs/ball-and-brick-activity
org.laptop.Bichos https://github.com/sugarlabs/Bichos
org.laptop.bridge https://github.com/sugarlabs/Bridge
edu.centenary.CellManagement https://github.com/sugarlabs/cellgame
mulawa.Countries https://github.com/sugarlabs/countries-activity
org.laptop.community.CowBulls https://github.com/sugarlabs/CowBulls-activity
org.laptop.Develop https://www.github.com/sugarlabs/develop-activity
org.ceibaljam.dotsandboxeS https://github.com/sugarlabs/dotsAndBoxes
org.laptop.community.falabracman https://github.com/sugarlabs/falabracman-activity
org.laptop.games.FiftyTwo https://github.com/sugarlabs/fifty-two-activity
org.ceibaljam.flappy https://github.com/sugarlabs/flappy
org.sugarlabs.flappy https://github.com/sugarlabs/flappy-birds-activity
org.laptop.community.FollowMe https://github.com/sugarlabs/followme
org.sugarlabs.GeoTonky https://github.com/sugarlabs/geotonky-activity
org.ceibaljam.conozcoamerica https://github.com/sugarlabs/iknowAmerica
org.ceibaljam.iknoweditor https://github.com/sugarlabs/iknowEditor
org.ceibaljam.conozcoindia https://github.com/sugarlabs/iknowIndia
org.ceibaljam.conozcomadagascar https://github.com/sugarlabs/iknowMadagascar
org.ceibaljam.conozcoperu https://github.com/sugarlabs/iknowPeru
org.ceibaljam.conozcoruanda https://github.com/sugarlabs/iknowRwanda
org.ceibaljam.conozcosrilanka https://github.com/sugarlabs/iknowSriLanka
mulawa.IQ https://github.com/sugarlabs/iq-activity
org.ceibaljam.JAMath https://github.com/sugarlabs/jamath-activity
mulawa.Jumble https://github.com/sugarlabs/jumble-activity
net.coderanger.olpc.JumpActivity https://github.com/sugarlabs/jump
mulawa.Letters https://github.com/sugarlabs/letters
org.sugarlabs.makethemfall https://github.com/sugarlabs/make-them-fall-activity
mulawa.Mancala https://github.com/sugarlabs/mancala-activity
org.sugarlabs.NumRush https://github.com/Gr33nMayhem/NumberRush
org.laptop.community.Numbers https://github.com/sugarlabs/numbers
org.laptop.PanoramaActivity https://github.com/sugarlabs/panorama
org.laptop.physics https://github.com/sugarlabs/physics
org.sugarlabs.Planets https://github.com/pedro-sales/planets-activity
org.laptop.community.Pointillism https://github.com/sugarlabs/pointillism
edu.rit.PyCut https://github.com/sugarlabs/PyCut
org.sugarlabs.stickhero https://github.com/sugarlabs/stick-hero-activity
mulawa.Triples https://github.com/sugarlabs/triples

This also breaks many other SDL based applications on Linux, so there's a good chance there may be a fix forthcoming. To find other bugs, look for SDL_WINDOWID in bug trackers.

For the moment, Physics 35 requires SDL 1.

We could have noticed it in December if we had tested with the latest SDL rather than waiting for Fedora to package it.

sparshg commented 1 year ago

The issue of pygame window not embedding correctly has been fixed in the upstream, (pygame 2.3.0 release). I have tested the activities and they are working as expected on pygame 2.4.0, python 3.11, Fedora 38 KDE.

quozl commented 1 year ago

Thanks, that's great.