diff --git a/changelog.txt b/changelog.txt index d7ce432cb..3f98e6c37 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,8 @@ EQEMu Changelog (Started on Sept 24, 2003 15:50) ------------------------------------------------------- +== 12/23/2014 == +Uleat: Tidied up some ItemInst* declarations and added some nullptr checks. + == 12/22/2014 == Trevius: (RoF2) Fixed Tracking. Trevius: (RoF+) Added a work-around for the cursor buffer issue. diff --git a/common/item.cpp b/common/item.cpp index 583cfdcb4..e5a788879 100644 --- a/common/item.cpp +++ b/common/item.cpp @@ -1186,13 +1186,13 @@ int16 Inventory::_HasItem(std::map& bucket, uint32 item_id, ui for (itb = inst->_begin(); itb != inst->_end(); ++itb) { ItemInst* baginst = itb->second; - if (baginst->GetID() == item_id) { + if (baginst && baginst->GetID() == item_id) { quantity_found += (baginst->GetCharges() <= 0) ? 1 : baginst->GetCharges(); if (quantity_found >= quantity) return Inventory::CalcSlotId(it->first, itb->first); } for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { - if (baginst->GetAugmentItemID(i) == item_id && quantity <= 1) + if (baginst && baginst->GetAugmentItemID(i) == item_id && quantity <= 1) return legacy::SLOT_AUGMENT; // Only one augment per slot. } } @@ -1230,13 +1230,13 @@ int16 Inventory::_HasItem(ItemInstQueue& iqueue, uint32 item_id, uint8 quantity) for (itb = inst->_begin(); itb != inst->_end(); ++itb) { ItemInst* baginst = itb->second; - if (baginst->GetID() == item_id) { + if (baginst && baginst->GetID() == item_id) { quantity_found += (baginst->GetCharges() <= 0) ? 1 : baginst->GetCharges(); if (quantity_found >= quantity) return Inventory::CalcSlotId(MainCursor, itb->first); } for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { - if (baginst->GetAugmentItemID(i) == item_id && quantity <= 1) + if (baginst && baginst->GetAugmentItemID(i) == item_id && quantity <= 1) return legacy::SLOT_AUGMENT; // Only one augment per slot. } @@ -1330,7 +1330,7 @@ int16 Inventory::_HasItemByLoreGroup(std::map& bucket, uint32 if (inst->GetItem()->LoreGroup == loregroup) return it->first; - ItemInst* Aug; + ItemInst* Aug = nullptr; for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { Aug = inst->GetAugment(i); if (Aug && Aug->GetItem()->LoreGroup == loregroup) @@ -1345,7 +1345,7 @@ int16 Inventory::_HasItemByLoreGroup(std::map& bucket, uint32 if (baginst && baginst->IsType(ItemClassCommon) && baginst->GetItem()->LoreGroup == loregroup) return Inventory::CalcSlotId(it->first, itb->first); - ItemInst* Aug2; + ItemInst* Aug2 = nullptr; for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { Aug2 = baginst->GetAugment(i); if (Aug2 && Aug2->GetItem()->LoreGroup == loregroup) @@ -1373,7 +1373,7 @@ int16 Inventory::_HasItemByLoreGroup(ItemInstQueue& iqueue, uint32 loregroup) if (inst->GetItem()->LoreGroup == loregroup) return MainCursor; - ItemInst* Aug; + ItemInst* Aug = nullptr; for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { Aug = inst->GetAugment(i); if (Aug && Aug->GetItem()->LoreGroup == loregroup) @@ -1389,7 +1389,7 @@ int16 Inventory::_HasItemByLoreGroup(ItemInstQueue& iqueue, uint32 loregroup) return Inventory::CalcSlotId(MainCursor, itb->first); - ItemInst* Aug2; + ItemInst* Aug2 = nullptr; for (int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { Aug2 = baginst->GetAugment(i); if (Aug2 && Aug2->GetItem()->LoreGroup == loregroup) @@ -1719,6 +1719,8 @@ void ItemInst::ClearByFlags(byFlagSetting is_nodrop, byFlagSetting is_norent) end = m_contents.end(); for (; cur != end;) { ItemInst* inst = cur->second; + if (inst == nullptr) + continue; const Item_Struct* item = inst->GetItem(); del = cur; ++cur; diff --git a/common/shareddb.cpp b/common/shareddb.cpp index 7dff17b43..07eb5e84b 100644 --- a/common/shareddb.cpp +++ b/common/shareddb.cpp @@ -177,6 +177,7 @@ bool SharedDatabase::SaveInventory(uint32 char_id, const ItemInst* inst, int16 s } bool SharedDatabase::UpdateInventorySlot(uint32 char_id, const ItemInst* inst, int16 slot_id) { + // need to check 'inst' argument for valid pointer uint32 augslot[EmuConstants::ITEM_COMMON_SIZE] = { NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM }; if (inst->IsType(ItemClassCommon)) @@ -221,6 +222,7 @@ bool SharedDatabase::UpdateInventorySlot(uint32 char_id, const ItemInst* inst, i } bool SharedDatabase::UpdateSharedBankSlot(uint32 char_id, const ItemInst* inst, int16 slot_id) { + // need to check 'inst' argument for valid pointer uint32 augslot[EmuConstants::ITEM_COMMON_SIZE] = { NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM, NO_ITEM }; if (inst->IsType(ItemClassCommon)) @@ -430,7 +432,7 @@ bool SharedDatabase::GetSharedBank(uint32 id, Inventory* inv, bool is_charid) { int16 put_slot_id = INVALID_INDEX; ItemInst* inst = CreateBaseItem(item, charges); - if (item->ItemClass == ItemClassCommon) { + if (inst && item->ItemClass == ItemClassCommon) { for(int i = AUG_BEGIN; i < EmuConstants::ITEM_COMMON_SIZE; i++) { if (aug[i]) { inst->PutAugment(this, i, aug[i]); @@ -527,6 +529,9 @@ bool SharedDatabase::GetInventory(uint32 char_id, Inventory* inv) { ItemInst* inst = CreateBaseItem(item, charges); + if (inst == nullptr) + continue; + if(row[11]) { std::string data_str(row[11]); std::string idAsString; @@ -637,6 +642,10 @@ bool SharedDatabase::GetInventory(uint32 account_id, char* name, Inventory* inv) continue; ItemInst* inst = CreateBaseItem(item, charges); + + if (inst == nullptr) + continue; + inst->SetAttuned(instnodrop); if(row[11]) { @@ -1195,9 +1204,16 @@ ItemInst* SharedDatabase::CreateItem(uint32 item_id, int16 charges, uint32 aug1, { const Item_Struct* item = nullptr; ItemInst* inst = nullptr; + item = GetItem(item_id); if (item) { inst = CreateBaseItem(item, charges); + + if (inst == nullptr) { + LogFile->write(EQEMuLog::Error, "Error: valid item data returned a null reference for ItemInst creation in SharedDatabase::CreateItem()"); + return nullptr; + } + inst->PutAugment(this, 0, aug1); inst->PutAugment(this, 1, aug2); inst->PutAugment(this, 2, aug3); @@ -1217,6 +1233,12 @@ ItemInst* SharedDatabase::CreateItem(const Item_Struct* item, int16 charges, uin ItemInst* inst = nullptr; if (item) { inst = CreateBaseItem(item, charges); + + if (inst == nullptr) { + LogFile->write(EQEMuLog::Error, "Error: valid item data returned a null reference for ItemInst creation in SharedDatabase::CreateItem()"); + return nullptr; + } + inst->PutAugment(this, 0, aug1); inst->PutAugment(this, 1, aug2); inst->PutAugment(this, 2, aug3); @@ -1242,6 +1264,11 @@ ItemInst* SharedDatabase::CreateBaseItem(const Item_Struct* item, int16 charges) inst = new ItemInst(item, charges); + if (inst == nullptr) { + LogFile->write(EQEMuLog::Error, "Error: valid item data returned a null reference for ItemInst creation in SharedDatabase::CreateBaseItem()"); + return nullptr; + } + if(item->CharmFileID != 0 || (item->LoreGroup >= 1000 && item->LoreGroup != -1)) { inst->Initialize(this); } diff --git a/zone/attack.cpp b/zone/attack.cpp index 5f4835395..deb0daf61 100644 --- a/zone/attack.cpp +++ b/zone/attack.cpp @@ -3890,6 +3890,7 @@ float Mob::GetDefensiveProcChances(float &ProcBonus, float &ProcChance, uint16 h return ProcChance; } +// argument 'weapon' not used void Mob::TryDefensiveProc(const ItemInst* weapon, Mob *on, uint16 hand) { if (!on) { diff --git a/zone/client_packet.cpp b/zone/client_packet.cpp index cc767dc6a..a6830d74b 100644 --- a/zone/client_packet.cpp +++ b/zone/client_packet.cpp @@ -3087,7 +3087,7 @@ void Client::Handle_OP_AugmentItem(const EQApplicationPacket *app) // Adding augment if (in_augment->augment_action == 0) { - ItemInst *tobe_auged, *auged_with = nullptr; + ItemInst *tobe_auged = nullptr, *auged_with = nullptr; int8 slot = -1; Inventory& user_inv = GetInv(); @@ -3157,7 +3157,7 @@ void Client::Handle_OP_AugmentItem(const EQApplicationPacket *app) } else if (in_augment->augment_action == 1) { - ItemInst *tobe_auged, *auged_with = nullptr; + ItemInst *tobe_auged = nullptr, *auged_with = nullptr; int8 slot = -1; Inventory& user_inv = GetInv(); @@ -12330,22 +12330,30 @@ void Client::Handle_OP_ShopPlayerSell(const EQApplicationPacket *app) int freeslot = 0; if (charges > 0 && (freeslot = zone->SaveTempItem(vendor->CastToNPC()->MerchantType, vendor->GetNPCTypeID(), itemid, charges, true)) > 0){ ItemInst* inst2 = inst->Clone(); - if (RuleB(Merchant, UsePriceMod)){ - inst2->SetPrice(item->Price*(RuleR(Merchant, SellCostMod))*item->SellRate*Client::CalcPriceMod(vendor, false)); + + while (true) { + if (inst2 == nullptr) + break; + + if (RuleB(Merchant, UsePriceMod)){ + inst2->SetPrice(item->Price*(RuleR(Merchant, SellCostMod))*item->SellRate*Client::CalcPriceMod(vendor, false)); + } + else + inst2->SetPrice(item->Price*(RuleR(Merchant, SellCostMod))*item->SellRate); + inst2->SetMerchantSlot(freeslot); + + uint32 MerchantQuantity = zone->GetTempMerchantQuantity(vendor->GetNPCTypeID(), freeslot); + + if (inst2->IsStackable()) { + inst2->SetCharges(MerchantQuantity); + } + inst2->SetMerchantCount(MerchantQuantity); + + SendItemPacket(freeslot - 1, inst2, ItemPacketMerchant); + safe_delete(inst2); + + break; } - else - inst2->SetPrice(item->Price*(RuleR(Merchant, SellCostMod))*item->SellRate); - inst2->SetMerchantSlot(freeslot); - - uint32 MerchantQuantity = zone->GetTempMerchantQuantity(vendor->GetNPCTypeID(), freeslot); - - if (inst2->IsStackable()) { - inst2->SetCharges(MerchantQuantity); - } - inst2->SetMerchantCount(MerchantQuantity); - - SendItemPacket(freeslot - 1, inst2, ItemPacketMerchant); - safe_delete(inst2); } // start QS code diff --git a/zone/embparser.cpp b/zone/embparser.cpp index 3a27fc42f..bfac13660 100644 --- a/zone/embparser.cpp +++ b/zone/embparser.cpp @@ -232,6 +232,7 @@ int PerlembParser::EventGlobalPlayer(QuestEventID evt, Client *client, std::stri int PerlembParser::EventItem(QuestEventID evt, Client *client, ItemInst *item, Mob *mob, std::string data, uint32 extra_data, std::vector *extra_pointers) { + // needs pointer validation on 'item' argument return EventCommon(evt, item->GetID(), nullptr, nullptr, item, client, extra_data, false, extra_pointers); } @@ -335,6 +336,9 @@ bool PerlembParser::ItemHasQuestSub(ItemInst *itm, QuestEventID evt) { if(!perl) return false; + if (itm == nullptr) + return false; + if(evt >= _LargestEventID) return false; @@ -449,6 +453,9 @@ void PerlembParser::LoadGlobalPlayerScript(std::string filename) { } void PerlembParser::LoadItemScript(std::string filename, ItemInst *item) { + if (item == nullptr) + return; + std::stringstream package_name; package_name << "qst_item_" << item->GetID(); @@ -855,6 +862,7 @@ void PerlembParser::GetQuestPackageName(bool &isPlayerQuest, bool &isGlobalPlaye } } else if(isItemQuest) { + // need a valid ItemInst pointer check here..unsure how to cancel this process -U const Item_Struct* item = iteminst->GetItem(); package_name = "qst_item_"; package_name += itoa(item->ID); @@ -1292,6 +1300,7 @@ void PerlembParser::ExportEventVariables(std::string &package_name, QuestEventID case EVENT_SCALE_CALC: case EVENT_ITEM_ENTER_ZONE: { + // need a valid ItemInst pointer check here..unsure how to cancel this process -U ExportVar(package_name.c_str(), "itemid", objid); ExportVar(package_name.c_str(), "itemname", iteminst->GetItem()->Name); break; @@ -1299,6 +1308,7 @@ void PerlembParser::ExportEventVariables(std::string &package_name, QuestEventID case EVENT_ITEM_CLICK_CAST: case EVENT_ITEM_CLICK: { + // need a valid ItemInst pointer check here..unsure how to cancel this process -U ExportVar(package_name.c_str(), "itemid", objid); ExportVar(package_name.c_str(), "itemname", iteminst->GetItem()->Name); ExportVar(package_name.c_str(), "slotid", extradata); diff --git a/zone/forage.cpp b/zone/forage.cpp index 7e2f7d457..8476ceaad 100644 --- a/zone/forage.cpp +++ b/zone/forage.cpp @@ -249,7 +249,7 @@ void Client::GoFish() Bait = m_inv.GetItem(bslot); //if the bait isnt equipped, need to add its skill bonus - if(bslot >= EmuConstants::GENERAL_BEGIN && Bait->GetItem()->SkillModType == SkillFishing) { + if(bslot >= EmuConstants::GENERAL_BEGIN && Bait != nullptr && Bait->GetItem()->SkillModType == SkillFishing) { fishing_skill += Bait->GetItem()->SkillModValue; } diff --git a/zone/inventory.cpp b/zone/inventory.cpp index 3afb2806b..e6f4b00b6 100644 --- a/zone/inventory.cpp +++ b/zone/inventory.cpp @@ -2007,7 +2007,7 @@ void Client::DyeArmor(DyeStruct* dye){ bool Client::DecreaseByID(uint32 type, uint8 amt) { const Item_Struct* TempItem = 0; - ItemInst* ins; + ItemInst* ins = nullptr; int x; int num = 0; for(x = EmuConstants::EQUIPMENT_BEGIN; x <= EmuConstants::GENERAL_BAGS_END; x++) @@ -2143,6 +2143,7 @@ void Client::RemoveNoRent(bool client_update) { std::list::iterator iter = local.begin(); while (iter != local.end()) { inst = *iter; + // should probably put a check here for valid pointer..but, that was checked when the item was put into inventory -U if (!inst->GetItem()->NoRent) mlog(INVENTORY__SLOTS, "NoRent Timer Lapse: Deleting %s from `Limbo`", inst->GetItem()->Name); else @@ -2268,6 +2269,7 @@ void Client::RemoveDuplicateLore(bool client_update) { std::list::iterator iter = local.begin(); while (iter != local.end()) { inst = *iter; + // probably needs a valid pointer check -U if (CheckLoreConflict(inst->GetItem())) { mlog(INVENTORY__ERROR, "Lore Duplication Error: Deleting %s from `Limbo`", inst->GetItem()->Name); safe_delete(*iter); @@ -2470,8 +2472,8 @@ void Client::CreateBandolier(const EQApplicationPacket *app) { _log(INVENTORY__BANDOLIER, "Char: %s Creating Bandolier Set %i, Set Name: %s", GetName(), bs->number, bs->name); strcpy(m_pp.bandoliers[bs->number].name, bs->name); - const ItemInst* InvItem; - const Item_Struct *BaseItem; + const ItemInst* InvItem = nullptr; + const Item_Struct *BaseItem = nullptr; int16 WeaponSlot; for(int BandolierSlot = bandolierMainHand; BandolierSlot <= bandolierAmmo; BandolierSlot++) { diff --git a/zone/lua_parser.cpp b/zone/lua_parser.cpp index b05e7619f..1b1bfb8e8 100644 --- a/zone/lua_parser.cpp +++ b/zone/lua_parser.cpp @@ -695,6 +695,9 @@ bool LuaParser::SpellHasQuestSub(uint32 spell_id, QuestEventID evt) { } bool LuaParser::ItemHasQuestSub(ItemInst *itm, QuestEventID evt) { + if (itm == nullptr) { + return false; + } evt = ConvertLuaEvent(evt); if(evt >= _LargestEventID) { return false; @@ -738,6 +741,8 @@ void LuaParser::LoadGlobalPlayerScript(std::string filename) { } void LuaParser::LoadItemScript(std::string filename, ItemInst *item) { + if (item == nullptr) + return; std::string package_name = "item_"; package_name += std::to_string(static_cast(item->GetID())); diff --git a/zone/quest_parser_collection.cpp b/zone/quest_parser_collection.cpp index c0680b43f..d3bde0d5b 100644 --- a/zone/quest_parser_collection.cpp +++ b/zone/quest_parser_collection.cpp @@ -200,6 +200,9 @@ bool QuestParserCollection::SpellHasQuestSub(uint32 spell_id, QuestEventID evt) } bool QuestParserCollection::ItemHasQuestSub(ItemInst *itm, QuestEventID evt) { + if (itm == nullptr) + return false; + std::string item_script; if(itm->GetItem()->ScriptFileID != 0) { item_script = "script_"; @@ -350,6 +353,8 @@ int QuestParserCollection::EventPlayerGlobal(QuestEventID evt, Client *client, s int QuestParserCollection::EventItem(QuestEventID evt, Client *client, ItemInst *item, Mob *mob, std::string data, uint32 extra_data, std::vector *extra_pointers) { + // needs pointer validation check on 'item' argument + std::string item_script; if(item->GetItem()->ScriptFileID != 0) { item_script = "script_"; diff --git a/zone/questmgr.cpp b/zone/questmgr.cpp index ab68f8361..eb901d92d 100644 --- a/zone/questmgr.cpp +++ b/zone/questmgr.cpp @@ -1229,6 +1229,8 @@ void QuestManager::itemlink(int item_id) { if (initiator) { const ItemInst* inst = database.CreateItem(item_id); char* link = 0; + if (inst == nullptr) + return; if (initiator->MakeItemLink(link, inst)) initiator->Message(0, "%s tells you, %c%s%s%c", owner->GetCleanName(), 0x12, link, inst->GetItem()->Name, 0x12); @@ -2336,7 +2338,7 @@ int QuestManager::collectitems_processSlot(int16 slot_id, uint32 item_id, bool remove) { QuestManagerCurrentQuestVars(); - ItemInst *item; + ItemInst *item = nullptr; int quantity = 0; item = initiator->GetInv().GetItem(slot_id); diff --git a/zone/spells.cpp b/zone/spells.cpp index a7ee31f2c..02e99490d 100644 --- a/zone/spells.cpp +++ b/zone/spells.cpp @@ -1195,22 +1195,29 @@ void Mob::CastedSpellFinished(uint16 spell_id, uint32 target_id, uint16 slot, uint32 recastdelay = 0; uint32 recasttype = 0; - for (int r = 0; r < EmuConstants::ITEM_COMMON_SIZE; r++) { - const ItemInst* aug_i = inst->GetAugment(r); - - if(!aug_i) - continue; - const Item_Struct* aug = aug_i->GetItem(); - if(!aug) - continue; - - if ( aug->Click.Effect == spell_id ) - { - recastdelay = aug_i->GetItem()->RecastDelay; - recasttype = aug_i->GetItem()->RecastType; - fromaug = true; + while (true) { + if (inst == nullptr) break; + + for (int r = AUG_BEGIN; r < EmuConstants::ITEM_COMMON_SIZE; r++) { + const ItemInst* aug_i = inst->GetAugment(r); + + if (!aug_i) + continue; + const Item_Struct* aug = aug_i->GetItem(); + if (!aug) + continue; + + if (aug->Click.Effect == spell_id) + { + recastdelay = aug_i->GetItem()->RecastDelay; + recasttype = aug_i->GetItem()->RecastType; + fromaug = true; + break; + } } + + break; } //Test the aug recast delay diff --git a/zone/tradeskills.cpp b/zone/tradeskills.cpp index f6112b560..453a8474b 100644 --- a/zone/tradeskills.cpp +++ b/zone/tradeskills.cpp @@ -94,7 +94,7 @@ void Object::HandleAugmentation(Client* user, const AugmentItem_Struct* in_augme return; } - ItemInst *tobe_auged, *auged_with = nullptr; + ItemInst *tobe_auged = nullptr, *auged_with = nullptr; int8 slot=-1; // Verify 2 items in the augmentation device @@ -1185,6 +1185,8 @@ void Client::CheckIncreaseTradeskill(int16 bonusstat, int16 stat_modifier, float bool ZoneDatabase::GetTradeRecipe(const ItemInst* container, uint8 c_type, uint32 some_id, uint32 char_id, DBTradeskillRecipe_Struct *spec) { + if (container == nullptr) + return false; std::string containers;// make where clause segment for container(s) if (some_id == 0) diff --git a/zone/trading.cpp b/zone/trading.cpp index b9b582bfe..7d66adb30 100644 --- a/zone/trading.cpp +++ b/zone/trading.cpp @@ -159,6 +159,9 @@ Mob* Trade::With() // Private Method: Send item data for trade item to other person involved in trade void Trade::SendItemData(const ItemInst* inst, int16 dest_slot_id) { + if (inst == nullptr) + return; + // @merth: This needs to be redone with new item classes Mob* mob = With(); if (!mob->IsClient())