• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!

[Solved] Mysterious Lag

Status
Not open for further replies.
Level 2
Joined
Mar 31, 2012
Messages
13
Hey guys. Simple question: Does this leak?

JASS:
function Inventory_Switch_Condition takes nothing returns boolean
    return GetSpellAbilityId() == 'A000'
endfunction

function Inventory_Switch takes nothing returns nothing
    local integer i = 0
    loop
        exitwhen i > 5
        set bj_lastCreatedItem = null
        if udg_The_Sack[i] != null then
            set bj_lastCreatedItem = CreateItem(udg_The_Sack[i], GetUnitX(GetSpellAbilityUnit()), GetUnitY(GetSpellAbilityUnit()))
            call SetItemCharges(bj_lastCreatedItem, udg_The_Sack[i + 6])
        endif
        set udg_The_Sack[i] = GetItemTypeId(UnitItemInSlot(GetSpellAbilityUnit(), i))
        set udg_The_Sack[i + 6] = GetItemCharges(UnitItemInSlot(GetSpellAbilityUnit(), i))
        call RemoveItem(UnitItemInSlot(GetSpellAbilityUnit(), i))
        if bj_lastCreatedItem != null then
            call UnitAddItem(GetSpellAbilityUnit(), bj_lastCreatedItem)
        endif
        set i = i + 1
    endloop
endfunction

function Learn_Inventory_Switch_Condition takes nothing returns boolean
    return GetLearnedSkill() == 'A000'
endfunction

function Learn_Inventory_Switch takes nothing returns nothing
    //Trigger doesn't have to be MUI because only one Hero at a time will have it, but variables need reset when
    //given to a different Hero.
    local integer i = 0
    loop
        exitwhen i > 5
        set udg_The_Sack[i] = 0
        set udg_The_Sack[i + 6] = 0
        set i = i + 1
    endloop
endfunction

//===========================================================================
function InitTrig_The_Sack takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function Inventory_Switch_Condition))
    call TriggerAddAction(t, function Inventory_Switch)
    set t = null
    set t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_HERO_SKILL)
    call TriggerAddCondition(t, Condition(function Learn_Inventory_Switch_Condition))
    call TriggerAddAction(t, function Learn_Inventory_Switch)
    set t = null
endfunction

My map was lag-free until i added this. Doesn't seem like it leaks to me, but who knows, bj_lastCreatedItem & the like are a very shady lot... Maybe it causes lag in some other way? The trigger itself works perfectly but as soon as you activate the spell even once, warcraft starts lagging continuously.

There are other ways to achieve this, i know, but this way fits my purposes best & if possible want to avoid having to rewrite the trigger completely.
 
Last edited:

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,202
JASS:
   call TriggerAddAction(t, function Learn_Inventory_Switch)
This line is referencing a different function not shown above. You are showing this very different function.
JASS:
function Learn_The_Inventory_Switch takes nothing returns nothing
     //Trigger doesn't have to be MUI because only one Hero at a time will have it, but variables need reset when
     //given to a different Hero.
     local integer i = 0
     loop
         exitwhen i > 5
         set udg_The_Sack[i] = 0
         set udg_The_Sack[i + 6] = 0
         set i = i + 1
     endloop
endfunction
 
Level 2
Joined
Mar 31, 2012
Messages
13
This line is referencing a different function not shown above. You are showing this very different function.
Whoops, sorry, edited. My bad. I renamed the functions before posting here because my original naming convention is a bit weird. Didn't check properly ha ha.

But that's not the point... I've been looking at it & the only thing i can think of is that maybe setting the charges of items that normally have no charges to zero again somehow causes some sort of strange interaction that lags? I don't know. Will test that tomorrow...
 
udg_The_Sack != null

->

udg_The_Sack != 0

Also, arrays are already set to 0 so initializing it is not useful unless you intend for the inventory to reset more than once.

Store repeat-referenced things like UnitItemInSlot(GetSpellAbilityUnit(), i)) into a local variable for quicker reference.

Use GetTriggerUnit() instead of GetSpellAbilityUnit()

Don't set the item charges if the stack+6 array index is 0.

You need an event for when a unit loses an item to set its previous values to 0.

Here's a thread from another user who has an MUI version: http://www.hiveworkshop.com/forums/...ackpack-hashtable-not-switching-items-271764/
 
Level 2
Joined
Mar 31, 2012
Messages
13
Also, arrays are already set to 0 so initializing it is not useful unless you intend for the inventory to reset more than once.
Here's a thread from another user who has an MUI version
I do intend for the inventory to reset more than once. Although it's not likely, in the current version of the game it could be necessary so i accounted for it.

Also, i really don't need an MUI version at the moment. Hashtables take up a lot of space and are difficult to manage, aren't they? Want to avoid that if possible, but will keep it in mind just in case, thanks.
You need an event for when a unit loses an item to set its previous values to 0.
Is that necessary? The unit can't lose items from the "invisible" inventory, and all the variables are set upon use, so unless i'm missing something, i don't see the point.
udg_The_Sack != null

