-
Notifications
You must be signed in to change notification settings - Fork 98
feat: rewrite AutomaticPointsTable #6941
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
base: main
Are you sure you want to change the base?
Conversation
faa7c37 to
3fbf2f9
Compare
looks slightly better
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.
skimmed through it on phone
can give a proper review in the next days
fwiw to me it seems it is still incompatible with the stuff craft wikis (plus several other wikis) use
| function AutomaticPointsTable:storeLPDB(opponents) | ||
| local date = DateExt.getContextualDateOrNow() | ||
| Array.forEach(opponents, function(opponent) | ||
| local teamName = Opponent.toName(opponent.opponent) |
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.
should rename, assuming we support this for non team opponents
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.
actually it is limited to team opponents:
Lua-Modules/lua/wikis/commons/AutomaticPointsTable.lua
Lines 160 to 161 in 21e0e5d
| assert(parsedOpponent.opponent.type == Opponent.team) | |
| assert(not Opponent.isTbd(parsedOpponent.opponent)) |
- the old version supports teams only
- I didn't bother expanding to party opponents mainly to keep the old alias handling behavior
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.
ugh, so this module is entirely unusable for quite a few wikis...
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.
well I think that it wouldn't be too hard to expand this to party opponents
we might need to rethink how to handle alias crap from scratch though
hjpalpha
left a comment
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.
seems cleaner, but still useless/unusable for lots of wikis
¯_(ツ)_/¯
Summary
Module:AutomaticPointsTableis an unused "legacy" component that has been left abandoned for years, while the "live" version used on wikis is a worse spaghetti that is not even on this repository. This PR rewrites the module with the current standards.How did you test this change?
https://liquipedia.net/valorant/User:ElectricalBoy/Sandbox3