-
Notifications
You must be signed in to change notification settings - Fork 97
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
Decide consensus of naming "cardinality" constants. #221
Comments
Im ok with a _MAX suffix |
_MAX is good, I didn't think of that one |
|
this is already not consistent in the code we have, but imo MULCH_MAX is the way to go here |
I think MAX is the best. What makes stuff like The issue still remains unresolved for set-like structures, such as arrays. What terminology should we use to describe the cardinality of an array? Some options are length, size, and count. |
length imo, considering stuff like player name sounds a lot better being PLAYER_NAME_LENGTH etc |
Between my day job and hobbies in Java and C#, they can't even agree and I have to use all three of length, size, and count. I don't have any preferences so any of those is fine. |
Fyi, this can be either LEN or LENGTH. |
LENGTH better imo, really dont like abbreviations when there's no reason to |
Fyi, we should probably clarify between using Num and Total. Total should be used when aggregating together a quantity-like value, so we say |
I'd also debate the use of |
Get is another issue entirely that I didn't want to open just yet because of how intricate it is. |
Gen 3 had the |
I personally prefer |
Count runs into issues as you can use it to refer to both a calculated value and a cardinality. E.g. for the given shared code: // take a guess what this array is
extern const u8 gVowels[];
#define VOWEL_A 'A'
#define VOWEL_E 'E'
#define VOWEL_I 'I'
#define VOWEL_O 'O'
#define VOWEL_U 'U' If we use the #define VOWEL_COUNT 5
u32 String_GetVowelCount(const u8 * data, u32 len) {
u32 count = 0;
int i, j;
for (i = 0; i < len; i++) {
u8 curChar = data[i];
for (j = 0; j < VOWEL_COUNT; j++) {
if (curChar == gVowels[j]) {
count++;
break;
}
}
}
return count;
} But if we use #define MAX_VOWELS 5
u32 String_GetVowelCount(const u8 * data, u32 len) {
u32 count = 0;
int i, j;
for (i = 0; i < len; i++) {
u8 curChar = data[i];
for (j = 0; j < MAX_VOWELS; j++) {
if (curChar == gVowels[j]) {
count++;
break;
}
}
}
return count;
} |
Gotcha, I understand now. |
This might be a problem though. AsparagusEduardo mentioned that in the gen 3 repos, MAX refers to a hard limit of the maximum cardinality a set could have, while NUM refers to the current cardinality of the set. This happens in the gen 4 repos as well. #define NUM_APRICORN_TREE 31
#define MAX_APRICORN_TREE 128
typedef struct SAVE_MISC_DATA {
APRICORN_TREE apricorn_trees[MAX_APRICORN_TREE];
// etc...
};
static const u8 sTreeApricorns[NUM_APRICORN_TREE] = {
APRICORN_BLK, // Route 1
APRICORN_PNK, // Route 2 North
APRICORN_YLW, // Route 8
// etc...
};
void ApricornTrees_Init(APRICORN_TREE *trees) {
int i;
MI_CpuClear8(trees, sizeof(APRICORN_TREE) * MAX_APRICORN_TREE);
for (i = 0; i < MAX_APRICORN_TREE; i++) {
trees[i].unk_0 = 0;
}
}
// ApricornTrees_IterateOverAndDoSomething
void sub_0202AE0C(APRICORN_TREE *trees) {
int i;
for (i = 0; i < MAX_APRICORN_TREE || i < NUM_APRICORN_TREE; i++) {
sub_0202AE30(&trees[i]);
trees[i].unk_0 = 1;
trees[i].unk_1 = 1;
}
} Both |
One possibility: say |
We could also say BOOL Save_NumModifiedPCBoxesIsTooMany(SaveData *saveData) {
return Save_CalcNumModifiedPCBoxes(saveData) >= 6;
} We would rename the relevant terms to: // actually, Save_HasTooManyModifiedPCBoxes is probably a better name here
BOOL Save_ModifiedPCBoxesCountIsTooMany(SaveData *saveData) {
return Save_CalcModifiedPCBoxesCount(saveData) >= 6;
} |
Fyi, count might have issues because it can be used as a verb? static u32 Save_CalcNumModifiedPCBoxes(SaveData *saveData) {
return PCModifiedFlags_CountModifiedBoxes(Save_CalcPCBoxModifiedFlags(saveData));
} |
Idea: use LIMIT for "hardware" limit // take a guess what this array is
extern const u8 gPokeBallItems[];
#define MAX_GROUND_ITEMS 10
#define LIMIT_GROUND_ITEMS 20
typedef struct GroundItem {
u16 item;
s16 x;
s16 y;
u16 quantity;
} GroundItem;
typedef struct GroundItems {
GroundItem data[LIMIT_GROUND_ITEMS];
} GroundItems;
const u16 sGroundItemIds[MAX_GROUND_ITEMS] = {
ITEM_POKE_BALL,
ITEM_POTION,
// etc...
};
extern GroundItems gGroundItems;
void GroundItems_Init(void) {
int i;
MI_CpuClear8(gGroundItems, sizeof(GroundItem) * LIMIT_GROUND_ITEMS);
for (i = 0; i < LIMIT_GROUND_ITEMS && i < MAX_GROUND_ITEMS; i++) {
gGroundItems.data[i].item = sGroundItemIds[i];
gGroundItems.data[i].x = Random();
gGroundItems.data[i].y = Random();
gGroundItems.data[i].quantity = Random();
}
}
// or GroundItems_GetNumOfSpecificItem
u32 GroundItems_FindNumInstancesOfItem(u16 item) {
u32 numInstances;
int i;
for (i = 0; i < LIMIT_GROUND_ITEMS && i < MAX_GROUND_ITEMS; i++) {
if (gGroundItems.data[i].item == item) {
numInstances++;
}
}
return numInstances;
}
u32 GroundItems_CalcTotalQuantityOfItem(u16 item) {
u32 totalQuantity;
int i;
for (i = 0; i < LIMIT_GROUND_ITEMS && i < MAX_GROUND_ITEMS; i++) {
if (gGroundItems.data[i].item == item) {
totalQuantity += gGroundItems.data[i].quantity;
}
}
return totalQuantity;
}
u32 GroundItems_GetNumGroundItems(void) {
u32 count;
int i;
for (i = 0; i < LIMIT_GROUND_ITEMS && i < MAX_GROUND_ITEMS; i++) {
if (gGroundItems.data[i].quantity != 0) {
count++;
}
}
return count;
} |
I don't mind NUM_MULCHES and MULCHES_MAX for the hardware limit |
The only issue with NUM is that it can lead to two concepts sharing the same name, e.g. u32 GroundItems_GetNumGroundItems(void) {
u32 count;
int i;
for (i = 0; i < MAX_GROUND_ITEMS && i < NUM_GROUND_ITEMS; i++) {
if (gGroundItems.data[i].quantity != 0) {
count++;
}
}
return count;
} |
So it seems like we have two choices when referring to set cardinality. Refer to my above code samples when I mention specific functions. Choice 1
Advantages:
Disadvantages:
Choice 2
Advantages:
Disadvantages:
#define MAX_GROUND_ITEMS 10
#define GROUND_ITEMS_LIMIT 20 An alternativeIt's possible that the instances where the hardware limit needs to be a constant are so far and few that we can get away with needing to create a name for it. For example, in the apricorn example, we can call the cardinality referring to the fixed data as |
Compilation of symbols containing the phrase "count" or "num" (case insensitive without word boundary): https://pastebin.com/4DGN38mU |
Worth mentioning: use #define BAG_LENGTH 20
// BAD
// #define NUM_BAG_ITEMS 20
typedef struct Gen1ItemBagSlot {
u8 itemId;
u8 quantity;
} Gen1ItemBagSlot;
typedef struct Gen1ItemBag {
u8 numBagItems;
Gen1ItemBagSlot bagSlots[BAG_LENGTH];
} Gen1ItemBag;
u8 GetNumBagItems(Gen1ItemBag * bag) {
return bag->numBagItems;
}
u8 CountNumPokeBallItems(Gen1ItemBag * bag) {
u32 count;
int i;
for (i = 0; i < bag->numBagItems; i++) {
if (IsPokeBallItem(bag->bagSlots[i].itemId)) {
count++;
}
}
}
u8 SumBagItemsQuantity(Gen1ItemBag * bag) {
u32 totalQuantity;
int i;
for (i = 0; i < bag->numBagItems; i++) {
totalQuantity += bag->bagSlots[i].quantity;
}
return totalQuantity;
}
u8 CountNumUniqueBagItems(Gen1ItemBag * bag) {
// almost becomes a problem? but NUM_ITEMS is different than numBagItems
// still might be confusing
bool8 uniqueBagItemsTracker[NUM_ITEMS] = {0};
int i;
u32 numUniqueBagItems = 0;
for (i = 0; i < bag->numBagItems; i++) {
// pretend Game Freak is bad and didn't write the code like this
// bool8 bagItemFoundAlready = uniqueBagItemsTracker[bag->bagSlots[i].itemId];
// if (!bagItemFoundAlready) {
// numUniqueBagItems++;
uniqueBagItemsTracker[bag->bagSlots[i].itemId] = TRUE;
//}
}
for (i = 0; i < NUM_ITEMS; i++) {
if (uniqueBagItemsTracker[i]) {
numUniqueBagItems++;
}
}
return numUniqueBagItems;
} Maybe you can name |
Currently I can think of two options for naming dynamic cardinalities: Option 1: typedef struct MapEvents {
u32 numBgEvents;
u32 numObjectEvents;
u32 numWarpEvents;
u32 numCoordEvents;
// rest omitted
} MapEvents; Option 2: typedef struct MapEvents {
u32 bgEventsCount;
u32 objectEventsCount;
u32 warpEventsCount;
u32 coordEventsCount;
// rest omitted
} MapEvents;
|
There's a lot to take in here, but based on the examples I listed, I think |
One final thing. Capacity could be used to describe the maximum amount of elements allowed in a "real life" container, so you can use capacity to describe a bag pocket ( |
Peanut Colada luckytyphlosion — Today at 3:26 PM
|
It's probably agreeable that |
This issue has had no activity for 60 days and will be marked stale. If there is no further activity, it will be closed in 30 days. |
Cardinality is just number of elements for a set. So it could refer to the total length of an array, the number of values an enum could have, and other stuff.
For example:
NUM_
to refer to the number of elements in a set is weird, because you could also havenumMulches
which stores the current amount of mulches. TODO investigate what naming we want.We could say
TOTAL_MULCHES
, but in the above example,total
is used to determine a dynamic cardinality.It's probably good to determine what terminology should be used.
The text was updated successfully, but these errors were encountered: