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

Proof of concept for a minimal PyQt5 UI #460

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Debilski
Copy link
Member

@Debilski Debilski commented Mar 6, 2018

Features:

  • zmq polling in a separate thread (in the single-threaded Tk we have to query every few milliseconds whether new data has arrived)
  • proper anti-aliasing on Linux and macOS (not tested on Windows yet)
  • the possibility for a useful png export (currently behind the --export cmd arg)
  • cubic bezier curves for the ghosts
  • pink food
  • longer startup times
  • no buttons (I did not bother to create them until now and also Qt buttons are slightly in the uncanny valley on macOS)

It’s just a proof of concept and I don’t expect this to be merged or be used as the standard client. Tk is more on the batteries-included side of things and probably more tested. Also, the license of PyQt5 is GPL so we might want to keep it separate. However, we might use it as a base for exporting pngs in a proper and convenient way.

image

@codecov-io
Copy link

codecov-io commented Mar 6, 2018

Codecov Report

Merging #460 into master will increase coverage by 1.57%.
The diff coverage is 7.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #460      +/-   ##
==========================================
+ Coverage   63.11%   64.69%   +1.57%     
==========================================
  Files          33       31       -2     
  Lines        4574     4002     -572     
==========================================
- Hits         2887     2589     -298     
+ Misses       1687     1413     -274
Impacted Files Coverage Δ
pelita/scripts/pelita_qtviewer.py 0% <0%> (ø)
pelita/scripts/pelita_tkviewer.py 0% <0%> (ø) ⬆️
pelita/libpelita.py 57.67% <20%> (+4.41%) ⬆️
pelita/scripts/pelita_main.py 33.82% <33.33%> (+33.82%) ⬆️
pelita/utils/__init__.py 50% <0%> (-38.89%) ⬇️
pelita/scripts/pelita_player.py 82.45% <0%> (-5.57%) ⬇️
pelita/player/FoodEatingPlayer.py 81.81% <0%> (-4.55%) ⬇️
pelita/player/SmartEatingPlayer.py 84.61% <0%> (-3.85%) ⬇️
pelita/player/__init__.py 100% <0%> (ø) ⬆️
pelita/ui/tk_canvas.py 0% <0%> (ø) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2665f13...7baa6b0. Read the comment docs.

@@ -399,7 +399,9 @@ def channel_setup(publish_to=None, reply_to=None):
yield { "publisher": publisher, "controller": controller }


def run_external_viewer(subscribe_sock, controller, geometry, delay):
def run_external_viewer(subscribe_sock, controller, geometry, delay, viewer=None, export=None):
if viewer == None:
Copy link

Choose a reason for hiding this comment

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

if viewer is None is the Pythonic way. ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I usually write is None but patching this method was such a bad hack already that I mentally blacked out that thing I guess. :)

@jni
Copy link

jni commented Mar 7, 2018

So cool! The pink food is a critical feature. =D

I'm impressed with the brevity of the code. I think if you can paint a proper maze (this could be done by painting a brick pattern, or just making black squares of enough size), it's worth merging.

@Debilski
Copy link
Member Author

Debilski commented Mar 7, 2018

Yeah, painting the maze properly is not much of a issue but I will have to look into implementing some caching for that. Currently it does a live redraw which will probably be slower if we have some connected walls (simple bricks don’t look good – there must be some common outline).

Qt comes with built-in scaling and transformations so we don’t have to write it. Apart from that it is not too different from the Tk version actually. Just more minimal. :)

@Debilski
Copy link
Member Author

Hmm, I think we should have some shading for the wall background (or more fluid lines). Otherwise it just looks too similar to the Berkeley one.
image

image
(http://ai.berkeley.edu/contest.html)

if export:
png_export_path = Path(export)
if not png_export_path.is_dir():
raise RuntimeError("Not a directory: {png_export_path}")
Copy link

Choose a reason for hiding this comment

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

shouldn't this get formatted?

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s a commit named ‘WIP’. This will be rebased eventually. Sorry for the noise. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants