Skip to content

Refactor AddToInventory to use pairs#144

Open
DJ73 wants to merge 3 commits intotagniam:masterfrom
DJ73:master
Open

Refactor AddToInventory to use pairs#144
DJ73 wants to merge 3 commits intotagniam:masterfrom
DJ73:master

Conversation

@DJ73
Copy link
Contributor

@DJ73 DJ73 commented Oct 31, 2020

Solves #133

This performs the refactoring of the function and other files that interact with said function but I'd recommend further work to restructure the code logic in light of changes such #140

@DJ73
Copy link
Contributor Author

DJ73 commented Oct 31, 2020

If there are no issues with this PR, could you accept it quickly for now so that I can submit another that replace calls to RandomInt() with equivalent calls to RandomEvent()

@tagniam
Copy link
Owner

tagniam commented Oct 31, 2020

Just as a heads up, you are free to submit multiple pull requests at a time, that's totally fine.

Also, don't replace RandomInt with RandomEvent - RandomEvent is for weighted random selections, while RandomInt is for uniformly distributed random integers, they serve completely different purposes.

@tagniam
Copy link
Owner

tagniam commented Oct 31, 2020

If you mean you'd like to replace all the switch cases with RandomEvent, then yes, that would be much appreciated.

@DJ73
Copy link
Contributor Author

DJ73 commented Oct 31, 2020

Yep, I'll do that across the project but was hoping that you review this first as this involves a lot of changes and I don't want to create conflicts between this and future PR

Copy link
Owner

@tagniam tagniam left a comment

Choose a reason for hiding this comment

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

Needs changes before merging, but marking this as hacktoberfest-accepted to count for Hacktoberfest.

include/Enemy.h Outdated
void DisplayHUD();

std::vector<int> GetDrops();
std::vector< std::pair<std::string, int> > GetDrops();
Copy link
Owner

Choose a reason for hiding this comment

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

Please use ITEMTYPE instead of strings

void GenerateValues();
int ReturnShakenDie();
char ReturnItem();
std::string ReturnItem();
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

// 2 -> Potions
// 3 -> Whetstones
int Item;
std::string Item;
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

include/Player.h Outdated
int Action();
void UseItem();
void AddToInventory(std::vector<int>);
void AddToInventory(std::pair<std::string, int>);
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

@DJ73
Copy link
Contributor Author

DJ73 commented Nov 4, 2020

Switched to ITEMTYPE wherever applicable

@DJ73
Copy link
Contributor Author

DJ73 commented Nov 11, 2020

Could you review this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants