weapon of jelly slaying splits before it kills

[rpolve]

version

v1.8.3

steps to reproduce

• wield a weapon of jelly slaying
• hit any kind of jelly

expected result

• jelly is instantly killed
• doesn't split

actual result

• splits first
• only first half jelly is instantly killed, second half stays there

additional info

Seed 166:
    Depth 2:
        A +2 dagger of jelly slaying <12>

[rpolve]

Same thing with quietus.

Seed 832, D1: +3 spear of quietus

[withinwheels]

I can reproduce on 4033c6b in seed 69. Pick up the southern of the two whips on D14, find jelly on D5

[withinwheels]

These lines illustrate things succinctly. In attack():

        moralAttack(attacker, defender);

        if (attacker == &player && rogue.weapon && (rogue.weapon->flags & ITEM_RUNIC)) {
            magicWeaponHit(defender, rogue.weapon, sneakAttack || defenderWasAsleep || defenderWasParalyzed);
        }

Jelly splitting is handled in moralAttack (presumably so they split on any kind of intentional damage, such as a staff of firebolt) while quietus and vorpal slaying are handled in magicWeaponHit.

The knee-jerk fix is to switch these two blocks. But is there a reason moral attacks are considered before runic effects.. :shrug:?

[tipleyg]

I tried to reproduce the issue but did not find the weapons indicated in seeds 166 and 832, possibly due to how long ago they were cited. I tried to figure out how to apply this enchantment to the starting weapon for testing purposes (in RogueMain.c) but I'm new here and I don't know my way around very well. I tried to add this and it wouldn't compile theItem->enchant2 = W_SLAYING;

[withinwheels]

On eb3df24 (tag v1.10.1) I can reproduce on seed 213 using the rapier of jelly slaying in the vault on D2 and the jelly on D4.

[withinwheels]

..I think the right move here may be to break the two blocks in moralAttack into two functions. The first block, which is just the conduct check, might remain as moralAttack.

The second block, dealing with defender reactions to being attacked, might be extracted to something like defensiveReaction. The jelly-splitting logic would move along with the rest of the second block into this new function.

Then, every call site to moralAttack would grow a call to defensiveReaction immediately after, except the call site in attack. There, the call to defensiveReaction would come after the magicWeaponHit block, and be guarded by a death check. This way quietus/slaying would get their chance before the defender reacts.

EDIT: The second block is already guarded by a death check, no need for an extra one.

[zenzombie]

In my PR I just moved the call to moralAttack after magicWeaponHit. Seems to work. Hope it doesn't cause a different issue.