Skip to content
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

Updates to emoji implementation #394

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Updates to emoji implementation #394

wants to merge 3 commits into from

Conversation

zardus
Copy link
Contributor

@zardus zardus commented Apr 7, 2024

Right now, we store emojis by putting the emoji in the name, a description, and a reference to the dojo (if any) for which the emoji was earned. This causes some headaches:

  1. To understand the type of emoji, you need to actually read the description or remember what different emojis mean.
  2. If we want to update an emoji (we did this for the welcome dojo, for example), we need to do crazy sql queries involving emoji, which is insane (and error prone, depending on one's terminal).

This PR changes the design to:

  • the emoji name is a signifier for the type of emoji, such as "dojo_sensei", "completion", "bug_bounty", "first_place", etc.
  • the emoji category remains the dojo hex id, if the emoji is bound to a dojo
  • no one has to consult the description for anything
  • the actual emoji to display is pulled, depending on the name field, from the SPECIAL_EMOJIS global or from the dojo.data['award']['emoji'] of the respective dojo

The main effects have to do with dojos changing their awards:

  1. When a dojo changes its emoji, the results are now immediately reflected rather than requiring manual effort. I'm not sure which style we prefer.
  2. This means that the dojo can delete its emoji, and then we don't know what emoji to display. Currently, this displays ❌ via SPECIAL_EMOJIS["missing"]

This PR maintains backwards compatibility with legacy emojis, though presumably we'll update the DB and remove that compatibility ASAP.

A further thing we might want to do is fix up emoji.category to key off the integer dojo ID, so that we can actually join onto it, but the current was is workable as well.

@zardus
Copy link
Contributor Author

zardus commented Apr 8, 2024

Oh, forgot the other thing that we might want to do going forward: we could further subtype awards, rather than just Award, Belt, and Emoji, to also include the different types of emoji awards (completion, sensei, etc) instead of (ab)using name. Not so excited about that, but it's probably a more correct way to do things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant