p2r3 / epochtal

Portal 2 tournament framework
https://epochtal.p2r3.com/
GNU General Public License v3.0
8 stars 4 forks source link

Consider removed runs when assigning points #108

Closed p2r3 closed 1 day ago

p2r3 commented 1 day ago

Uses the weeklog to restore user-deleted runs when calculating elo deltas, and implements a way to distinguish user and admin run deletions. Tested on data copied from the live version at epochtal.p2r3.com. After accounting for edge cases, only 3 runs were considered "missing", each of which I checked individually to ensure that they were indeed removed by the player. Placement on the global leaderboard is hardly different, which I see as a good indication that everything is working correctly.

Week 146 top 5 standings post-recalculation:

image

Edge cases specific to the epochtal.p2r3.com deployment are caused by faulty historical data. Patches for those will be applied to the deployment itself and are not part of this pull request. On a less related note, week logs would really benefit from having an extra byte for flags to mark things like proof type, but that would require lots of awkward leaderboard scraping to grandfather that data into the existing logs. I simply do not have the time for this now. As such, the current implementation is forced to assume all deleted LP runs are segmented.

Closes #24

p2r3 commented 1 day ago

Completely agreed, I also dislike how hacky the logs have become. Definitely an excellent application for a more coherent database, though I'm not currently in the position to implement that.

soni801 commented 1 day ago

I'd really like to work on db integration, but my understanding was that you didn't really want to move over to that just yet? I can absolutely write the implementation myself (so no time spent for you), but I'm waiting on your seal of approval to go ahead with the dev effort, aint gonna do it if its just gonna get rejected anyway lmao

PancakeTAS commented 1 day ago

As far as I know p2r3 is studying databases right these few days, so perhaps you should wait up until that class is done?

p2r3 commented 1 day ago

I'd really like to work on db integration, but my understanding was that you didn't really want to move over to that just yet?

I don't think a full db integration system-wide would be a good idea right now, both because I wouldn't be competent to review that, and because many still growing parts of the system would be impacted. Many utils still confidently assume the context.[data|file].[week|leaderboard|log|...] structure, and it's best to not touch that yet.

However, I see no downside to rewriting compact week logs to use a relational database. The scope of that wouldn't be extensive, there's no util that directly tries to parse the binary format without going through the weeklog util first, so it's already abstracted, and I think I also have enough experience now to review a relatively small-scale change like this. Baby steps.