poelstra
commented
7 years ago Had a (very) lengthy discussion with @kmeesters about how to best do this.
Initially, we went in the direction of using CRUD actions for updating the scores, which would enable multiple people (Scrutineers) to work together from different machines, on the same scores.json.
We created some pseudo-code to see how that would actually work. Similar to how Idan's ng-Indepedence works, probably. However, we finally concluded that we cannot reliably make this work on a relatively short time-scale, if we want to:
- smoothly support both online and offline modes (i.e. in case of network failure)
- keep it simple to understand for users
- not cause all kinds of very tricky cornercase issues
I won't go into the details here right now (Kenny may give it a go later).
We concluded the following:
- The existing software basically supports a single Scrutineer
- Although we would like to improve this in the future, we think it is way too complicated to get right to attempt that right now
- Submitting a scoresheet today doesn't cause it to be added to scores.json (not on a Referee tablet, neither on the Scrutineer laptop). We would like to improve this in the future, too, to better support a 'full offline' mode, but again this is not in scope right now.
- The use-case of Alan is about having no Scrutineer at all (i.e. not e.g. multiple Scrutineers)
- Thus, Alan's use-case does require moving scoresheets insertion to the server, so we then can have two 'owners' of the data: server for adding sheets, a single client (scrutineer) for editing scores (eliminates the "Check for new scores" button)
- The way polling for new scoresheets works today basically makes it an idempotent action (because we remember which sheets we've processed before). This means that even if a score edit would overwrite the addition of a scoresheet, then another pollSheet() call will simply re-add the sheet again
- Because only published scores will end up in rankings, a 'glitch' like this will not be noticed by end-users, unless AutoPublish is enabled, but then there won't be such glitches because there will not be a scrutineer that could cause a 'merge conflict'
- We'll let the server emit an event whenever it detects a change to scores.json
- We'll move ranking computation to a common file (used by server and client). On client it is used for updating the Ranking view whenever its scores.json changes. On server it is used to broadcast rankings also whenever it detects a change to scores.json (or teams.json or stages.json or settings.json).
- In practice, the chances of having two conflicting edits is made negligibly small by the MHub events for updated scores.
- These events and a periodic refresh (PR #281) eliminate the need for the "Refresh Scores" button.
So, remaining steps:
- [x] Seamless scores refresh (#278)
- [x] Enable automatic refreshing (every 30s for now) of scores.json (#281)
- [x] Refactor scores to prepare for moving ranking computation to common code (#283)
- [x] Remove special case for 'first duplicate score' condition in ranking computation (#286)
- [x] Use Published scores in ranking computation (instead of scores that have no errors) (#288)
- [x] Move Ranking computation to common code (used by ng-scores or a new ng-ranking service) (#295)
- [ ] Move function for adding a scoresheet file to scores.json to the server
- [ ] Add a setting that this "add scoresheet" function can set published to true by default
- [x] Add 'fs hook' mechanism to server's fs module to allow registering for updates to files/directories. E.g.
POST /fs/scores.jsonwill first call each hook with the POSTed data, and the filename. Each hook will return the new contents (if it modified it). Then, these contents will be saved to disk, and then POST will return "OK" to the client. (#289) - [ ] Add fs hook for updates to scoresheets dir -> add the scoresheet to scores.json, return unmodified scoresheet
- [ ] Add fs hook for updates to scores.json -> call pollSheets(), return the POSTed data including any newly added sheets (note: don't call pollSheets if the update was caused by scoresheet hook, above)
- [ ] Periodically (e.g. 30s) and at startup: call pollSheets() (in case of manual file edits etc.)
- [x] Add MHub client to localserver (#285, #296)
- [ ] Add fs hook for updates to scores.json, teams.json, stages.json and settings.json (the latter for which stage(s)/rounds to broadcast), and when it triggers: recompute rankings and broadcast on MHub (same type of message as currently broadcasted, but this can be discussed further)
- [ ] Add fs hook for updates to scores.json to trigger broadcasting "scores changed" message on MHub (to be discussed whether that will broadcast the scores, or just a notification, or a hash or sequence number)
- [ ] Listen for "scores changed" message in client to reload scores.json (in addition to the periodic poll of #281, in case MHub is not present or network glitches)
Notable things we won't change / are out of scope:
- We will not change scores to CRUD operations (yet), i.e. we'll keep posting full scores.json from the client
- We will not change anything to the publishing of scoresheets, it will still just post a file (we may add some kind of queue of unsubmitted scoresheets later to deal with network errors, but this will be an unrelated change)
- I don't think we need any locking: we only atomically update the full scores.json file in one call anyway. If locking is deemed necessary, we can discuss this after the rest of the code is done (should be easy to add). Note that the locking can easily be done using just in-memory state and e.g. some promises and/or a throttle functions. I already have an implementation for that, so ask first.
- We won't need unique id's in scores for now (although it is a good idea, and we may decide to add it in a separate PR later)
We may need to create individual GitHub issues to discuss the details for some of these.
Contribution guidelines: I think all or most of these points can and should be implemented as separate PRs. It may be that it makes no sense to start some PRs before another one is implemented (e.g. using the hooks requires the mechanism to be present). But in that case, e.g. adding the hooks-mechanism should be a separate PR. All PRs should be branched from master. Any merge conflicts (which should be small) should be addressed as late as possible (i.e. don't do tons of 'merge master into bugfix' commits).
I may have forgotten a few things, I'll update this post as necessary to keep an overview.
cc @idanstark42 @alanggreen @kmeesters @rikkertkoppes @Jheronymus
kmeesters
Jheronymus
In smaller competitions where there might not be a scorekeeper and where the ref enters the score directly into a tablet, there needs to be an option where pressing Submit on the scoresheet is all that is required for the score to appear on the display. Bypass Check for Scores, Refresh Scores, Publish and Broadcast.