[fix] Vellums

[Thyros]

Lightguard, can you please post the complete patch?

[Lightguard]

No, as i said i can't update it yet, i cannot compile mangos now.

[KAPATEJIb]

it's correct?

            case SPELL_EFFECT_ENCHANT_ITEM:
           {
               if(Item* targetItem = m_targets.getItemTarget())
               {
                   if(targetItem->GetProto()->Class != m_spellInfo->EquippedItemClass ||
                      ((1<<targetItem->GetProto()->SubClass) & m_spellInfo->EquippedItemSubClassMask)==0)
                      return SPELL_FAILED_BAD_TARGETS;
               }
               if(m_spellInfo->EffectItemType[i] && m_targets.getItemTarget())
               {
                   if(m_targets.getItemTarget()->IsWeaponVellum() || m_targets.getItemTarget()->IsArmorVellum())
                   {
                        ItemPosCountVec dest;
                        uint8 msg = ((Player*)m_caster)->CanStoreNewItem( NULL_BAG, NULL_SLOT, dest, m_spellInfo->EffectItemType[i], 1 );
                        if(msg != EQUIP_ERR_OK)
                        {
                           ((Player*)m_caster)->SendEquipError( msg, NULL, NULL );
                           return SPELL_FAILED_DONT_REPORT;
                        }
                   }
               }

[pasdVn]

Thanks for your work guys ;-)

Just -out of interest- one question to the exploit: Can anybody try to explain again, how this exploit is working? All cast checks are done by the server and are not influenced by any packages, send by the client, or am I wrong? And isn't the check in the suggested fix already done in another place in the code (the generic item check in CheckItem() ).

I'd like to inform myself about the that I don't understand Russian :-(.

[goodbadguy]

Still not consuming the vellums, but otherwise awesome work. Thanks!

[Wowka321]

Thanks for your work guys ;-)

Just -out of interest- one question to the exploit: Can anybody try to explain again, how this exploit is working? All cast checks are done by the server and are not influenced by any packages, send by the client, or am I wrong? And isn't the check in the suggested fix already done in another place in the code (the generic item check in CheckItem() ).

I'd like to inform myself about the that I don't understand Russian :-(.

U can use Vellum on incorrect items (like tabard, etc) with some cheating programs.

[pasdVn]

Yes, this is clear, of course^^

My question is how this is working in the code. Why should this fix change something?

[Lynx3d]

Looking at the proposed check i'd say that normally the client prevents you from applying the scroll to an unsuitable item class, but if you use cheats you can tell the server to apply it to any kind of equipment, so it needs to check if the client is actually giving a valid target item.

However, wouldn't this already have affected all kind of enchanting items like spell thread, armor kits, arcanums and whatnot? Unofrtunately, i can't read that russian bug report...

[Wowka321]

If items affected before patch - enchant still here.

Patch preventing to aplly new enchant.

[Wowka321]

But now u can't enchant rings with "classic" enchant

[KAPATEJIb]

But now u can't enchant rings with "classic" enchant

Confirmed, with cheating fix you can't enchant rings.

[biggui]

i would be much interested to see where you guys are at concerning this patch.

thats is the only real thing that doenst work and annoys me in MaNGOS right now.

would also like to make sure it gets included in the next MaNGOS master update.

thanks for your work guys and girls !

[Davletuner]

Some News ?

[Lightguard]

Well, i updated and corrected the patch as much as i could for those who need it. It should also solve the cheating part (tried to cheat with it and i couldn't). The cheat fix part can be easilly separated if you want only that.

rev. 8252

diff --git a/src/game/Item.cpp b/src/game/Item.cpp
index 0ac47c6..7ac271f 100644
--- a/src/game/Item.cpp
+++ b/src/game/Item.cpp
@@ -751,6 +751,16 @@ bool Item::IsFitToSpellRequirements(SpellEntry const* spellInfo) const
{
    ItemPrototype const* proto = GetProto();

+    // Enchant spells have only effect[0]
+    if(proto->IsVellum() && spellInfo->Effect[0] == SPELL_EFFECT_ENCHANT_ITEM && spellInfo->EffectItemType[0])
+    {
+        if(proto->SubClass == ITEM_SUBCLASS_WEAPON_ENCHANTMENT && spellInfo->EquippedItemClass == ITEM_CLASS_WEAPON)
+            return true;
+        else if(proto->SubClass == ITEM_SUBCLASS_ARMOR_ENCHANTMENT && spellInfo->EquippedItemClass == ITEM_CLASS_ARMOR)
+            return true;
+    }
+    // Vellum enchant case should ignore everything below
+
    if (spellInfo->EquippedItemClass != -1)                 // -1 == any item class
    {
        if(spellInfo->EquippedItemClass != int32(proto->Class))
@@ -1013,4 +1023,4 @@ bool ItemRequiredTarget::IsFitToRequirements( Unit* pUnitTarget ) const
        default:
            return false;
    }
-}
\\ No newline at end of file
+}
diff --git a/src/game/ItemPrototype.h b/src/game/ItemPrototype.h
index 9974030..f9fb7da 100644
--- a/src/game/ItemPrototype.h
+++ b/src/game/ItemPrototype.h
@@ -115,6 +115,7 @@ enum ITEM_FLAGS
    ITEM_FLAGS_THROWABLE                      = 0x00400000, // not used in game for check trow possibility, only for item in game tooltip
    ITEM_FLAGS_SPECIALUSE                     = 0x00800000, // last used flag in 2.3.0
    ITEM_FLAGS_BOA                            = 0x08000000, // bind on account (set in template for items that can binded in like way)
+    ITEM_FLAGS_SCROLL_ENCHANT                 = 0x10000000,
    ITEM_FLAGS_MILLABLE                       = 0x20000000
};

