-
-
Notifications
You must be signed in to change notification settings - Fork 134
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
SkyBlock Achievements #1292
base: development
Are you sure you want to change the base?
SkyBlock Achievements #1292
Conversation
Wouldn't it be better to cache Achievements endpoint instead of hardcoding all the achievements? This way we don't have to manually update them when they change, or is there another reason you hardcoded them? |
Thx. I was looking for it and just didn't find - but was certain it exists. |
Implemented. The only 'problem' with this is, that secret achievements now stay secret - well then that's how it is. I'm not doing a hybrid... |
src/lib.js
Outdated
@@ -2396,6 +2397,54 @@ export const getStats = async ( | |||
misc.claimed_items = hypixelProfile.claimed_items; | |||
} | |||
|
|||
const response = await axios("https://api.hypixel.net/resources/achievements"); |
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.
Don't really like this, it gets called every time a profile loads... we should fetch it and cache it in db maybe like... every hour or so
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.
Agreed. But where should I cache it? New table, one value and put the whole list in there? How does that work? I've never used db.collcetion so far...
I guess the code should look something like the following, but neither do I know how to do this, nor do I know how and where you want it cached.
let list = await db.collection("achievements")... // get list from db
if (!list || +new Date() - list.last_updated > 3600 * 1000) {
const response = await axios("https://api.hypixel.net/resources/achievements");
list = response;
await db.collection("achievements")... // put updated list into db
}
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.
Me neither honestly, I would look at how we store bazaar data (that gets refreshed every 30min or so I think) and do it in the same way
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've tried but the query stays empty. There has to be some place where these are properly initialized - but the lines I set in init-collections.js seem not to work properly either:
db.collection("achievements_tiered").createIndex({ key: "text" }, { unique: true }),
db.collection("achievements_one_time").createIndex({ key: "text" }, { unique: true }),
let tieredAchievements = await db.collection("achievements_tiered").find().toArray();
let oneTimeAchievements = await db.collection("achievements_one_time").find().toArray();
console.log(tieredAchievements);
console.log(oneTimeAchievements);
if (!tieredAchievements || tieredAchievements == [] || !oneTimeAchievements || oneTimeAchievements == []) {
//||
//+new Date() - tieredAchievements?.last_updated > 3600 * 1000 ||
//+new Date() - oneTimeAchievements?.last_updated > 3600 * 1000
const response = await axios("https://api.hypixel.net/resources/achievements");
let skyblock_achievements = response.data.achievements.skyblock;
let tieredList = skyblock_achievements.tiered;
let oneTimeList = skyblock_achievements.one_time;
for (const key in tieredList) {
const tmp = tieredList[key];
await db
.collection("achievements_tiered")
.updateOne(
{ key },
{ $set: { name: tmp.name, description: tmp.description, tiers: tmp.tiers } },
{ upsert: true }
);
}
for (const key in oneTimeList) {
const tmp = oneTimeList[key];
await db
.collection("achievements_one_time")
.updateOne(
{ key },
{ $set: { name: tmp.name, description: tmp.description, points: tmp.points, amount: tmp.amount } },
{ upsert: true }
);
}
tieredAchievements = await db.collection("achievements_tiered").find().toArray();
oneTimeAchievements = await db.collection("achievements_one_time").find().toArray();
console.log(tieredAchievements);
console.log(oneTimeAchievements);
}
I think I leave that caching part to someone else... (reset my working copy to the last working version)
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.
Sorry for directly commiting it to your repo, but I hope this is what you wanted.
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.
Sorry for directly commiting it to your repo, but I hope this is what you wanted.
On the contrary - one of the reasons I allow "edits by mantainers". It was a little bit out of my reach considdering my current knowledge about this project, javascript and mongodb. Thank you for your commit to my repo.
As for the icon... book a book n' quill? or just a book? |
The db thing looks good, there's a couple minor things left then this is good to go... I'll try to make a PR/review later |
Made a PR to fix a couple things doej1367#92 For the |
Status 2022-04-28: Waiting for a more reusable implementation of the "Show All" / "Show Less" feature for long lists like the kills-deaths-container has one
This pull request adds all skyblock achievements to the misc section.
I don't know if the added one-time-achivements cause caching problems as that is a lot more data in the cache then before. The second concern is all the data in the misc constants. Both did not cause any issues in my local tests, but maybe do a load test before you lgtm-merge ;)
After this has been deployed it will also take about an hour for the changes to take affect, because of the following line in helper.js. Especially the one time achievements don't show until the page of a player is loaded for the second time.
Another point is the length of one time achievements. Someone could add the button from the kills if you think it's necessary and add a better picture for the section...
Test Screenshots: