server icon indicating copy to clipboard operation
server copied to clipboard

[CPP] Fix non-equipment items can cause map crash

Open ampitere opened this issue 9 months ago • 2 comments

I affirm:

  • [x] I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • [x] I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • [x] I have read and understood the Contributing Guide and the Code of Conduct.
  • [x] I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Currently since CLuaBaseEntity::canEquipItem only takes an item ID as a param then casts it, it will sometimes be a non-equipment item. With the addition of isEquippableByRace it attempts to check a modifier that doesn't exist on non-equipment items. To fix this we'll add a return false if the item is not equipment. Resolves https://github.com/LandSandBoat/server/issues/5703

Steps to test these changes

  1. Go to a conquest NPC
  2. Keep opening the "Are you sure you want to purchase the X?" menu on a non-equipment item (Scroll of Instant Warp for example)

ampitere avatar May 13 '24 05:05 ampitere

Just to be sure -- you also checked ranged weapons, ammo, shields, things that aren't weapons but are equippable in ammo, etc?

I'm sure it works based on the set type/get type functions

void CItem::setType(uint8 type)
{
    m_type |= type;
}

bool CItem::isType(ITEM_TYPE type) const
{
    return (m_type & type);
}

all of those things should be under "CItemEquipment" but this GetType junk is a bit unknown to me

WinterSolstice8 avatar May 13 '24 05:05 WinterSolstice8

Just to be sure -- you also checked ranged weapons, ammo, shields, things that aren't weapons but are equippable in ammo, etc?

I'm sure it works based on the set type/get type functions

void CItem::setType(uint8 type)
{
    m_type |= type;
}

bool CItem::isType(ITEM_TYPE type) const
{
    return (m_type & type);
}

all of those things should be under "CItemEquipment" but this GetType junk is a bit unknown to me

The cast to CItemEquipment changes the type to EQUIPMENT so if it's not that type then it failed the cast and still has it's previous type as far as I can tell. Shields, ammo, fishing rods, bait, lures, guns, bows, etc. are all in item_equipment.sql so those all should properly cast to equipment.

ampitere avatar May 13 '24 05:05 ampitere