Skip to content

Add item type: Molotov#131

Open
Shaedil wants to merge 9 commits intotagniam:masterfrom
Shaedil:master
Open

Add item type: Molotov#131
Shaedil wants to merge 9 commits intotagniam:masterfrom
Shaedil:master

Conversation

@Shaedil
Copy link
Contributor

@Shaedil Shaedil commented Oct 21, 2020

Solves #128.

Shaedil and others added 5 commits October 16, 2020 14:49
Merge pull request #3 from tagniam/master
Merge pull request #4 from tagniam/master
* Every player starts with 1 molotov in inventory
* It costs 75 coins to buy 1 molotov in the store
* It can be gambled for
* It is documented in the game instructions
add: Comments about molotov item implementation
@tagniam
Copy link
Owner

tagniam commented Oct 23, 2020

Left some comments in the diff, also when I play the game, the extra damage messages go too fast, I think there should be a pause after each. Looks good so far though!

@tagniam
Copy link
Owner

tagniam commented Oct 25, 2020

Looks like the build failed?

@tagniam
Copy link
Owner

tagniam commented Oct 28, 2020

Any updates @Shaedil?

@Shaedil
Copy link
Contributor Author

Shaedil commented Oct 28, 2020

So sorry, I've been busy with my classes. I've been working on it since two days ago, I believe I have the fix for this. I just can't believe how silly the mistake was. I'll be pushing the final changes by the end of the day.

@tagniam
Copy link
Owner

tagniam commented Oct 28, 2020

No worries! Just making sure I have time to merge this before the end of October, if you wanted it to count for Hacktoberfest. Good luck with your classes :)

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.

Left some more comments, but basically we'd like to avoid adding any code to the main game loop if possible. LMK if you have any questions about that.

Comment on lines +361 to +374
// Player turn is over, enemy's status effect damage is calculated
if (0 < _Player->extradamageturns && _Player->extradamageturns <= 2 && damagePlayer != -1) {
_Player->extradamageturns--;
_Enemy->TakeDamage(extradamagePlayer);
_Player->ReturnDialog(extradamagePlayer, " damage points from burn dealt");
}
// Check if enemy has had molotov thrown at him before or now
if (damagePlayer == -3) {
damagePlayer = Common::RandomInt(30, 40);
_Enemy->TakeDamage(damagePlayer);
_Player->ReturnDialog(damagePlayer, " regular damage points dealt");
_Player->extradamageturns = 2;
_Player->ReturnDialog(_Player->extradamageturns, " turns of extra burn damage");
}
Copy link
Owner

Choose a reason for hiding this comment

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

There is still molotov logic in the game loop, we'd like to avoid that, I think this can be done wholly in the Player/Enemy classes. Let me know if you have any questions about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can reduce this to just one function call, if that is what you mean by "done wholly in the Player/Enemy classes". Otherwise, would you rather the molotov damage be added onto the main damage i.e. damagePlayer?

Copy link
Owner

@tagniam tagniam Oct 30, 2020

Choose a reason for hiding this comment

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

Ideally there shouldn't be any extra function calls or code in the main game loop. So yeah, I think the molotov damage should be added onto the main damage, then the dialog messages can be printed in the Player class' UseMolotov, similar to UseBomb, BowAndArrow, etc. Keeping track of extradamageturns should also be in the Player class.

Copy link
Owner

Choose a reason for hiding this comment

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

With that approach, you don't need to modify anything in the Enemy class. So really it should be done wholly in the Player class, if I'm not mistaken.

int extradamagePlayer = _Player->ReturnExtraMolotovDamage();
// Player's turn to attack Enemy or choose other action.
if (damagePlayer != SKIP_TURN) {
if (damagePlayer != SKIP_TURN && damagePlayer != -3) {
Copy link
Owner

Choose a reason for hiding this comment

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

Avoid using magic numbers like -3, you should define constants e.g. like SKIP_TURN so it's easier to read :)

Comment on lines +388 to +389
// Reset remaining molotov damage, and prevent it from overlapping into the next battle.
_Player->extradamageturns = 3;
Copy link
Owner

Choose a reason for hiding this comment

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

See comment above

bombs += amount;
break;
case ITEMTYPE::MOLOTOV:
++molotovs;
Copy link
Owner

Choose a reason for hiding this comment

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

This should be molotovs += amount

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