DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
739 stars 259 forks source link

Balloon list: only add 'first in contest' if there is no earlier unju… #2812

Open meisterT opened 1 week ago

meisterT commented 1 week ago

…dged submission.

The 'first for problem' is already working correctly as it re-uses the corresponding scoreboard data.

Fixes #2810.

Example submission list: image

Corresponding balloon list: image

Previously we would have awarded the first balloon a "first in contest".

eldering commented 1 week ago

How will this deal with the scenario where team A submits, then team B submits, B gets judged as correct, and then A gets judged as wrong answer?

  1. Will the balloon already be marked for team B even when A's submission is still pending?
  2. I so, will B's balloon retro-actively be updated to be marked as first in contest?
eldering commented 1 week ago

How will this deal with the scenario where team A submits, then team B submits, B gets judged as correct, and then A gets judged as wrong answer?

1. Will the balloon already be marked for team B even when A's submission is still pending?

2. I so, will B's balloon retro-actively be updated to be marked as `first in contest`?

I think the ideal scenario is that a balloon will only be added once, never updated, and so a balloon should only be added after it is known that it's contents (including first in contest/for problem) won't change anymore. Otherwise there's a high risk that users of the balloon interface hand out the wrong balloons.

meisterT commented 1 week ago

In the same way as on the scoreboard as of today. We mark it as solved, but not first to solve while we have incomplete information. Then we have more information and you revisit that page later it gets marked as first to solve.

We could decide to hold the balloon back or mark it as potentially first to solve. Both things are more complex than this but of course can be done. Since this is fixing a clear bug and not making anything worse, I think we should address that separately.

meisterT commented 1 week ago

@nickygerritsen how is this btw handled in ICPC tools?

nickygerritsen commented 1 week ago

@nickygerritsen how is this btw handled in ICPC tools?

🤷 I can check the code to see if I can determine this, but I think it's easier there since it is a long running process that has state.

eldering commented 1 week ago

In the same way as on the scoreboard as of today. We mark it as solved, but not first to solve while we have incomplete information. Then we have more information and you revisit that page later it gets marked as first to solve.

We could decide to hold the balloon back or mark it as potentially first to solve. Both things are more complex than this but of course can be done. Since this is fixing a clear bug and not making anything worse, I think we should address that separately.

I'm a bit wary of various such fixes we've been making over time that all patch a bug by doing their own DB querying, leading to inconsistent logic and making it harder to eventually implement other scoring strategies as we'd have to refactor each of these.

But ok, merge it. Also tagging #2525 for visibility.

meisterT commented 1 week ago

That's a fair point with the DB query. We currently don't have first in contest in the award service, otherwise I would have checked whether we can reuse that. But this doesn't address the other point that you are making about immutability. Let's discuss in person before merging anything.