Skip to content

Conversation

@Pepe20129
Copy link
Collaborator

No description provided.

@@ -0,0 +1,163 @@
#ifndef UPGRADE_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Organization wise, I don't particularly feel that item upgrades are a substantially complex system to where we need to split them from item.h


static_assert(UPG_DEKU_NUTS_MAX <= 1 << BIT_WIDTH_UPG_DEKU_NUTS, "All deku nut upgrade values should fit in its assigned bitwidth");

#define BITS_PER_BYTE 8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has come up before (someone could correct me if I'm wrong or wishes to debate this), but the preference is to just inline the constant value of 8 instead of using CHAR_BIT from <climits> or a custom BITS_PER_BYTE define

/* 0x08 */ UPG_MAX
} UpgradeType;

#define BIT_WIDTH_UPG_QUIVER 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think personally I'd prefer it that you have this section organized like...

  • enum UpgradeType
  • BIT_WIDTH_* all listed in sequence, assert "Upgrades must fit in a u32"
  • SHIFT_* all listed in sequence

Then finally enum QuiverUpgrades, assert, enum BombBagUpgrades, assert

BIT_WIDTH_UPG_WALLET +
BIT_WIDTH_UPG_BULLET_BAG +
BIT_WIDTH_UPG_DEKU_STICKS +
BIT_WIDTH_UPG_DEKU_NUTS < sizeof(u32) * BITS_PER_BYTE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
BIT_WIDTH_UPG_DEKU_NUTS < sizeof(u32) * BITS_PER_BYTE,
BIT_WIDTH_UPG_DEKU_NUTS <= sizeof(u32) * BITS_PER_BYTE,

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.

2 participants