->

udg_The_Sack != 0

Store repeat-referenced things like UnitItemInSlot(GetSpellAbilityUnit(), i)) into a local variable for quicker reference.

Use GetTriggerUnit() instead of GetSpellAbilityUnit()

Don't set the item charges if the stack+6 array index is 0.

I did all of this and it worked! Doesn't lag anymore, at all. Thank you so much! I didn't think small optimizations like that would make such a massive difference. + rep

Also it looks like this now:
JASS:
function Inventory_Switch_Condition takes nothing returns boolean
    return GetSpellAbilityId() == 'A000'
endfunction

function Inventory_Switch takes nothing returns nothing
    local integer i = 0
    local item a
    loop
        exitwhen i > 5
        set bj_lastCreatedItem = null
        if udg_The_Sack[i] != 0 then
            set bj_lastCreatedItem = CreateItem(udg_The_Sack[i], GetUnitX(GetTriggerUnit()), GetUnitY(GetTriggerUnit()))
            if udg_The_Sack[i + 6] > 0 then
                call SetItemCharges(bj_lastCreatedItem, udg_The_Sack[i + 6])
            endif
        endif
        set a = UnitItemInSlot(GetTriggerUnit(), i)
        set udg_The_Sack[i] = GetItemTypeId(a)
        set udg_The_Sack[i + 6] = GetItemCharges(a)
        call RemoveItem(a)
        set a = null
        if bj_lastCreatedItem != null then
            call UnitAddItem(GetTriggerUnit(), bj_lastCreatedItem)
        endif
        set i = i + 1
    endloop
endfunction

function Learn_Inventory_Switch_Condition takes nothing returns boolean
    return GetLearnedSkill() == 'A000'
endfunction

function Learn_Inventory_Switch takes nothing returns nothing
    local integer i = 0
    loop
        exitwhen i > 5
        set udg_The_Sack[i] = 0
        set udg_The_Sack[i + 6] = 0
        set i = i + 1
    endloop
endfunction

//===========================================================================
function InitTrig_The_Sack takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function Inventory_Switch_Condition))
    call TriggerAddAction(t, function Inventory_Switch)
    set t = null
    set t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_HERO_SKILL)
    call TriggerAddCondition(t, Condition(function Learn_Inventory_Switch_Condition))
    call TriggerAddAction(t, function Learn_Inventory_Switch)
    set t = null
endfunction
Is that okay? Like i said, lag's gone now so i'm satisfied either way :D
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,202
really now? Really?
Was there a point to your post? Or did you just like spamming quotes and "really"?

As to why the initial trigger lagged it was likely because you were creating a lot of items.
JASS:
if udg_The_Sack[i] != null then
This probably evaluated to true all the time so you created more and more items which then leaked.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
I will be more precise this time.
You really had to dedicate a post just to say he had a typo and/or didnt actually understand that if the function misses "The_" from the name, it is completly different function? (thats exactly what you said).
 
I will be more precise this time.
You really had to dedicate a post just to say he had a typo and/or didnt actually understand that if the function misses "The_" from the name, it is completly different function? (thats exactly what you said).

It was a legitimate thing to point out, but in this case was irrelevant by chance. Please don't read too much into it and just continue with peaceful discussions.
 
Level 2
Joined
Mar 31, 2012
Messages
13
JASS:
Jass:
if udg_The_Sack[i] != null then
This probably evaluated to true all the time so you created more and more items which then leaked.
Duly noted, thanks.

As to the whole "The_" thing, you pick up typos like that every time you save the map, so i agree w/ edo494 that it was a little redundant to mention, but whatever. Doesn't matter.

Anyway, thanks for the help, guys :)
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,202
As to the whole "The_" thing, you pick up typos like that every time you save the map, so i agree w/ edo494 that it was a little redundant to mention, but whatever. Doesn't matter.
No you do not pickup all such typos on map save. You pickup undeclared function name typos on map save. You do not pickup wrong function name typos on map save. It is only a syntax error to reference a function that does not exist and not the wrong function that does exist.

My reasoning was that maybe he was referencing a different function not shown in his extract and was unaware of it. Perhaps the function name referenced did exist but in another trigger. As such the lag was produced by something else running which was not intended. If this was not the case then clearly he had not copied the script properly and without a accurate copy people will probably be unable to help him (the copy might be missing the error).

Would anyone really be that stupid not to notice? Yes, and I have seen it dozens of times already. We all make such a mistake from time to time and sometimes do not notice it.
 
Level 2
Joined
Mar 31, 2012
Messages
13
Okay, in my defense it's a very common mistake, & picking up a syntax error is as good as picking up the whole thing because syntax errors are pretty obvious as soon as you know you're looking for one & not for something else entirely.

But you do have a point that i may have been referencing a function not stated here & thus might have given insufficient information, so sorry about that.
 
Status
Not open for further replies.
Top