[Trigger] Enchanting, Is there away to make it efficient?

[CroMoX]

Hey guys, I have a trigger in my game, to enchant items (Combine), where you buy the item Enchant, and it creates (imbues) a new item. I have the trigger correct, as in it works properly, but it looks messy, and I plan to add in more items to enchant.

So my question is; Is there a way to make this trigger more efficient (Smaller, No leaks, less memory, etc)?

Trigger for Enchant

• Combine
  • Events
    • Unit - A unit Acquires an item
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • ((Item carried by (Triggering unit) of type |CFF959697Claws of Attack|R) is owned) Equal to True
        • ((Triggering unit) has an item of type Enchant) Equal to True
      • Then - Actions
        • -------- Creates effect on unit, removes items, takes gold, creates item --------
        • Special Effect - Create a special effect attached to the origin of (Triggering unit) using Abilities\Spells\Items\AIem\AIemTarget.mdl
        • Item - Remove (Item carried by (Triggering unit) of type |CFF959697Claws of Attack|R)
        • Item - Remove (Item carried by (Triggering unit) of type Enchant)
        • Player - Add -500 to (Owner of (Triggering unit)) Current gold
        • Hero - Create |cff87cefaPoison Claws|r and give it to (Triggering unit)
      • Else - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Item carried by (Triggering unit) of type |CFF959697Dagger|R) is owned) Equal to True
            • ((Triggering unit) has an item of type Enchant) Equal to True
          • Then - Actions
            • -------- Creates effect on unit, removes items, takes gold, creates item --------
            • Special Effect - Create a special effect attached to the origin of (Triggering unit) using Abilities\Spells\Items\AIem\AIemTarget.mdl
            • Item - Remove (Item carried by (Triggering unit) of type |CFF959697Dagger|R)
            • Item - Remove (Item carried by (Triggering unit) of type Enchant)
            • Player - Add -750 to (Owner of (Triggering unit)) Current gold
            • Hero - Create |cff87cefaMithril Dagger|r and give it to (Triggering unit)
          • Else - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • ((Item carried by (Triggering unit) of type |CFF959697Gloves of Will|R) is owned) Equal to True
                • ((Triggering unit) has an item of type Enchant) Equal to True
              • Then - Actions
                • -------- Creates effect on unit, removes items, takes gold, creates item --------
                • Special Effect - Create a special effect attached to the origin of (Triggering unit) using Abilities\Spells\Items\AIem\AIemTarget.mdl
                • Item - Remove (Item carried by (Triggering unit) of type |CFF959697Gloves of Will|R)
                • Item - Remove (Item carried by (Triggering unit) of type Enchant)
                • Player - Add -750 to (Owner of (Triggering unit)) Current gold
                • Hero - Create |cff87cefaPower Gauntlets|r and give it to (Triggering unit)
              • Else - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • ((Item carried by (Triggering unit) of type |CFF959697Odd Shield|R) is owned) Equal to True
                    • ((Triggering unit) has an item of type Enchant) Equal to True
                  • Then - Actions
                    • -------- Creates effect on unit, removes items, takes gold, creates item --------
                    • Special Effect - Create a special effect attached to the origin of (Triggering unit) using Abilities\Spells\Items\AIem\AIemTarget.mdl
                    • Item - Remove (Item carried by (Triggering unit) of type |CFF959697Odd Shield|R)
                    • Item - Remove (Item carried by (Triggering unit) of type Enchant)
                    • Player - Add -650 to (Owner of (Triggering unit)) Current gold
                    • Hero - Create |cff87cefaElite Shield|r and give it to (Triggering unit)
                  • Else - Actions
                    • -------- Removes the enchant item from inventory if no item found --------
                    • Item - Remove (Item carried by (Triggering unit) of type Enchant)

NOTE: ENCHANT IS NOT AN ABILITY, IT IS AN ITEM, DISGUISED AS AN ABILITY FOR A SHOP OWNER

 

[Spartipilo]

Of course there is a better way Long chains of If / Then /Else statements aren't the right way. There are several other programing and logic resources provided by Warcraft3 that makes these things a peace of cake, like loops, Id's, hashtables, etc.

Your trigger has some flaws:
· What if you want to Enchant Item2, but you also have Item1, and you, accidentaly, enchant Item1 just because it's first in the list?

There are 3 solutions:
#1 Preset an item slot as the "Item to be enchanted" Like "Place the item you want to enchant in slot 1 (top left corner) and use enchant ability".
#2 Drop the item and use the Enchant ability in the dropped item.
#3 (I'm not sure if this is possible) Use the ability to an item inside your inventory

· What if the unit doesn't have the required gold?

You can just detect the required gold for the thing you want to enchant and do something if gold isn't enough, or just do nothing and display "Not enough gold"

·It triggers everytime you pick an item

That's what conditions are made for. I would suggest using an Ability instead, or even a chat command.

