-
Notifications
You must be signed in to change notification settings - Fork 21
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
advent: display leaderboards as a colour image #212
Conversation
uses pillow to render leaderboards into an image with a style very close to the website. inline display in the discord app is a bit smaller than i would've liked but alas. text format is kept alongside the image since it's probably nice to have. maybe this should be behind an argument, but there are already so many. implementation has some weaknesses: - unicode multi-character sequences and emoji aren't rendered - relies very much on a monospace font for alignment let me know if this is something you want.
I hate to well-actually, because this is uber cool, but it would probably be a better UX experience to have the boards be an embed with fancy discord interactions (e.g. buttons to scroll down or scroll up)? I'll let others chime in on what they want AoC to "be" into the future though, because I don't see anyone jumping to write alternatives either - it would be silly to veto a good change just to make way for a "better" version that might not ever get written. |
A Discord embed is probably a good way to modernise it. I don't think full pagination is good (way too many clicks), but maybe something like showing the first 20 rows in an image, then a button to expand to the full image. From testing, this is comfortable to read on Discord and mobile. I do like the images (I just think they're neat) so I'd like to make them work, but also open to feedback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it! It looks and feels a lot better than the old text format, especially on mobile.
My comments here are not blocking; just minor interaction points.
- support show all <-> show less - add comment about font choice other things: - remove all buttons on timeout - send text file as a separate message, keeping image in original reponse - tweak some comments and rename print_leaderboard
changes in 344c0f6. re fonts, i wanted to have the most supported glyphs. i think this is import important than matching the website exactly. some comparisons: noto ibm sourcecodepro. noto mono is the most extensive, but it does break the consistent monospace width sometimes. luckliy for us, this is not a problem since the only non-ascii characters are at the far right of each line (discord display names, example), so everything stays aligned. |
still not ephemeral, because it's probably useful to see if people are having problems with the command.
previously, total star2 time was actually the total time, and total time was double counting.
after 1fd826d:
this can be separated from the bigger pr if you want |
Ah, yeah. I agree, that is more important. Shame pillow doesn't support fallback fonts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The time changes probably should be in a separate PR, but they do work so it's all good. I'll merge sometime tomorrow if nobody has further comments.
pytz stays around because of dependencies. tzdata was also already transitively-required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
uses pillow to render leaderboards into an image with a style very close to the website. inline display in the discord app is a bit smaller than i would've liked but alas. it should be much better on mobile at least.
text format is kept alongside the image since it's probably nice to have. maybe this should be behind an argument, but there are already so many.
also added url to website into the reply message
implementation has some weaknesses:
let me know if this is something you want.
OR, just inline the leaderboard into the message. it's only done as an attachment because slack let you preview text files on desktop and mobile.