-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement scoring for SR2025 #1
Conversation
This adds the basic data conversion logic for the form, though doesn't add a UI template yet, as well as a first pass at the scoring logic itself, though without any validation.
Completely untested.
This relies on v1.6 of the scorer.
This makes them more uniform and (combined with v1.7 of the scorer) helps ensure the input placeholders are visible.
Looks good. Zones need to be rotated around by one anticlockwise so that green zone 0 is bottom left to match the physical scoresheet |
Good spot! a9ff512 |
This makes it easier to understand the intent of these code blocks as well as enabling non-Python-fluent readers to consider which checks have been implemented.
Pallets in districts which don't have high-rises must be on the floor if there is only one pallet in the district. This is an imperect check since it cannot validate districts which do have high-rises. It's probably still useful though.
I'm happy with the checks we have now. Just needs a code review (from not me) to ensure that the checks are doing what we wish them to |
Following user testing at yesterday's Tech Day this turns out to be the preferred input format for the paper forms. This commit fully moves over to that style -- UI, converter, storage and scoring logic.
scoring/update.html
Outdated
</foreignObject> | ||
{% endmacro %} | ||
|
||
{% macro input_pallets(x, y, name, colour_symbol) %} |
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.
Can we maybe colour the letters, too? My mental process for "P" -> Pink, "O" -> Orange, etc.. is a bit slower than just colour matching.
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.
Do you mean in the input
fields in the UI? So having the letters the user is typing being in the relevant colours?
If so, I'm not sure that that would actually help -- the score-sheets which are the reference input will be in either black or blue pen and the user does not need to know what the letters stand for.
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.
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.
Oh, I see. (I had also forgotten that we'd made that layout change 🤦). Yeah, I can definitely see this being useful in the virtual competition where we're going straight in rather than copying from the paper sheets (which as mentioned are greyscale).
I'll have a look at how easy this is to do.
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.
Done in f473854
form.get(f'left_starting_zone_{zone_id}', None) is not None, | ||
} | ||
|
||
def form_district_to_score(self, form: InputForm, name: str) -> RawDistrict: |
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.
[nit] It doesn't look like this returns a score, it returns the raw district data, maybe we should rename the function for clarity?
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.
Hrm, the returned object is not itself a whole score, however it is in the score-flavoured structure (rather than the form-flavoured structure passed in). Open to better ideas if we have any, though worth bearing in mind that this is following existing patterns for naming of these functions.
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.
Feel free to submit as-is,
maybe in the future we could rename "score" to "data" but I don't think it matters
Looks good overall, I checked the scoring logic against the rules pretty thoroughly so I'm happy with this. |
Looks like the checks are:
|
This aims to help use of the scorer during the virtual competition, where we don't have the instructions on the physical sheet guiding users on which colours are what.
This introduces the scoring & conversion logic and the input UI.
CI failure is unrelated -- caused by lack of league matches in the schedule.
Compare to srobo/arena#6
Contributes to srobo/tasks#1402
Relates to srobo/tasks#1439
Fixes srobo/tasks#1401