Open paglias opened 5 years ago
Also see #11257
Hello, can I take on this request?
@moniwang Sure, thanks! Post here if you have questions.
What are some of the bugs that occurred because of this achievements code?
@moniwang mainly some achievements did not show up correctly and were fixed in https://github.com/HabitRPG/habitica/commit/2842087a432d7d31de1b98fe751102e079ee33fc
so right now there are no bugs due to this but it would be great if we could change the way it works to make it less error prone for the future, let me know if you have any question :)
I can work on this.
@Helcostr Thanks! I've marked it as in progress for you. Sorry about the delay in our reply. Let us know if you've changed your mind since you posted!
Hi @Helcostr! Just checking in about this one as well. :)
You can mark as help wanted @shanaqui
@Helcostr Thanks for letting us know!
I'm taking this one!
We've found out that the achievements code is unnecessarily complex and difficult to manage, both on client and server which caused a few bugs, it should be simplified:
Instead of having a different notification type for each achievement (https://github.com/HabitRPG/habitica/blob/develop/website/server/models/userNotification.js#L36-L49) we should use one single type (maybe
ACHIEVEMENT
) with thedata
object of the achievement notification containing:user.achievements
)This should allow for a big simplification of the client side code here https://github.com/HabitRPG/habitica/blob/develop/website/client/src/components/notifications.vue#L137
UPDATE by @SabreCat 5/24/2022: We've done a chunk of this work, condensing the many different notification types related to achievements down to a handful of categories. We could go still further, though, to get down to just the
ACHIEVEMENT
type with the right data for parsing across platforms--the trick will be getting all the right data, strings, etc. without breaking anything in the process.