[patch] small cleanup and fixing some old bugs..

[zhenya``]

1) fixing dupe and apply enchantment while disarm state

http://paste2.org/p/609895

2) move GroupSet to GroupMap ( nty ) and some replacements .count to .find for better performance

http://paste2.org/p/609894

* Who has been writing this patch? Please include either forum user names or email addresses.

zhenya

[zhenya``]

also i think need to check m_lootGenerated in bool Item::CanBeTraded(bool mail)

and in WorldSession::HandleSellItemOpcode ( 2 variants are possible after sell item doesn`t go to buyback slot or sell isn`t possible? )

and i worked with items which have duration. there is no checks at all (sell items, trade,auction) l.. i think its a problem..

[VladimirMangos]

Duration items possible autobound all. If functionality base at this then need check this at loading.

Player::_CanTakeMoreSimilarItems

By function emaning this too hacky way place like сheck to it.

Also this not catch all cases... with loot state merge. Spcially when looting item auto slected as merge slot.

I will add rewrited patch for this part.

CanUseAttackType

This function is not exist in mangos.

also i think need to check m_lootGenerated in bool Item::CanBeTraded(bool mail)

Agree. This is make safe many places with possible absent or not so clear check inclding guild bank and etc.

[VladimirMangos]

1) fixing dupe and apply enchantment while disarm state

http://paste2.org/p/609895

First part of first patch added in rewrited form in [9227]. Thank you. I think now we check all cases looting state for source and targeted items.

[VladimirMangos]

2) move GroupSet to GroupMap ( nty ) and some replacements .count to .find for better performance

http://paste2.org/p/609894

I think all ok, except use uint64 as key foru group map.

No reason use full guid as key for map because uint64 key _lot_ more slow as key at x86

Alsoreplace mGroupMap.erase(itr++) by relookup removed element by guid will more slow that existed case, so i will not include this change also.

[VladimirMangos]

Second patch added in [9228] with described excluded/modifed parts. Thank you.

So from patches not added only second part of first patch. In suggested form it not compatibel with mangos sources.

[zhenya``]

apply enchantment while disarm state

because disarm may be not only main hand (UNIT_FLAG_DISARMED)

this is function.

       bool CanUseAttackType( uint8 attacktype ) const
       {
           switch(attacktype)
           {
               case BASE_ATTACK: return !HasFlag(UNIT_FIELD_FLAGS,UNIT_FLAG_DISARMED);
               case OFF_ATTACK: return !HasFlag(UNIT_FIELD_FLAGS_2,UNIT_FLAG2_DISARM_OFFHAND);
               case RANGED_ATTACK: return !HasFlag(UNIT_FIELD_FLAGS_2,UNIT_FLAG2_DISARM_RANGED);
           }
           return true;
       }

btw. you can rewrite spelleffects with disarm effect.. in more generic way.

GetWeaponForAttack not exclude players with disarm flag... missing check may be...

[zhenya``]

btw i rewrite some inventory operations my question:

function:

void Player::_LoadInventory(QueryResult *result, uint32 timediff)

if item in INVENTORY_SLOT_BAG_0 all okey in item = StoreItem(dest, item, true); i have this != NULL

but in others bag slots or bags ( QuickEquipItem(dest, item); and BankItem(dest, item, true); and itr->second->StoreItem(slot, item, true); ) i have Player NULL :-(

how to avoid it ? i need non NULL player..

[zhenya``]

so i will post my work on inventory later...

now some about TRADE

example:

1) 1 player place all items in free bag which it want to trade and 1 player must have 1 free bag

2) 1 player init trade with 2 player

3) 1 player place all items from bag , 2 player place money

4) 1 player swap bags .. as items it trade save position..

   _player->tradeItems[tradeSlot] = pos;

if(Item* item  =_player->GetItemByPos( _player->tradeItems[i] ))

not passing..

   for(int i=0; i<TRADE_SLOT_TRADED_COUNT; ++i)
   {
       if(_player->tradeItems[i] != NULL_SLOT )
       {
           if(Item* item  =_player->GetItemByPos( _player->tradeItems[i] ))
           {
               if(!item->CanBeTraded())
               {
                   SendTradeStatus(TRADE_STATUS_TRADE_CANCELED);
                   return;
               }
           }
       }
       if(_player->pTrader->tradeItems[i] != NULL_SLOT)
       {
           if(Item* item  =_player->pTrader->GetItemByPos( _player->pTrader->tradeItems[i]) )
           {
               if(!item->CanBeTraded())
               {
                   SendTradeStatus(TRADE_STATUS_TRADE_CANCELED);
                   return;
               }
           }
       }
   }

possible fix.. not allow swap items which containes trade items..

or send cancel trade.. if item is not found

2 possible fix

   for (int i=0; i<TRADE_SLOT_TRADED_COUNT; ++i)
   {
       if(_player->tradeItems[i] != NULL_SLOT )
       {
           Item* item  =_player->GetItemByPos( _player->tradeItems[i] );
           if(!item || !item->CanBeTraded())
           {
               SendTradeStatus(TRADE_STATUS_TRADE_CANCELED);
               return;
           }
       }
       if(_player->pTrader->tradeItems[i] != NULL_SLOT)
       {
           Item* item  =_player->pTrader->GetItemByPos( _player->pTrader->tradeItems[i]);
           if(!item || !item->CanBeTraded())
           {
               SendTradeStatus(TRADE_STATUS_TRADE_CANCELED);
               return;
           }
       }
   }

5. as a result 1 player get money 2 player get nothing.

[VladimirMangos]

1) we still not restored from adding old part... so i not plan add more form this thread until prev. resolving..

2) please mot make from threads list of patches. 1 thread for 1 patch. I close thread for avoid continue this