Jump to content

[fix] Vellums


Auntie Mangos

Recommended Posts

  • Replies 125
  • Created
  • Last Reply

Top Posters In This Topic

OK, I've been working on this for a few hours now and I'm going to have to bite the bullet and admit that I cannot figure out why these changes aren't taking effect in my client. I've cleared the cache every time and gone over every changed line with a fine-tooth comb, but this is practically my first excursion into the bowels of the backend (no pun intended), so I'd like to know if anyone else has had any luck implementing the patch at this point.

To provide a bit more detail, I am receiving the same error message as in the unpatched version ("Must have a Miscellaneous equipped") with the basic test of applying "Enchant Bracer - Minor Health" to an Armor Vellum (basic version) using GM-summoned items and a GM-`.learn`ed Enchanting skill at 14.

Before applying the patch, I fully updated my repositories to the latest MaNGOS master branch, SD2 trunk, and applied the latest UDB patch/hotfix from their forums (my goal of actually playing the game yet today is looking quite farfetched), so any of those could have interfered if quite-recent changes altered related bits of the beast, entropy being what it is, so I suppose I'll be hashing those bits out for a few hours, now.... yippee.

EDIT: OK, scratch that; I just reverted my Vellums branch and applied the patch (minus the missing newlines and lowercase 'i') with git-apply and now it seems to be working beautifully. I must have made some typos of my own when I applied by hand. Thank you for the patch, Lightguard, but now velociraptors will most certainly smell my weakness, so I suppose it is something of a mixed blessing.

Link to comment
Share on other sites

Just noticed a new issue: enchanting a vellum appears to give 2 skill points (at orange difficulty, at least) when the same enchant used on a normal piece of armour/weapon naturally gives only 1.

I'll try to look into it, but I'm not sure how much help I'll be.

EDIT: I left out some important info - I have my server set up to use the normal 1x craft skill gain rate, and only one skillup message appears per vellum, reporting a 2-point gain.

Link to comment
Share on other sites

Alright, after an extensive montage ending in me running up a bunch of steps and throwing my hands in the air, I appear to have fixed it.

diff --git a/src/game/Item.cpp b/src/game/Item.cpp
index eb755b0..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))
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 88e4bc3..b6c6133 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -3541,6 +3541,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)
@@ -4428,6 +4432,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;
        }
    }
@@ -5067,12 +5097,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
@@ -5110,6 +5145,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]) )
diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index e177536..26ff02f 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -2753,7 +2753,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);
@@ -3883,11 +3887,6 @@ void Spell::EffectEnchantItemPerm(uint32 effect_idx)
    if (!itemTarget)
        return;

-    Player* p_caster = (Player*)m_caster;
-
-    // not grow at item use at item case
-    p_caster->UpdateCraftSkill(m_spellInfo->Id);
-
    uint32 enchant_id = m_spellInfo->EffectMiscValue[effect_idx];
    if (!enchant_id)
        return;
@@ -3901,6 +3900,29 @@ void Spell::EffectEnchantItemPerm(uint32 effect_idx)
    if(!item_owner)
        return;

+    Player* p_caster = (Player*)m_caster;
+
+    // Enchanting a vellum requires special handling, as it creates a new item
+    // instead of modifying an existing one.
+    ItemPrototype const* targetProto = itemTarget->GetProto();
+    if(targetProto->IsVellum())
+    {
+        // Do not allow a vellum to be enchanted through the Do Not Trade slot 
+        // to prevent the various bugs associated with creating items in that slot.
+        if(item_owner != p_caster)
+            return;
+
+        if(m_spellInfo->EffectItemType[effect_idx])
+        { 
+            unitTarget = m_caster;
+            DoCreateItem(effect_idx,m_spellInfo->EffectItemType[effect_idx]);
+            return;
+        }
+    }
+
+    // not grow at item use at item case
+    p_caster->UpdateCraftSkill(m_spellInfo->Id);
+
    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)",

Download vellums_normal_skillup.patch

Note: the patch is cumulative - it includes Lightguard's latest version as well as the minor tweak to fix double enchant skillups.

Also note that as a side effect, you should no longer be able to enchant a vellum that is in another player's No Trade Slot, as there does not appear to have been a resolution of this bug. I am not aware if this is a client issue, or whether MaNGOS is prone to the same disasterous shenanigans, but it was simplest to just turn it off. In any case, lines 161-165 just need to be removed in order to re-enable it and unleash unspeakable horrors back into downtown New York City.

Other than installing the ECTO Containment Unit, the skillup fix just moves stuff around.

Link to comment
Share on other sites

  • 1 month later...

Does anyone have a fixed patch for Vellums?

I got this rejection but I don't know exactly what to change...Can someone help me?

Thanx anyway!

***************
*** 2753,2759 ****

         // send info to the client
         if(pItem)
             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);
--- 2753,2763 ----

         // 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);

edit: I'm sorry I forgot to post some more information...It is the patch which was posted here few posts up and it's spelleffect.cpp which gave this rejection on mangos revision 8541.