@@ -622,6 +623,7 @@ struct ItemPrototype

    bool IsPotion() const { return Class==ITEM_CLASS_CONSUMABLE && SubClass==ITEM_SUBCLASS_POTION; }
    bool IsConjuredConsumable() const { return Class == ITEM_CLASS_CONSUMABLE && (Flags & ITEM_FLAGS_CONJURED); }
+    bool IsVellum() const { return Class == ITEM_CLASS_TRADE_GOODS && (0xC000 & (1<<SubClass)); }
};

struct ItemLocale
diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
index b8c0f80..c0f0019 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -3521,6 +3521,10 @@ void Spell::TakeReagents()
    if (p_caster->CanNoReagentCast(m_spellInfo))
        return;

+    // Vellum case, m_CastItem is consumable -> removed on use, vellum is deleted in DoCreateItem
+    if(m_CastItem && m_CastItem->GetProto()->Flags & ITEM_FLAGS_SCROLL_ENCHANT)
+        return;
+
    for(uint32 x = 0; x < 8; ++x)
    {
        if(m_spellInfo->Reagent[x] <= 0)
@@ -4405,6 +4409,32 @@ SpellCastResult Spell::CheckCast(bool strict)
                    return SPELL_FAILED_BAD_TARGETS;
                break;
            }
+            case SPELL_EFFECT_ENCHANT_ITEM:
+            {
+                if(Item* Target = m_targets.getItemTarget())
+                {
+                    // Check cheating case
+                    if(!Target->IsFitToSpellRequirements(m_spellInfo))
+                        return SPELL_FAILED_BAD_TARGETS;
+
+                    // Do not enchant vellum with scroll
+                    if(m_CastItem && m_CastItem->GetProto()->Flags & ITEM_FLAGS_SCROLL_ENCHANT && Target->GetProto()->IsVellum())
+                        return SPELL_FAILED_BAD_TARGETS;
+
+                    // Check if we can store a new scroll
+                    if(Target->GetProto()->IsVellum() && m_spellInfo->EffectItemType[i])
+                    {
+                         ItemPosCountVec dest;
+                         uint8 msg = ((Player*)m_caster)->CanStoreNewItem( NULL_BAG, NULL_SLOT, dest, m_spellInfo->EffectItemType[i], 1 );
+                         if(msg != EQUIP_ERR_OK)
+                         {
+                            ((Player*)m_caster)->SendEquipError( msg, NULL, NULL );
+                            return SPELL_FAILED_DONT_REPORT;
+                         }
+                    }
+                }
+                break;
+            }
            default:break;
        }
    }
@@ -5044,12 +5074,17 @@ SpellCastResult Spell::CheckItems()
            uint32 itemid    = m_spellInfo->Reagent[i];
            uint32 itemcount = m_spellInfo->ReagentCount[i];

