Make tarot face more configurable #289
Conversation
Admittedly, this is a personal preference, so feel free not to merge it. I just personally fine it useful to be able to draw a single card, and I suspect other people would as well — for a big draw, I'm likely to use a physical deck anyways.
It can be hard to remember that the "12 of Wands" is really the Knight of Wands, for instance. This puts a small reminder in the weekday digits, P for Page, Kn for Knight, Q for Queen, and K for King. It defaults to being disabled.
WesleyAC
force-pushed
the
tarot-fixes
branch
from
2f11ec2 to
328779b
Compare
| @@ -125,13 +152,25 @@ static void tarot_display(tarot_state_t *state) { | |||
| uint8_t suit = (card - NUM_MAJOR_ARCANA) / NUM_CARDS_PER_SUIT; | |||
| uint8_t rank = ((card - NUM_MAJOR_ARCANA) % NUM_CARDS_PER_SUIT) + 1; | |||
|
|
|||
| if (TAROT_FACE_CARD_REMINDER) { | |||
| if (rank == 11) { | |||
There was a problem hiding this comment.
Only issue I see with this is if you're using TAROT_LOOP_STYLE_ST_EN plus TAROT_FACE_CARD_REMINDER, you lose the start/end indicator here. I'm actually inclined to forget about the "St" and "En" strings altogether and use the lap indicator (with the default being LOOP_STYLE_LAP_END or honestly just remove the ability to configure it completely).
| #define DEFAULT_MAJOR_ARCANA_ONLY true | ||
| #define SHOW_REVERSED_CARDS true | ||
| #define TAROT_MIN_CARDS_TO_DRAW 1 | ||
| #define TAROT_FACE_CARD_REMINDER false |
There was a problem hiding this comment.
I say remove this configuration and just always show the rank
| @@ -32,6 +32,23 @@ | |||
| #include <string.h> | |||
| #include "tarot_face.h" | |||
|
|
|||
| #define DEFAULT_MAJOR_ARCANA_ONLY true | |||
| #define SHOW_REVERSED_CARDS true | |||
| #define TAROT_MIN_CARDS_TO_DRAW 1 | |||
There was a problem hiding this comment.
I like the default of 3 since it covers a "normal" spread but also can be used for a 1 or 2-card spread without changing this value (since you can just ignore the 2nd/3rd cards depending on your desired spread)
| #define TAROT_LOOP_STYLE_NONE 0 | ||
| #define TAROT_LOOP_STYLE_ST_EN 1 | ||
| #define TAROT_LOOP_STYLE_LAP_START 2 | ||
| #define TAROT_LOOP_STYLE_LAP_END 3 |
There was a problem hiding this comment.
I like showing the lap indicator on the last card. if that's your preferred mode too, I'm down to just make it the default and remove the configurability.
| start_end_string = " "; | ||
| start_end_string = " "; | ||
| watch_clear_indicator(WATCH_INDICATOR_LAP); | ||
| if (TAROT_LOOP_STYLE != TAROT_LOOP_STYLE_NONE && state->num_cards_to_draw > 1) { |
There was a problem hiding this comment.
if you want to keep the ability to configure this at compile time, change these if's to #ifdef statements to avoid unnecessary code making its way into the compiled object.
WesleyAC
added
the
tested-on-hw
This largely adds variables and uses existing variables in places they should already be, but it also changes the default minimum number of cards to one, down from three. Since that's personal preference, feel free to drop that commit if you'd like.