· It also has some event and special effect leaks, and repetitive callings (both programimg issues that makes your map lag)

Take a look at https://www.hiveworkshop.com/forums/triggers-scripts-269/things-leak-35124/

· How to improve?

1st - Remove repetitite callings. It's another way to say "Reduce parentheses as much as you can". Create a Unit variable and call it whatever you want (I call it "Unit"). Set Unit = (Triggering Unit) at the beginning of the trigger, then replace all the (Triggering Unit) in the trigger, with the "Unit" variable. You'll see that parentheses go away.

2nd- When you do a "Unit - Generic Event" the system actually creates 12 events, one for each player (A unit owned by player (1, 2, 3... 12) .. Acquires an item). If you just have 3 players, and you're creating 12 events, men, you have some leaks there. Read again https://www.hiveworkshop.com/forums/triggers-scripts-269/things-leak-35124/. There are ways to detect the amount of players you have in the game and create these specific events just for those players.

3rd- Look for a Hashtable tutorial. You could pre-set a Hashtable where you store the Resulting_Enchanted_ItemType on the Non-Enchanted_ItemType_ID, also the required gold for that enchant. So, when you use one of the 3 suggested methods above, you just have to retrieve the information from the targeted item Id. It's the shortest and easiest way.

Feel free to ask if you have any doubt This is pretty easy, I could have done it in less time than I required to write all this, but it's best if you learn.

 

[CroMoX]

Ok, I will have a squiz at all the stuff later.

Thanks for the help, +Rep for all the information.'

EDIT: Can you link me a good tutorial involving hashtables with items. I could only find one with an ability as an example. Either way I will be reading all I can find of hashtables. Fixed the simple leaks, thanks for that.
I also modified my trigger, and fixed most of the leaks, couldn't figure out about the 2nd Point you told me to fix, because the link doesn't talk about that.

Another thing Enchant isn't an ability, it is an item sold to you, disguised as an ability.

Modified Trigger

• Combine
  • Events
    • Unit - A unit Acquires an item
  • Conditions
    • (Item-type of (Item being manipulated)) Equal to Enchant
  • Actions
    • Set EnchantUnit = (Triggering unit)
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • ((Item carried by EnchantUnit of type |CFF959697Claws of Attack|R) is owned) Equal to True
        • (EnchantUnit has an item of type Enchant) Equal to True
      • Then - Actions
        • Special Effect - Create a special effect attached to the origin of EnchantUnit using Abilities\Spells\Items\AIem\AIemTarget.mdl
        • Item - Remove (Item carried by EnchantUnit of type |CFF959697Claws of Attack|R)
        • Item - Remove (Item carried by EnchantUnit of type Enchant)
        • Special Effect - Destroy (Last created special effect)
        • Player - Add -500 to (Owner of EnchantUnit) Current gold
        • Hero - Create |cff87cefaPoison Claws|r and give it to EnchantUnit
      • Else - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Item carried by EnchantUnit of type |CFF959697Dagger|R) is owned) Equal to True
            • (EnchantUnit has an item of type Enchant) Equal to True
          • Then - Actions
            • Special Effect - Create a special effect attached to the origin of EnchantUnit using Abilities\Spells\Items\AIem\AIemTarget.mdl
            • Item - Remove (Item carried by EnchantUnit of type |CFF959697Dagger|R)
            • Item - Remove (Item carried by EnchantUnit of type Enchant)
            • Special Effect - Destroy (Last created special effect)
            • Player - Add -750 to (Owner of EnchantUnit) Current gold
            • Hero - Create |cff87cefaMithril Dagger|r and give it to EnchantUnit
          • Else - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • ((Item carried by EnchantUnit of type |CFF959697Gloves of Will|R) is owned) Equal to True
                • (EnchantUnit has an item of type Enchant) Equal to True
              • Then - Actions
                • Special Effect - Create a special effect attached to the origin of EnchantUnit using Abilities\Spells\Items\AIem\AIemTarget.mdl
                • Item - Remove (Item carried by EnchantUnit of type |CFF959697Gloves of Will|R)
                • Item - Remove (Item carried by EnchantUnit of type Enchant)
                • Special Effect - Destroy (Last created special effect)
                • Player - Add -750 to (Owner of EnchantUnit) Current gold
                • Hero - Create |cff87cefaPower Gauntlets|r and give it to EnchantUnit
              • Else - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • ((Item carried by EnchantUnit of type |CFF959697Odd Shield|R) is owned) Equal to True
                    • (EnchantUnit has an item of type Enchant) Equal to True
                  • Then - Actions
                    • Special Effect - Create a special effect attached to the origin of EnchantUnit using Abilities\Spells\Items\AIem\AIemTarget.mdl
                    • Item - Remove (Item carried by EnchantUnit of type |CFF959697Odd Shield|R)
                    • Item - Remove (Item carried by EnchantUnit of type Enchant)
                    • Special Effect - Destroy (Last created special effect)
                    • Player - Add -650 to (Owner of EnchantUnit) Current gold
                    • Hero - Create |cff87cefaElite Shield|r and give it to EnchantUnit
                  • Else - Actions
                    • Item - Remove (Item carried by EnchantUnit of type Enchant)

 

[Spartipilo]

You did great with the Unit Variable, however, you don't need an specific "EnchantUnit" variable just for this thing. You can use one and only tempunit variable for this, and any other trigger at the same time, as long as you don't use any "Wait" action, or need the information of the variable in other triggers. In that case then you need an specific global for that (or a Hashtable )

There aren't many good Hashtables tutorials around, but once you get it is really easy. It's better with some custom scripts tough. This way you save on all the NON-Enchanted_ItemTypes the resulting Enchanted_ItemType AND the gold cost. The following script is motly all you need. It stores the ItemData, retrieves it, and uses it. It still has some flaws, but it's a good start

// With this Function you retrieve all the data stored in the item, and compare to do the enchanting.
function ItemEnchant takes nothing returns nothing
    local unit u = GetTriggerUnit() // The Triggering Unit
    local player p = GetTriggerPlayer() // The Owner of the Triggering Unit
    local item i = GetSpellTargetItem() // Target Item of Ability Being Cast
    local integer iID = GetItemTypeId(i) // The ID of the Non_Enchanted ItemType
    local item newitem = LoadInteger(udg_Hash, iID, 0) // Remember, we saved 'zzzz' in slot 0 of this item (asuming it's 'xxxx')
    local integer gold = LoadInteger(udg_Hash, iID, 1) // Remember we stored the required gold for 'xxxx' in slot 1 of it's ID.
    local real x = GetWidgetX(i)
    local real y = GetWidgetY(i)

    if GetPlayerState(p, PLAYER_STATE_RESOURCE_GOLD) < gold then // If Player doens't have required gold
        call DisplayTimedTextToPlayer(p, 0, 0, 10, "|cfff0000You require|r " + I2S(gold) + " |cffff0000 to enchant this item|r") // Displays a text to the player for 10 seconds saying the required gold if it's not enough
    then
        call RemoveItem(GetItemOfTypeFromUnitBJ(u, GetItemTypeId('hhhh'))) // Removes 'hhhh', Where hhhh is the ItemType of the "Item Enchanter"
        call RemoveItem(i) // Removes the Non_Enchanted Item
        call CreateItem(newitem, x, y) // Creates the new enchanted item
        call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Items\\AIem\\AIemTarget.mdl", x, y)) // Creates the Special Effect
        call SetPlayerState(p, PLAYER_STATE_RESOURCE_GOLD, (GetPlayerState(p, PLAYER_STATE_RESOURCE_GOLD)) - gold)) // Sets the Gold
    endif
        
    set p = null
    set i = null
    set newitem = null
    set u = null
endfunction

//===========================================================================
function InitTrig_ItemEnchant takes nothing returns nothing
    set gg_trg_ItemEnchant = CreateTrigger(  )
    call TriggerAddAction( gg_trg_ItemEnchant, function ItemEnchant )
    
    set udg_Hash = InitHashtable()
    
    // Item Setup
    /* Example */ call SaveInteger(udg_Hash /*The hashtable*/, 'xxxx' /*The ItemType ID*/, 0 /*The slot where we're going to save the ID of Item this will turn when enchanted*/, 'zzzz' /*The ID of the enchanted ItemType*/
    call SaveInteger(udg_Hash, 'xxxx', 0, 'zzzz') // Item 'xxxx' will turn into item 'zzzz' when enchanted
    call SaveInteger(udg_Hash, 'xxxx', 1, 750) // The Gold required for item 'xxxx'
endfunction

NOTE: Some of those locals aren't really neccesary, but helps seeing things with more clarity
· Think of Locals as Temporal Globals
· Read the comments, and you'll see everything is a common GUI Function, but in JASS.
· This will still not work, since has no registered event. You have to add it on Map Initialization the way I explain ahead

About the Event Leak: Leave the event on this kind of trigger (Generic Unit Events) empty, and On Map Initialization do this (This is an example, adjust it to the right events)

• Player Group - Pick Every Player in (All Players) and do (Actions)
  • Set Player = (Picked Player)
  • If (Conditions) Then (Actions) else (Actions)
    • Player slot status is Equal to Is Playing
    • Player controller is Equal to User
  • Then - Actions
    • Trigger - Add to *YourTrigger* the event: A unit owned by Player *YourGenericEvent*
  • Else - Actions

Then, about the Item acquisition, we have the same problem The user may enchant the incorrect item since it has no way to control wich one is going to be enchanted. As said before, tell me if you want to use an specific slot, or use the Echant Item in an item in the floor (I would suggest the second one)