-
-
Notifications
You must be signed in to change notification settings - Fork 6
Add expectation tier function for non-members #23
Conversation
Before this function implemented, user cannot know where they are and what are the proper problems without tier This function works following steps 1. Parse the solved problem in user page 2. Get Level information from Api server 3. Calculate the total exp point from level information 4. Display expectation tier Signed-off-by: Seokho Song <0xdevssh@gmail.com>
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.
Thank you for your pull request, I've reviewed your code and requested some changes.
Although, this is extremely inefficient for users with like >1000 solved problems; it will wait for the plugin to process >1000 requests. Maybe in future I'll make a separate endpoint that calculates this information.
src/content/main.js
Outdated
| + "</a>" | ||
|
|
||
| if(!userData && isShowUserTempTier) | ||
| { |
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.
Please use K&R style brackets.
| if(!userData && isShowUserTempTier) | ||
| { | ||
| var acceptPanel = document.querySelector(".panel.panel-default") | ||
| var acceptProblems = acceptPanel.querySelectorAll(".problem_number>a") |
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.
Since BOJ and solved.ac calculates accepted problems differently, the actual experience points calculated from this method will differ from official calculation. See https://solved.ac/faq/
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.
Could you explain more details?
I'm not sure what I need to change on this part.
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.
If problem is marked with tag 'Preparing Judge', it should not be calculated as accepted.
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.
Does api server respond 'level 0' if the problem that was tagged 'preparing judge'?
src/content/utils.js
Outdated
| const levelData = [320, 480, 912, 1642, 2791, 4465, 6698, 9913, 14472, 20840, 29593, | ||
| 41431, 57588, 79472, 108877, 148072, 200638, 270862, 364309, 488174, | ||
| 651712, 866777, 1148479, 1515993, 1993531, 2611525, 3408040, 4430452, | ||
| 5737436, 7401292, 9510661] |
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.
Level experience points are subject to change, it should NOT be declared as constants. Please use https://api.solved.ac/exp_table.php
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.
If we had an api document(or just list), it would be great to develop some of functions.
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.
I will document APIs in the future, but it's a shame that I'm currently too busy to make and publish documented APIs.
src/content/utils.js
Outdated
| const expTable = [0, 4800, 15740, 38720, 83400, 163700, 298000, 785400, 1202800, 1795000, 2834700, | ||
| 4276000, 6261000, 8982000, 12704000, 18796000, 26842000, 37941000, 52792000, | ||
| 720000000, 152000000, 213000000, 294000000, 380000000, 639000000, 1000000000, | ||
| 1200000000, 1500000000] |
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.
Tier bounds are also subject to change, hence it should NOT be declared as constants. Currently we don't have API endpoints for this, I'll fix it later.
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.
Yes it does, but its experience points has to be calculated as zero.
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.
Please change this to use https://api.solved.ac/exp_cap.php.
src/options/options_logged_info.html
Outdated
|
|
||
| <div class="option_item" data-key="show_userpage_temp_tier"> | ||
| <span class="option_caption"> | ||
| 유저페이지 임시티어 보기 |
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.
Please change it to "사용자 페이지에서 비공식 티어 보기".
src/content/main.js
Outdated
|
|
||
| var expectLevel = getExpectLevelFromExpPoint(totalExpPoint) | ||
| newRowHeader.innerText = "solved.ac 임시티어" | ||
| newRowDescription.innerHTML = "<a href=\"https://boj.com/" + userId + "\">" |
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.
What's with boj.com? It links to some random premium domain broker site.
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.
It's my mistake, I'll fix to boj.kr or acmicpc.net.
|
Please check my commit that your request has been resolved! |
1. Fix missed coding style code to k&r style 2. Replace option menu 3. Replace constant exp point table to server based table 4. Fix URL on expectation tier 5. Ignore level 0 problem to calculate total exp point Signed-off-by: Seokho Song <0xdevssh@gmail.com>
|
I'm currently too busy preparing problems for Sogang Programming Contest. I'll take a look when I'm not busy. Thanks. |
|
Ah Okay! Take a time! |
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.
Please fix some trivial formatting errors, and I think we're good to go.
src/content/utils.js
Outdated
| const expTable = [0, 4800, 15740, 38720, 83400, 163700, 298000, 785400, 1202800, 1795000, 2834700, | ||
| 4276000, 6261000, 8982000, 12704000, 18796000, 26842000, 37941000, 52792000, | ||
| 720000000, 152000000, 213000000, 294000000, 380000000, 639000000, 1000000000, | ||
| 1200000000, 1500000000] |
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.
Please change this to use https://api.solved.ac/exp_cap.php.
src/content/utils.js
Outdated
| 4276000, 6261000, 8982000, 12704000, 18796000, 26842000, 37941000, 52792000, | ||
| 720000000, 152000000, 213000000, 294000000, 380000000, 639000000, 1000000000, | ||
| 1200000000, 1500000000] | ||
| for(i=0;i<expTable.length-1;i++) { |
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.
Code is not well-formatted; there must be spaces between operands and operators.
Add spaces between operands and opraters Replace from constant exp table to exp table fetched from api Signed-off-by: Seokho Song <0xdevssh@gmail.com>
|
And here we go! |
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.
LGTM
Before this function implemented, user cannot know where they are
and what are the proper problems without tier
This function works following steps
Thanks
Seokho