Skip to content
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

Displays missing item in battle #44

Merged
merged 7 commits into from
Oct 27, 2017
Merged

Displays missing item in battle #44

merged 7 commits into from
Oct 27, 2017

Conversation

mario51y1
Copy link
Contributor

refers #41

Copy link
Contributor

@Shanmus4 Shanmus4 left a comment

Choose a reason for hiding this comment

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

Merge this with my work

@tagniam
Copy link
Owner

tagniam commented Oct 22, 2017

@mario51y1 It looks like there's a bug where if you repeatedly use an item not in the inventory, the enemy will heal themselves instead of doing nothing.

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.

If you're still up for this PR, I think I found why the bug occurs

// Player's turn to attack Enemy.
_Enemy->TakeDamage(_Player->Attack());

_Enemy->TakeDamage(damagePlayer);
Copy link
Owner

Choose a reason for hiding this comment

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

Bug occurs here, when damagePlayer = 2, 2 points are added to enemy's health (which makes it look like it's healing)

@mario51y1
Copy link
Contributor Author

Yeah, thought I made the if statement that checks the damage but seems it's missing.

@tagniam
Copy link
Owner

tagniam commented Oct 26, 2017

Could you make -2 a constant, like SKIP_TURN? This will ensure readability

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.

Some more suggested changes

@@ -114,8 +114,10 @@ int Player::Attack(){
{
PlaySecondaryAttack();
return BowAndArrow();
}
Copy link
Owner

Choose a reason for hiding this comment

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

Missing ending curly brace

case 7:
// Player sharpens their weapon with a whetstone.
// Does not execute if there are no whetstones in inventory.
// No damage is done to the enemy.
if (whetstones > 0) {
UseWhetstone();
return 0;
} else {
cout << "No whetsones in the inventory!" << endl;
Copy link
Owner

Choose a reason for hiding this comment

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

Spelling: whetstones

}
break;
else
cout << "No arrows in the inventory!" << endl;
Copy link
Owner

Choose a reason for hiding this comment

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

Wrap this else block with curly braces

@@ -126,24 +128,30 @@ int Player::Attack(){
// Does not execute if there are no bombs in the inventory.
if (bombs > 0)
return UseBomb();
break;
else
Copy link
Owner

Choose a reason for hiding this comment

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

Wrap this else block with curly braces

@tagniam
Copy link
Owner

tagniam commented Oct 26, 2017

Also, weirdly when I try to use a non-existent item, the message is displayed twice as long.

@mario51y1
Copy link
Contributor Author

Ok, so is something else missing? Had some troubles with visual sutudio and I couldnt test if everything is working as suposed.

@tagniam tagniam merged commit d398695 into tagniam:master Oct 27, 2017
@tagniam
Copy link
Owner

tagniam commented Oct 27, 2017

Thanks for the patience while I reviewed this PR @mario51y1. Everything's good now, I just had to fix a little bug in the code but it's all working now. Happy hacktoberfest! 🎃

@mario51y1
Copy link
Contributor Author

Thanks for your patience as well working wiht a newbie in GitHub, I'm happy that evertyhing is working now. Happy Hacktoberfest too! 🎃

@tagniam
Copy link
Owner

tagniam commented Oct 27, 2017

I was in your position as a newbie on github just a few years ago, everyone starts somewhere 😄 Hope you continue to contribute to open source in the future!

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.

3 participants