edit2: It seems that the whole patch while compiling gives a lot of errors...anyone has a patch which works?

Thanx!

Link to comment
Share on other sites

  • 2 weeks later...

Any updates? I get this error when compile

9>SpellEffects.cpp
9>..\\..\\src\\game\\SpellEffects.cpp(4316) : error C2065: 'effect_idx' : undeclared identifier
9>..\\..\\src\\game\\SpellEffects.cpp(4319) : error C2065: 'effect_idx' : undeclared identifier
9>..\\..\\src\\game\\SpellEffects.cpp(4319) : error C2065: 'effect_idx' : undeclared identifier

part of code

    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;
   }

Link to comment
Share on other sites

Is there any chanse to fix this in official git?

MaNGOS/0.14.0 (* * Revision 8585 - *) for Linux_x64 (little-endian)
Using script library: ScriptDev2 (for MaNGOS 8555+) Revision [1440] 2009-10-04 13:31:04 (Unix)
Using World DB: PSDB WotLK (252)
Using creature EventAI: PSDB EventAI & ACID 3.0.0
VMap support included. LineOfSight:1, getHeight:1
VMap config keys are: vmap.enableLOS, vmap.enableHeight, vmap.ignoreMapIds, vmap.ignoreSpellIds

Link to comment
Share on other sites

  • 3 weeks later...
  • 2 weeks later...

Hm originally i wanted to tweak the patch a bit, but it pretty much ended up in a rewrite.

The stuff in CheckCast() should be in CheckItems() in my opinion, at least there is already similar checks, and the IsFitToSpellRequirements() call seems completely redundant to me, it is already called in CheckItems().

Also, the way the vellum is taken as reagent looked strange to me, i'm not sure if you didn't make a too many assumptions here. I only delete item when really enchanting a vellum.

Finally, i noticed that i got skillups in enchanting when using a scroll, so i added a check there.

I think that's about it, if anyone has an idea what you still could improve, let me know, but this is the first time i have the feeling i wrote a hack-free patch :D

diff --git a/src/game/Item.cpp b/src/game/Item.cpp
index 00bbe88..60180e4 100644
--- a/src/game/Item.cpp
+++ b/src/game/Item.cpp
@@ -751,6 +751,15 @@ 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) ||
+            (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))
diff --git a/src/game/ItemPrototype.h b/src/game/ItemPrototype.h
index cae4fd3..70fcfb6 100644
--- a/src/game/ItemPrototype.h
+++ b/src/game/ItemPrototype.h
@@ -632,6 +632,10 @@ 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 && (1 << ITEM_SUBCLASS_ARMOR_ENCHANTMENT | 1 << ITEM_SUBCLASS_WEAPON_ENCHANTMENT) & (1<<SubClass));
+    }
};

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

+    // Scroll case, reagents already taken on scroll creation; scoll itself destroyed in TakeCastItem()
+    if(m_CastItem && m_CastItem->GetProto()->Flags & ITEM_FLAGS_ENCHANT_SCROLL)
+        return;
+
    for(uint32 x = 0; x < 8; ++x)
    {
        if(m_spellInfo->Reagent[x] <= 0)
@@ -5195,6 +5199,8 @@ SpellCastResult Spell::CheckItems()
        return SPELL_CAST_OK;

    Player* p_caster = (Player*)m_caster;
+    bool isScrollItem = false;
+    bool isVellumTarget = false;

    // cast item checks
    if(m_CastItem)
@@ -5207,6 +5213,8 @@ SpellCastResult Spell::CheckItems()
        if(!proto)
            return SPELL_FAILED_ITEM_NOT_READY;

+        if(proto->Flags & ITEM_FLAGS_ENCHANT_SCROLL) isScrollItem = true;
+
        for (int i = 0; i < 5; ++i)
            if (proto->Spells[i].SpellCharges)
                if(m_CastItem->GetSpellCharges(i) == 0)
@@ -5273,8 +5281,13 @@ SpellCastResult Spell::CheckItems()
        if(!m_targets.getItemTarget())
            return SPELL_FAILED_ITEM_GONE;

+        isVellumTarget = m_targets.getItemTarget()->GetProto()->IsVellum();
        if(!m_targets.getItemTarget()->IsFitToSpellRequirements(m_spellInfo))
            return SPELL_FAILED_EQUIPPED_ITEM_CLASS;
+    
+            // Do not enchant vellum with scroll
+        if(isVellumTarget && isScrollItem)
+            return SPELL_FAILED_BAD_TARGETS;
    }
    // if not item target then required item must be equipped
    else
@@ -5305,8 +5318,9 @@ SpellCastResult Spell::CheckItems()
        focusObject = ok;                                   // game object found in range
    }

