ppb / pursuedpybear

A python game engine.
https://ppb.dev/
Artistic License 2.0
258 stars 98 forks source link

Flaky visual test: Loading #578

Open pathunstrom opened 3 years ago

pathunstrom commented 3 years ago

ppb commit: b2eee07a7f0ea OS Windows 10 Python 3.8.6

Output:

==========
loading.py
==========

Tests loading scenes.

Should take some time to progress. This may need to be run several times to suss
out threading bugs.

UserWarning: Using SDL2 binaries from pysdl2-dll 2.0.10
bullet.png 1.527485162932843
player.png 4.096429452946413
target.png 5.921888454316235
AssetLoaded(asset=<Image name='target.png' loaded at 0x149caf40220>, total_loaded=1, total_queued=0)
Traceback (most recent call last):
  File "C:\Users\pathu\PycharmProjects\pursuedpybear\viztests\loading.py", line 60, in <module>
    ppb.run(starting_scene=LoadingScene)
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\__init__.py", line 125, in run
    eng.run()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 311, in run
    self.main_loop()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 340, in main_loop
    self.loop_once()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 356, in loop_once
    self.publish()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 382, in publish
    method(event, self.signal)
  File "C:\Users\pathu\PycharmProjects\pursuedpybear\viztests\loading.py", line 37, in on_update
    assert self.bullet.is_loaded()
AssertionError
Traceback (most recent call last):
  File "C:\Users\pathu\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\pathu\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Users\pathu\PycharmProjects\pursuedpybear\viztests\__main__.py", line 24, in <module>
    subprocess.run([sys.executable, str(script)], check=True)
  File "C:\Users\pathu\AppData\Local\Programs\Python\Python38\lib\subprocess.py", line 512, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['C:\\Users\\pathu\\PycharmProjects\\pursuedpybear\\venv\\Scripts\\python.exe', 'C:\\Users\\pathu\\PycharmProjects\\pursuedpybe
ar\\viztests\\loading.py']' returned non-zero exit status 1.

Some kind of race condition I think? It's the assert failing.

AstraLuma commented 3 years ago

This may need to be run several times to suss out threading bugs.

Thanks past me for noting this and trying to write a test to induce this.

Present us now have to debug multi-threaded code.

mark-boer commented 3 years ago

Interestingly, the problem seems to be that the "target.png" is finished loading, before the three delayed images have started loading. (So surprisingly it seems that the problem does not arise due to the sleep) Therefore the loading screen, thinks it has loaded all 1 of 1 images and thus continues to the quitter screen.

You could fix it by starting the threadpool later, after all AssetLoading jobs have been submitted. Or maybe better not using the total_queued in the event, but instead look at the global ThreadPool._finished from the LoadingScene.on_asset_loaded.

Also the DelayedThreadExecutor._finish is run from the worker threads, which also touch the _finished and _started variables. It's read only, so it shouldn't be too bad, but it could lead to race conditions.

Edit: fix typo's

AstraLuma commented 3 years ago

New assets can be loaded at any time. And I'd really rather that the loading scene not reach into the internals of the asset loading system.

mark-boer commented 3 years ago

You could maybe register assets, that you would want to wait on in the loading screen?

Also calling the DelayedThreadExecutor._finish on the main thread might work. Or checking finished jobs in the main loop?

Or, maybe not use the data in the AssetLoaded event, but instead in on_asset_loaded, check if all the assets in the next scene are loaded.

AstraLuma commented 3 years ago

You could maybe register assets, that you would want to wait on in the loading screen?

Gotta make sure you both reference the asset and then pre-declare it for the loading screen? And then you forget to update one of the two places.

Also calling the DelayedThreadExecutor._finish on the main thread might work. Or checking finished jobs in the main loop?

I mean, it's never actually finished, just idle. Again, new, previously unknown assets can be referenced at any time.

Or, maybe not use the data in the AssetLoaded event, but instead in on_asset_loaded, check if all the assets in the next scene are loaded.

How do you make that list?

mark-boer commented 3 years ago

I'm just spitballing here, I don't actually know what would make most sense in this case.

Gotta make sure you both reference the asset and then pre-declare it for the loading screen?

It would be difficult, because asset classes are not instantiated until the scenes are instantiated.

How do you make that list?

You could use meta classes, similar to how SQLAlchemy ORM is used. But I don't know if that would work with the current syntax.