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

Cleanup for weapon explosions #7573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StephenCWills
Copy link
Member

With the changes in this PR, weapon explosions only do the collision check once, in AddWeaponExplosion(). I set the dontDeleteOnCollision flag to prevent the missile from immediately deleting itself. This means the missile animation will always play to completion regardless of whether the elemental damage strikes the target.

I modified DoAttack() to use the didhit flag to determine whether the missile should attempt to do damage to the target. Just like with elemental arrows, if the physical attack doesn't hit, the elemental attack doesn't hit either. In order to skip the collision check but still play the missile animation, DoAttack() simply sets the missile damage to zero if the physical attack missed. AddWeaponExplosion() will skip the collision check entirely if it sees that the missile damage is zero.

This resolves #7571

Source/missiles.cpp Show resolved Hide resolved
Comment on lines +795 to +796
int elementalDamage = didhit ? RandomIntBetween(player._pIFMinDam, player._pIFMaxDam) : 0;
AddMissile(position, { 1, 0 }, Direction::South, MissileID::WeaponExplosion, TARGET_MONSTERS, player, elementalDamage, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

didhit is calculated by PlrHitMonst / PlrHitPlr and this calculates if the hit can fail. This check is used to calculate the damage (0 if the hit failed).
In AddWeaponExplosion we call CheckMissileCol and this calls Plr2PlrMHit / MonsterMHit. These functions also calculate if the missile hits or not.
Does the weapon explosion now have to pass two hit checks instead of one? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I did this on purpose to match the behavior of elemental arrows. For arrows, the elemental explosion can't even spawn unless the arrow connects with a target so we can't really fix that behavior without refactoring the collision logic or adding yet another parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at the arrow logic and I'm with you.
It just showed me that we are changing / nerfing the elemental damage. But I'm fine with aligning it with the way elemental arrows are handled.

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.

Prevent multiple collision checks for elemental damage of melee weapons
2 participants