-    // check reagents (ignore triggered spells with reagents processed by original spell) and special reagent ignore case.
-    if (!m_IsTriggeredSpell && !p_caster->CanNoReagentCast(m_spellInfo))
+    // check reagents (ignore triggered spells with reagents processed by original spell) and special reagent ignore case,
+    // including enchanted scrolls
+    if (!m_IsTriggeredSpell && !p_caster->CanNoReagentCast(m_spellInfo) && !isScrollItem)
    {
        for(uint32 i = 0; i < 8; ++i)
        {
@@ -5340,17 +5354,22 @@ SpellCastResult Spell::CheckItems()

    // check totem-item requirements (items presence in inventory)
    uint32 totems = 2;
-    for(int i = 0; i < 2 ; ++i)
+    // scrolls don't need tools to apply enchant, unlike normal enchant spell that is also used to create scrolls
+    if(isScrollItem) totems = 0;
+    else
    {
-        if(m_spellInfo->Totem[i] != 0)
+        for(int i = 0; i < 2 ; ++i)
        {
-            if( p_caster->HasItemCount(m_spellInfo->Totem[i], 1) )
+            if(m_spellInfo->Totem[i] != 0)
            {
-                totems -= 1;
-                continue;
-            }
-        }else
-        totems -= 1;
+                if( p_caster->HasItemCount(m_spellInfo->Totem[i], 1) )
+                {
+                    totems -= 1;
+                    continue;
+                }
+            }else
+            totems -= 1;
+        }
    }
    if(totems != 0)
        return SPELL_FAILED_TOTEMS;                         //0x7C
@@ -5401,6 +5420,17 @@ SpellCastResult Spell::CheckItems()

                if( targetItem->GetProto()->ItemLevel < m_spellInfo->baseLevel )
                    return SPELL_FAILED_LOWLEVEL;
+                // Check if we can store a new scroll, enchanting vellum has implicit SPELL_EFFECT_CREATE_ITEM
+                if(isVellumTarget && m_spellInfo->EffectItemType[i])
+                {
+                    ItemPosCountVec dest;
+                    uint8 msg = p_caster->CanStoreNewItem( NULL_BAG, NULL_SLOT, dest, m_spellInfo->EffectItemType[i], 1 );
+                    if(msg != EQUIP_ERR_OK)
+                    {
+                        p_caster->SendEquipError( msg, NULL, NULL );
+                        return SPELL_FAILED_DONT_REPORT;
+                    }
+                }
                // Not allow enchant in trade slot for some enchant type
                if( targetItem->GetOwner() != m_caster )
                {
@@ -5410,6 +5440,9 @@ SpellCastResult Spell::CheckItems()
                        return SPELL_FAILED_ERROR;
                    if (pEnchant->slot & ENCHANTMENT_CAN_SOULBOUND)
                        return SPELL_FAILED_NOT_TRADEABLE;
+                    // cannot replace vellum with scroll in trade slot
+                    if (isVellumTarget)
+                        return SPELL_FAILED_BAD_TARGETS;
                }
                break;
            }
diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index 16170da..4a173a6 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -3945,11 +3945,6 @@ void Spell::EffectEnchantItemPerm(uint32 effect_idx)
    if (!itemTarget)
        return;

-    Player* p_caster = (Player*)m_caster;
-
-    // not grow at item use at item case
-    p_caster->UpdateCraftSkill(m_spellInfo->Id);
-
    uint32 enchant_id = m_spellInfo->EffectMiscValue[effect_idx];
    if (!enchant_id)
        return;
@@ -3963,6 +3958,25 @@ void Spell::EffectEnchantItemPerm(uint32 effect_idx)
    if (!item_owner)
        return;

+    Player* p_caster = (Player*)m_caster;
+
+    // Enchanting a vellum requires special handling, as it creates a new item
+    // instead of modifying an existing one.
+    ItemPrototype const* targetProto = itemTarget->GetProto();
+    if(targetProto->IsVellum() && m_spellInfo->EffectItemType[effect_idx])
+    {
+        unitTarget = m_caster;
+        DoCreateItem(effect_idx,m_spellInfo->EffectItemType[effect_idx]);
+        // Vellum target case: Target becomes additional reagent, new scroll item created instead in Spell::EffectEnchantItemPerm()
+        // cannot already delete in TakeReagents() unfortunately
+        p_caster->DestroyItemCount(targetProto->ItemId, 1, true);
+        return;
+    }
+
+    // not grow at item use at item case, using scrolls does not increase enchanting skill!
+    if (!(m_CastItem && m_CastItem->GetProto()->Flags & ITEM_FLAGS_ENCHANT_SCROLL))
+        p_caster->UpdateCraftSkill(m_spellInfo->Id);
+
    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)",

> patch file <

-edit-

deleting the vellum in TakeReagents() was a bad idea, because it happens before executing spell effects...updated patch.

Link to comment
Share on other sites

don`t work with the newst rev.

Could you give bit more useful information?

Which patch does not work in which way?

I'm pretty sure mine applies and compiles with with [8799], just tried again.

I'd really like some feedback.

@darkstalker: Aren't you getting skillups in enchanting when using a scroll with your patch?

Link to comment
Share on other sites

Guest
This topic is now closed to further replies.
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue. Privacy Policy Terms of Use