tmewett / BrogueCE

Brogue: Community Edition - a community-lead fork of the much-loved minimalist roguelike game
https://sites.google.com/site/broguegame/
GNU Affero General Public License v3.0
989 stars 107 forks source link

Possible to apply scroll of protect weapon and protect armor twice, etc. to the same item without warning, no extended protection? #614

Open ElLutzo opened 10 months ago

ElLutzo commented 10 months ago

I don't know if it is a real bug, but it feels not quite right:

It is possible to apply a scroll of protect weapon or protect armor twice, i.e. when applying a scroll of protect weapon to an already protected weapon or when applying a scroll of protect armor to an already protected armor item, there seems to be no difference to the first application. The same message is displayed, the item is still protected and the scroll is consumed. Is there a difference between an item, that was protected once, and an item, that was protected twice or more times?

It would be nice to be given a warning message that the item is already protected and that it is not possible to protect it more than once. Cancel the application. So the scroll is saved and can be used on another item.

Tested with Brogue CE, v.1.13.0 (normal brogue, normal mode) on seed 345925997: protect weapon on D1 and D5.

ElLutzo commented 10 months ago

I sketched a fix for standard Brogue 1.7.5 (not tested):

Items.c, lines from line 6977 (CE, v.1.13.0):

        case SCROLL_PROTECT_ARMOR:
            if (rogue.armor) {
                tempItem = rogue.armor;
                if ((tempItem->flags & ITEM_PROTECTED) && !(tempItem->flags & ITEM_CURSED)) {
                    itemName(tempItem, buf2, false, false, NULL);
                    sprintf(buf, "your %s is already protected. Save this scroll!", buf2);
                    messageWithColor(buf, &itemMessageColor, false);
                    scrollRead = false;
                    break;
                }
                tempItem->flags |= ITEM_PROTECTED;
                itemName(tempItem, buf2, false, false, NULL);
                sprintf(buf, "a protective golden light covers your %s.", buf2);
                messageWithColor(buf, &itemMessageColor, false);
                if (tempItem->flags & ITEM_CURSED) {
                    sprintf(buf, "a malevolent force leaves your %s.", buf2);
                    messageWithColor(buf, &itemMessageColor, false);
                    tempItem->flags &= ~ITEM_CURSED;
                }
            } else {
                message("a protective golden light surrounds you, but it quickly disperses.", false);
            }
            createFlare(player.xLoc, player.yLoc, SCROLL_PROTECTION_LIGHT);
            break;
        case SCROLL_PROTECT_WEAPON:
            if (rogue.weapon) {
                tempItem = rogue.weapon;
                if ((tempItem->flags & ITEM_PROTECTED) && !(tempItem->flags & ITEM_CURSED)) {
                    itemName(tempItem, buf2, false, false, NULL);
                    sprintf(buf, "your %s is already protected. Save this scroll!", buf2);
                    messageWithColor(buf, &itemMessageColor, false);
                    scrollRead = false;
                    break;
                }
                tempItem->flags |= ITEM_PROTECTED;
                itemName(tempItem, buf2, false, false, NULL);
                sprintf(buf, "a protective golden light covers your %s.", buf2);
                messageWithColor(buf, &itemMessageColor, false);
                if (tempItem->flags & ITEM_CURSED) {
                    sprintf(buf, "a malevolent force leaves your %s.", buf2);
                    messageWithColor(buf, &itemMessageColor, false);
                    tempItem->flags &= ~ITEM_CURSED;
                }
                if (rogue.weapon->quiverNumber) {
                    rogue.weapon->quiverNumber = rand_range(1, 60000);
                }
            } else {
                message("a protective golden light covers your empty hands, but it quickly disperses.", false);
            }
            createFlare(player.xLoc, player.yLoc, SCROLL_PROTECTION_LIGHT);
            break;
btaylor84 commented 10 months ago

I would argue this "mistake" should be allowed. It's not dangerous to the player, it's just a redundant play. You have the knowledge to prevent this loss, so leaving the possibility seems, um, educational. Who's to say that two scrolls might add levels of protection? Until you try, you might not know. Not everything unhelpful needs to be protected against.