Make penalty time a per contest setting#3347
Make penalty time a per contest setting#3347nickygerritsen wants to merge 2 commits intoDOMjudge:mainfrom
Conversation
vmcj
left a comment
vmcj
left a comment
There was a problem hiding this comment.
I get that the spec specifies this as only needed for pass-fail but I think the code is much easier if we don't have those branches if we're not forced to need them.
So instead of not allowing it, setting it to 0 by default for scoring contests and checking in the other sections if it's non 0 and if so to display the extra penalty.
|
|
||
| public function up(Schema $schema): void | ||
| { | ||
| $this->addSql('ALTER TABLE contest ADD penalty_time INT UNSIGNED DEFAULT 20 NOT NULL COMMENT \'Penalty time in minutes per wrong submission (if eventually solved)\' AFTER scoreboard_type'); |
There was a problem hiding this comment.
You pick our default here, but someone might have set this to another value. I wonder if we should keep the global setting and just force it to 0 for Scoring contests?
This way they would need to (re)set this value each time.
There was a problem hiding this comment.
I don't follow what you mean. For migrated contests we pick what they configured. For new contests we use 20 as default but allow to set it
There was a problem hiding this comment.
I don't agree that allowing it makes sense. Then all of a sudden we have to take it into account on the scoring scoreboard. I don't think the branches are complicated but if others think it makes more sense to do it I can do it
There was a problem hiding this comment.
I don't follow what you mean. For migrated contests we pick what they configured. For new contests we use 20 as default but allow to set it
In case someone configured 30min or 1h (or whatever) it would be applied for all contests, both current and future.
Your code fixes the current ones but not the future ones as you set it to 20 instead of the current value. I think in case someone picks another value as the current one they have a reason for that and probably prefer it for newer ones also.
There was a problem hiding this comment.
My suggestion to improve the changelog message then to make it more clear and not carry that legacy over.
There is no time displayed for scoring contests anywhere in the interface |
And the point I'm trying to make that instead of having a scoreboard per contest type we should try to generalize such things and use sane defaults. But if I'm the only one (and it seems to be I am) I'm fine with just duplicating all situations everywhere. |
I am with you on generalizing and using sane defaults where it makes sense. There is just nothing to generalize here since penalty time is not a thing for scoring. |
For score contests we use `0`, since the spec says "Only relevant if scoreboard_type is pass-fail.".
nickygerritsen
force-pushed
the
penalty-time-per-contest
branch
from
47fae19 to
69f3fbb
Compare
Yeah I don't see how we would do this. And then I think it doesn't make sense showing a value that doesn't get used. |
3 participants
For score contests we use
0, since the spec says "Only relevant if scoreboard_type is pass-fail.".