Open satoshinm opened 7 years ago
Hey, I'm just jumping in and don't mean to be pedantic but wouldn't it be wiser to merge those 2 new integers you added (show_info_text
and show_ui
) into a single one and use flags? I'm not fully familiar with Craft's code so I'm asking purely out of curiosity
Bitfields you mean? I suppose it could be more compact, but more complicated (requiring bitmasks etc.). Maybe something like this? (works on my system, but not sure how portable it is):
--- a/src/main.c
+++ b/src/main.c
@@ -165,8 +165,10 @@ typedef struct {
Block block1;
Block copy0;
Block copy1;
- int show_info_text;
- int show_ui;
+ struct {
+ int show_info_text : 1;
+ int show_ui : 1;
+ };
} Model;
static Model model;
and/or was also considering using stdbool.h, new in C99 (but the rest of the codebase doesn't use it, so I stuck with int for consistency)
I was thinking about direct bitwise manipulation on a single integer. Admittedly it leads to more verbose tests but those 2 integers seemed so closely related that when I read your patch I wondered if there was a rational behind this choice I might have missed. Anyway, don't mind me I was just passing by! Thanks for answering
Ah I see what you're saying, something like:
if (SHOW_WIREFRAME && (g->show_flags & 1)) { // check
...
g->show_flags |= 1; // show ui
g->show_flags &= ~1; // hide ui
if (g->show_flags & 2) // show info text
I think it's simpler to have separate variables (maybe faster too? not benchmarked).
But for what it's worth, I did end up changing to use the C standard stdbool.h type where possible in my fork: https://github.com/satoshinm/NetCraft/commit/e3ce375e4374e2df227e3f65484a4a506135b8a3, including for these two flags. Since bool
(or in actuality _Bool
from C99) only stores non-zero values as 1, this may allow further optimization.
A simple addition to allow the user to toggle all the user interface elements (F1 keybinding) and the debug info text (F3), useful for taking clutter-free screenshots etc.