-
Notifications
You must be signed in to change notification settings - Fork 172
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
fix: weekly leaderboards and shooting gallery high score #1719
base: main
Are you sure you want to change the base?
Conversation
c54360d
to
2054df5
Compare
@@ -180,7 +182,7 @@ std::vector<ILeaderboard::Entry> FilterTo10(const std::vector<ILeaderboard::Entr | |||
std::vector<ILeaderboard::Entry> FilterWeeklies(const std::vector<ILeaderboard::Entry>& leaderboard) { | |||
// Filter the leaderboard to only include entries from the last week | |||
const auto currentTime = std::chrono::system_clock::now(); | |||
auto epochTime = currentTime.time_since_epoch().count(); | |||
auto epochTime = std::chrono::duration_cast<std::chrono::seconds>(currentTime.time_since_epoch()).count(); | |||
constexpr auto SECONDS_IN_A_WEEK = 60 * 60 * 24 * 7; // if you think im taking leap seconds into account thats cute. |
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.
std::chrono should have built-in conversion factors for a lot of this
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.
and that call would be?
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.
Something like this?
Not sure if it's feasible to store the lastPlayedTimestamp as an std::chrono::time_point, but if it is, it would avoid some of the pitfalls of using time_since_last_epoch() and count()
std::vector<ILeaderboard::Entry> FilterWeeklies(const std::vector<ILeaderboard::Entry>& leaderboard) {
// Filter the leaderboard to only include entries from the last week
const auto currentTime = std::chrono::system_clock::now();
std::vector<ILeaderboard::Entry> weeklyLeaderboard;
for (const auto& entry : leaderboard) {
const auto elapsedTime = currentTime - entry.lastPlayedTimestamp;
if (elapsedTime < std::chrono::weeks{ 1 }) {
weeklyLeaderboard.push_back(entry);
}
}
return weeklyLeaderboard;
}
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.
Alternatively, you could also use std::copy_if:
// Filter the leaderboard to only include entries from the last week
std::vector<ILeaderboard::Entry> FilterWeeklies(const std::vector<ILeaderboard::Entry>& leaderboard) {
const auto currentTime = std::chrono::system_clock::now();
std::vector<ILeaderboard::Entry> weeklyLeaderboard;
std::copy_if(leaderboard.cbegin(), leaderboard.cend(), std::back_inserter(weeklyLeaderboard), [&](const auto& entry) {
const auto elapsedTime = currentTime - entry.lastPlayedTimestamp;
return elapsedTime < std::chrono::weeks{ 1 };
});
return weeklyLeaderboard;
}
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.
Something like this?
Not sure if it's feasible to store the lastPlayedTimestamp as an std::chrono::time_point, but if it is, it would avoid some of the pitfalls of using time_since_last_epoch() and count()
std::vector<ILeaderboard::Entry> FilterWeeklies(const std::vector<ILeaderboard::Entry>& leaderboard) { // Filter the leaderboard to only include entries from the last week const auto currentTime = std::chrono::system_clock::now(); std::vector<ILeaderboard::Entry> weeklyLeaderboard; for (const auto& entry : leaderboard) { const auto elapsedTime = currentTime - entry.lastPlayedTimestamp; if (elapsedTime < std::chrono::weeks{ 1 }) { weeklyLeaderboard.push_back(entry); } } return weeklyLeaderboard; }
i dont think this will work. the timestamp in the database is already a unix timestamp and we need to compare with that, thats the whole reason im casting to seconds before the subtraction. the default is in milliseconds and as such no time meets the criteria
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.
Altered approach:
// Filter the leaderboard to only include entries from the last week
std::vector<ILeaderboard::Entry> FilterWeeklies(const std::vector<ILeaderboard::Entry>& leaderboard) {
using namespace std::chrono;
const auto currentTime = system_clock::now();
std::vector<ILeaderboard::Entry> weeklyLeaderboard;
std::copy_if(leaderboard.cbegin(), leaderboard.cend(), std::back_inserter(weeklyLeaderboard), [&](const auto& entry) {
const auto lastPlayedTime = sys_time{ seconds{ entry.lastPlayedTimestamp } };
return (currentTime - lastPlayedTime) < weeks{ 1 };
});
return weeklyLeaderboard;
}
Not sure if this is valid since I'm writing this on my phone, but hopefully you get the gist
tested that weekly leaderboards work again.
Fixes #1710
tested that shooting gallery shows the correct highscore.