[Snippet] Stack

[Anachron]

Can someone please review this?
I created like 6 resources with this base.

 

[Magtheridon96]

Approved.

I'm against the use of function interfaces, but then again, the only people who are actually going to use an entire library to have a Stack are also the same people who are going to be all for using function interfaces.

 

[Luorax]

It could be "inlined" though; as the code only an argument and you don't need to store it (which you couldn't, that's why it matters), it could be just fired using a static trigger, and "this" + "value" could be retrieved using static members/globals.

I don't like function interfaces either, simply because they duplicate the code, and if you have longer functions/methods, they can add quite a lot to the code :S I've still got like 2 libraries based on them though, simply because they're old, and I'm lazy to change half of my spells :'D At least they're short, so it's not THAT big of a deal.

 

[Anachron]

Meh, people don't need to use it
They could simply use my iterator and just do whatever they like if they care about performance.
For those who don't, they will use Interface.

 

[Luorax]

Well, true. But changing it wouldn't take more than 5 minutes in my opinion, and it can make a difference; it's not really the performance (it does the very same thing, so I think there's little to no difference between an "inlined" and a ".evaluate" solution), but the fact that it makes the code beefier.

But hey, it's your resource => it's your call

 

[Anachron]

Okay I will switch Interface to code in the next version, happy?

 

[Luorax]

Words can't describe how happy that makes me! :'D

 

[Anachron]

Updated:
1.7.0: Replaced FunctionInterface with TriggerEvaluate, added full and empty methods (Thanks to Lourax and Maghteridon96 and overcold_ice)

 

[Luorax]

You may want to change the documentation to "code forEach"

But yea, other than that, cool. Definitely better, and it's not harder to use it.

 

[Anachron]

You may want to change the documentation to "code forEach"

What do you mean exactly? The name of the topic for the "Each Static Members" ?

Now since I changed this I had to change IndexStack too. (Just because of dependency)

 

[Luorax]

It took a function interface, now it takes a code. That's all I want to point out.

 

[Anachron]

Ah yeah, fixed. Thanks

 

[Troll-Brain]

About the forEach method :

Boolexpr are like strings, in the sense that they are not created again if it's already there, meaning that calling Filter(function F1) X times will not create X boolexpr but only one.
Then, if you use DestroyBoolExpr it will screw up things which used this boolexpr, simply because this boolexpr will be destroyed.
Imagine a trigger condition as example.
So just don't use it, you don't have to care about leaks.

Note that the functions And() and Or() create a new boolexpr each time they are called.

What i'm saying can be simply checked with GetHandleId.

Also if we use a function which takes and returns nothing as a code argument, TriggerEvaluate should return false.
I'm not sure (need to be tested) but maybe you could just use ResetTrigger before the loop, TriggerEvaluate in the loop and then use GetTriggerEvalCount.

But instead of this stuff, you could simply use a ForForce with a dummy force, however i'm not sure that using a ForForce in the function argument will work as expected. (a ForForce in a ForForce callback)
Or simply use ExecuteFunc, but i'm not fan of strings as code argument, it's error prone.

 

[Anachron]

Is anyone actually using this?

 

[Nestharus]

Don't think so no

It has too much, lol

 

[Troll-Brain]

Is anyone actually using this?

Welcome to the jass ressources section.

Btw you have completly ignored my previous post, you can potentially create not obvious bugs for the map maker in the current state.

 

[Anachron]

Hi Troll-Brain,

in my local PC version that feature is already implemented.
However, it's not completely tested and therefor I won't update it yet.

@Nes:
Well, it's a fully-featured list you can do nearly anything with

 

[Luorax]

@Nes:
Well, it's a fully-featured list you can do nearly anything with

And that's the problem. We put even single globals into separate resources here, in the JASS section, y'know.

Also, the documentation doesn't meet the standards either, because it's actually informative.

 

[Troll-Brain]

@Anachron : You mean that you don't understand what i want mean ?
Do you need an example where there will be a bug ?
I mean seriously it's a fact, not a guess, you just have to remove DestroyBoolExpr, which is 100 % useless and bug prone there.

@Luorax : At least i'm not in this "we", same for structs extends array. Yeah you probably don't care and that's fine but i had to say it.

 

[Anachron]

Troll-Brain,

that completely makes sense. I will remove it to prevent bugs.
Thanks!

 

[Bribe]

And that's the problem. We put even single globals into separate resources here, in the JASS section, y'know.

Also, the documentation doesn't meet the standards either, because it's actually informative.

I agree with these points. No one has been able to stop Nestharus' creativity killing, but I support the attempts.

 

[Anachron]

But you shouldn't create resources because you can,
you should do it because you need them :/

If everybody would create all the resources he can imagine ...
We would have 1{any number of 0s} resources that nobody actually needs to use.

 

[Bribe]

But you shouldn't create resources because you can,
you should do it because you need them :/

Nestharus will claim he needs all those resources and that they are very practical and better off being separate and modular resources.

I agree though the JASS section is cluttered with his ego name.

 

[Nestharus]

Lol, you do realize that I am actually using those collections right? Trigger will be used in PriorityTrigger, which will be used for DDS. I released all of those in preparation for the DDS update ;o.

 

[Bribe]

Lol, you do realize that I am actually using those collections right? Trigger will be used in PriorityTrigger, which will be used for DDS. I released all of those in preparation for the DDS update ;o.

I told you Anachron.

 

[Anachron]

Lol, you do realize that I am actually using those collections right? Trigger will be used in PriorityTrigger, which will be used for DDS. I released all of those in preparation for the DDS update ;o.

Uhm yeah, so you wanna tell me you need Trigger, BooleanExpression, Alloc Alternative, StaticQueueArray, StaticStackArray, List, Dummy, SharedUniqueNXQueue, UniqueNXQueue, NXQueue, SharedQueue, NXStack, UniqueNXStack, SharedUniqueNXStack, Stack, SharedStack, UniqueNXList, UniqueList, StaticQueue, etc. for it?

I told you Anachron.

You can look into the future! :O

 

[Nestharus]

Uhm yeah, so you wanna tell me you need Trigger, BooleanExpression, Alloc Alternative, StaticQueueArray, StaticStackArray, List, Dummy, SharedUniqueNXQueue, UniqueNXQueue, NXQueue, SharedQueue, NXStack, UniqueNXStack, SharedUniqueNXStack, Stack, SharedStack, UniqueNXList, UniqueList, StaticQueue, etc. for it?


You can look into the future! :O

Around 7 of the collections, Trigger, and Alloc Alternative. Trigger uses BooleanExpression. I just coded most of the collection implementations cuz I thought, "might as well." They are all very general, so they can be used in anything that uses collections .

Dummy is used for something else entirely, usually projectile systems and spells.

 

[Swatosj]

There are iterator issues when using this library with the use of stack arrays.

Even if the reset function is called, the hasNext one stops after one iteration when I've tried to iterate over a stack element.

This seems to happen when one iteration has already been done, I can provide you the code which points the matter.

 

[Anachron]

Yes please show me.

 

[Swatosj]

Context is that I got a stash of units which is represented by a Stack, I've made an array of Stacks each player having his own stash.

Here is the INIT and adding part of the code

globals
    Stack array unitsInStash
endglobals

    //Updating the stash part
    if (unitsInStash[playerIndex] == null) then
        set unitsInStash[playerIndex] = Stack.create()
    endif
    call unitsInStash[playerIndex].add(GetUnitUserData(boughtUnit))
    //Check if the new unit has 3 occurences of the same unit-type in the stack
    call AutoUpgradeUnit(boughtUnit, playerIndex)

This is my AutoUpgrade function, the stack is firstly iterated with "GetUnitOccurencesInStash" call, iterator is reset and then used in this function. I get the correct unitStashOccurences value but then the iterator just stops after one loop even if the count stack attribute (the actual stash size) returns 5. This is happening when unitStashOccurences is equals to 3 by the way.

function AutoUpgradeUnit takes unit target, integer playerIndex returns nothing
    local integer unitStashOccurences
    local location unitLocation = GetUnitLoc(target)
    local integer targetUnitType = GetUnitTypeId(target)
    local integer raceId
    local unit currentUnit
    local integer currentUnitId
    local integer unitUpgrade
    //local boolean found = false
    set unitStashOccurences = GetUnitOccurencesInStash(target, playerIndex)
    //call unitsInStash[playerIndex].reset()
    if unitStashOccurences == 3 then
        call DisplayTextToForce( GetPlayersAll(), "ACTUAL STASH SIZE : " + I2S(unitsInStash[playerIndex].count) )
       loop
           exitwhen not unitsInStash[playerIndex].hasNext()
           set currentUnitId = unitsInStash[playerIndex].getNext()
           call DisplayTextToForce( GetPlayersAll(), "CURRENT LOOP UNIT ID : " + I2S(currentUnitId) )
           if ( currentUnitId == targetUnitType ) then
               //Clean handlers and removing necessary units to combine
               set currentUnit = GetUnitFromAliveUnits(playerIndex, currentUnitId)
               call unitsInStash[playerIndex].delete(currentUnitId)
               call RemoveUnit(currentUnit)
           endif
          //Replace the unit by his next tier
           if ( currentUnit == target ) then
               set raceId = GetRaceId(target)
               //Load (tier_level) of (unit_type) in (unit_race_upgrades)
               set unitUpgrade = String2UnitIdBJ( LoadStringBJ( ( GetUnitPointValue(target) + 1 ), udg_UnitTypesInRaces[GetUnitUserData(target)], udg_UNIT_UPGRADES[raceId] ) )
               call DisplayTextToForce( GetPlayersAll(),  LoadStringBJ( ( GetUnitPointValue(target) + 1 ), udg_UnitTypesInRaces[GetUnitUserData(target)], udg_UNIT_UPGRADES[raceId] ) )
               call CreateNUnitsAtLoc( 1, unitUpgrade, ConvertedPlayer(playerIndex), unitLocation, 270.00 )
               call AddSpecialEffectLocBJ( unitLocation, "Abilities\\Spells\\Orc\\AncestralSpirit\\AncestralSpiritCaster.mdl" )
               call DestroyEffectBJ( GetLastCreatedEffectBJ() )
               call unitsInStash[playerIndex].add( GetUnitUserData(GetLastCreatedUnit()) )
                //set found = true
            endif
        endloop
        call unitsInStash[playerIndex].reset()
   endif
   call RemoveLocation(unitLocation)
endfunction

GetUnitOccurencesInStash function

function GetUnitOccurencesInStash takes unit target, integer playerIndex returns integer
    local integer targetUnitType = GetUnitTypeId(target)
    local integer currentUnitId
    local integer occurencesCount = 0
    local unit currentUnit
    //call DisplayTextToForce( GetPlayersAll(), "OccurenceFunction - targetUnitType : " + I2S(targetUnitType) + " / stashCount : " + I2S(currentStack.count) )
    loop
        exitwhen not unitsInStash[playerIndex].hasNext()
        set currentUnitId = unitsInStash[playerIndex].getNext()
        set currentUnit = GetUnitFromAliveUnits(playerIndex, currentUnitId)
        if ( GetUnitTypeId(currentUnit) == targetUnitType ) then
            set occurencesCount = occurencesCount + 1
            set udg_CurrentUnitLocation[currentUnitId] = GetUnitLoc(currentUnit)
        endif
    endloop
    call unitsInStash[playerIndex].reset()
    return occurencesCount
endfunction

 

[Anachron]

               [B]call[/B] [COLOR=#dd4444]CreateNUnitsAtLoc[/COLOR][COLOR=sienna]([/COLOR] [COLOR=#3333FF]1[/COLOR][COLOR=sienna],[/COLOR] unitUpgrade[COLOR=sienna],[/COLOR] [COLOR=#dd4444]ConvertedPlayer[/COLOR][COLOR=sienna]([/COLOR]playerIndex[COLOR=sienna]),[/COLOR] unitLocation[COLOR=sienna],[/COLOR] [COLOR=#3333FF]270.00[/COLOR] [COLOR=sienna])[/COLOR]
               [B]call[/B] [COLOR=#dd4444]AddSpecialEffectLocBJ[/COLOR][COLOR=sienna]([/COLOR] unitLocation[COLOR=sienna],[/COLOR] [COLOR=blue]"Abilities\\Spells\\Orc\\AncestralSpirit\\AncestralSpiritCaster.mdl"[/COLOR] [COLOR=sienna])[/COLOR]
               [B]call[/B] [COLOR=#dd4444]DestroyEffectBJ[/COLOR][COLOR=sienna]([/COLOR] [COLOR=#dd4444]GetLastCreatedEffectBJ[/COLOR][COLOR=sienna]()[/COLOR] [COLOR=sienna])[/COLOR]
               [B]call[/B] unitsInStash[COLOR=sienna][[/COLOR]playerIndex[COLOR=sienna]][/COLOR].[COLOR=color: #666666]add[/COLOR][COLOR=sienna]([/COLOR] [COLOR=purple]GetUnitUserData[/COLOR][COLOR=sienna]([/COLOR][COLOR=#dd4444]GetLastCreatedUnit[/COLOR][COLOR=sienna]())[/COLOR] [COLOR=sienna])[/COLOR]

First, I am not sure you can use GetLastCreatedUnit() with CreateNUnitsAtLoc.
Second: Where does GetLastCreatedUnit get it's UserData from when its just created? Aren't you just adding zero values to the stack?

 

[Swatosj]

First, I am not sure you can use GetLastCreatedUnit() with CreateNUnitsAtLoc.

The same behaviour is working in GUI but I could not test it here because this part of code will never be reached cause of the iterator problem...

Second: Where does GetLastCreatedUnit get it's UserData from when its just created? Aren't you just adding zero values to the stack?

Their UserData is initialized with an Unit Indexer event, I've checked everything until I've came across this bug. I'll guess I'll stick with hastables for the moment, your library itself is cool though.

 

[Anachron]

Please debug your script, normally a thread is run to the end before other actions are run. Make sure you really have UnitUserData.