Skip to content
This repository has been archived by the owner on Mar 31, 2020. It is now read-only.

Flamboyant Flamingos #16

Open
wants to merge 95 commits into
base: master
Choose a base branch
from
Open

Conversation

Starwort
Copy link

Did I do this right?

Starwort and others added 7 commits February 22, 2019 16:38
Alt-F4 saving skeleton proven working
- Replaced requirements.txt with Pipfile so that it works with pipenv
- Moved and renamed the main python file to fit the original project layout
@Starwort Starwort force-pushed the master branch 3 times, most recently from 63b21b9 to 78a71a3 Compare February 23, 2019 15:21
slushiegoose and others added 11 commits February 23, 2019 15:45
Plus edited .gitignore to omit VS Code settings
This does almost nothing at the moment, but will aid in organising the rest of the program

Added a key bind
Also implemented the kata as references into the main file
- Added the main base of the program - including the Canvas and the Entry Section.

- Installed Pillow to be used for the Canvas sections since Canvases can't be turned into images

- Added new locale phrases (for Star to translate) to the locale file

- Changed the save function to write to a file instead of just printing out the file path
slushiegoose and others added 25 commits March 2, 2019 14:50
- Alter Individual Pixel
- Alter Block Pixel
- Alter Colour
- Do Nothing
- Pixelate
visualisation window doesn't quite work though
This does actually work - it's just a royal pain in the ass - which is exactly what it's supposed to be :)
- Added more notes to README.md
- Added a JUDGEINFO.md to allow the judges to see what we actually have done that isn't troll-based
- Last minute locale fix
- Idk the Pipfile.lock hash changed so I thought let's add it just in case

WE'VE BEEN FLAMBOYANT FLAMINGOS AND GOODNIGHT
Sorry we noticed spelling mistakes so we couldn't end off like that
Grammar errors.... oops :D
@Starwort
Copy link
Author

Starwort commented Mar 4, 2019

Mistake in JUDGEINFO.md: Top left is (1, 1) not (0, 0)

that's all lol

JUDGEINFO.md Show resolved Hide resolved
README.md Show resolved Hide resolved
try:
int(colour)
except ValueError:
raise TypeError
Copy link
Member

Choose a reason for hiding this comment

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

this is a bit of an antipattern. you should raise this error with a message explaining what's happening, or it becomes very frustrating to debug.

Copy link
Author

Choose a reason for hiding this comment

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

I'll leave this one to @Suhail6inkling , who wrote this code.

project/canvas.py Show resolved Hide resolved
project/locale.py Show resolved Hide resolved
@lemonsaurus
Copy link
Member

This looks like a really fun project. Great documentation, although your use of comments is a bit inconsistent - some places you've got one comment every 2 or 3 lines, while other places have neither docstrings nor block comments, and not much in the way of newlines either. Perhaps this boils down to difference in style between team members. Ideally we would have liked to see a consistent style across the entire project.

Uses some older Python techniques such as map() rather than comprehensions, shadows some build-ins, some exceptions pass silently. Some very creative uses of classes in the katakana transcription file. Overall this isn't the most Pythonic code I've ever seen, but it's mostly of acceptable quality.

Using async is a brave choice, but it's clear that you guys know what you're doing with it. I'm looking forward to testing it, and it will be interesting to see how performant it is.

@Starwort
Copy link
Author

Starwort commented Mar 6, 2019

Thank you for the praise, it was enjoyable to create and I look forward to seeing you find all its, let's call them quirks. The reason maps and loops were used over comprehensions is almost entirely so that the asynchronous framework could work correctly - maps are faster and loops can have async microsleeps so that the framework can work at the same time.

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

Successfully merging this pull request may close these issues.

4 participants