+            // If item is a scroll enchant we don't need reagents
+            if(m_CastItem && m_CastItem->GetProto()->Flags & ITEM_FLAGS_SCROLL_ENCHANT)
+                break;
+
            // if CastItem is also spell reagent
            if( m_CastItem && m_CastItem->GetEntry() == itemid )
            {
                ItemPrototype const *proto = m_CastItem->GetProto();
                if(!proto)
                    return SPELL_FAILED_ITEM_NOT_READY;
+
                for(int s = 0; s < MAX_ITEM_PROTO_SPELLS; ++s)
                {
                    // CastItem will be used up and does not count as reagent
@@ -5087,6 +5122,13 @@ SpellCastResult Spell::CheckItems()
    uint32 TotemCategory = 2;
    for(int i= 0; i < 2; ++i)
    {
+        // Skip for scroll enchant case
+        if(m_CastItem && m_CastItem->GetProto()->Flags & ITEM_FLAGS_SCROLL_ENCHANT)
+        {
+            TotemCategory = 0;
+            break;
+        }
+
        if(m_spellInfo->TotemCategory[i] != 0)
        {
            if( p_caster->HasItemTotemCategory(m_spellInfo->TotemCategory[i]) )
@@ -5839,4 +5881,4 @@ void Spell::FillRaidOrPartyHealthPriorityTargets( UnitList &TagUnitMap, Unit* me
        TagUnitMap.push_back(healthQueue.top().getUnit());
        healthQueue.pop();
    }
-}
\\ No newline at end of file
+}
diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index 7a6ed0c..e60573e 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -2749,7 +2749,11 @@ void Spell::DoCreateItem(uint32 i, uint32 itemtype)

        // send info to the client
        if(pItem)
+        {
+            if(Item* ItemTarget = m_targets.getItemTarget())
+                player->DestroyItemCount(ItemTarget->GetEntry(), 1, true);
            player->SendNewItem(pItem, num_to_add, true, true);
+        }

        // we succeeded in creating at least one item, so a levelup is possible
        player->UpdateCraftSkill(m_spellInfo->Id);
@@ -3902,6 +3906,14 @@ void Spell::EffectEnchantItemPerm(uint32 effect_idx)
    if(!item_owner)
        return;

+    ItemPrototype const* targetProto = itemTarget->GetProto();
+    if(m_spellInfo->EffectItemType[effect_idx] && targetProto->IsVellum())
+    {
+        unitTarget = m_caster;
+        DoCreateItem(effect_idx,m_spellInfo->EffectItemType[effect_idx]);
+        return;
+    }
+
    if(item_owner!=p_caster && p_caster->GetSession()->GetSecurity() > SEC_PLAYER && sWorld.getConfig(CONFIG_GM_LOG_TRADE) )
    {
        sLog.outCommand(p_caster->GetSession()->GetAccountId(),"GM %s (Account: %u) enchanting(perm): %s (Entry: %d) for player: %s (Account: %u)",

Patchfile

[goodbadguy]

Thanks for this!

The real link location is here:

PatchFile

[KAPATEJIb]

http://filebeam.com/d332eaac45b0fd52a1cae63b5f50d4f4.jpg but i used too many patches and line number can be different. I can post a code of that functions if you want. Crash happens on

            pItem->SendUpdateToPlayer( this );

[Lightguard]

Well it is really some sort of specific case i think next time could you please print spellId, player, m_CastItem, pItem in the next function and *this everywhere? Sorry for asking this much but it would be really useful.

[Lynx3d]

I just gave the patch a try. It bitched a bit at applying with missing newlines at end of file for spell.cpp and item.cpp but those can be ignored...

I enchanted some armor and a weapon vellums, reagents were taken correctly, scrolls generated correctly, and scrolls aswell as enchants were still intact after relog. Other enchants (like rings) also still seem to work just fine.

That was hardly an intensive test, but so far everything looks fine over here.

[muelli]

works for me too! vellums get used up, etc

[turk3y]

Works great! Thanks!

[bulek]

very nice patch, works fine thank

[Espec]

Thanks! :cool:

[goodbadguy]

Works except I get crashes here:

+ player->DestroyItemCount(itemTarget->GetEntry(), 1, true);

Fixed it by removing:

+ if(Item* ItemTarget = m_targets.getItemTarget())

+ player->DestroyItemCount(itemTarget->GetEntry(), 1, true);

This (again) makes it so Vellums aren't getting used up when making scrolls, however prevents the crash which seems to be caused by something other than vellums.

[Hummer]

Works except I get crashes here:

+ player->DestroyItemCount(itemTarget->GetEntry(), 1, true);

Fixed it by removing:

+ if(Item* ItemTarget = m_targets.getItemTarget())

+ player->DestroyItemCount(itemTarget->GetEntry(), 1, true);

This (again) makes it so Vellums aren't getting used up when making scrolls, however prevents the crash which seems to be caused by something other than vellums.

can lay out the fix in the form of a patch ready?

P.S and as I scroll pognyal themselves will not be removed after applying this patch?

[timmons]

Works except I get crashes here:

+ player->DestroyItemCount(itemTarget->GetEntry(), 1, true);

Fixed it by removing:

+ if(Item* ItemTarget = m_targets.getItemTarget())

+ player->DestroyItemCount(itemTarget->GetEntry(), 1, true);

This (again) makes it so Vellums aren't getting used up when making scrolls, however prevents the crash which seems to be caused by something other than vellums.

Could perhaps the issues be caused by the typo (*I*temTarget in the if, and *i*temTarget in the destroy)?

I'm patching by hand so I don't know if the typo is present in the downloaded patch as well as the quote block in this thread, but that would be kind of... bad.

EDIT: Finally got the patch from the download link (you know you're having some serious Internet connection issues when you have trouble downloading a text file) and confirmed that yes, the typo is present there as well.