[System] BuffGenerator

[Magtheridon96]

The TIMEOUT constant could be moved outside the module
You can make it public and use BuffGenerator_TIMEOUT inside the module =D

 

[LuizBills]

Updated.

- real TIMEOUT is now a static constant member of struct Buff -> Buff.TIMEOUT

 

[Bribe]

I recommend changing the ForEachBuff module (changes are highlighted):

    // ForEachBuff Modules
    module ForEachBuff
        static method ForEachBuff takes unit enumUnit returns nothing
            local Buff this=total
    endmodule
    module ForEachBuffLoop
            @implement ForEachBuff@ //This way the above module is optional, because JassHelper only includes the first "implement"
            loop
                exitwhen this==0
                if UnitHasBuff(enumUnit,this) then
    endmodule
    module ForEachBuffNull
                endif
                set this=this-1
            endloop
    endmodule
    module ForEachBuffEnd
            @implement ForEachBuffNull@ //Same as the above
        endmethod
    endmodule

I think it's weird in one of your functions you declare a local variable "i" and then declare "this" which simply points to "i", is there a reason for it?

 

[LuizBills]

I recommend changing the ForEachBuff module (changes are highlighted):

    // ForEachBuff Modules
    module ForEachBuff
        static method ForEachBuff takes unit enumUnit returns nothing
            local Buff this=total
    endmodule
    module ForEachBuffLoop
            @implement ForEachBuff@ //This way the above module is optional, because JassHelper only includes the first "implement"
            loop
                exitwhen this==0
                if UnitHasBuff(enumUnit,this) then
    endmodule
    module ForEachBuffNull
                endif
                set this=this-1
            endloop
    endmodule
    module ForEachBuffEnd
            @implement ForEachBuffNull@ //Same as the above
        endmethod
    endmodule

Updated to v3.2 and implemented this.
Now can use too:

implement ForEachBuffLoop
	// code here
implement ForEachBuffEnd

I think it's weird in one of your functions you declare a local variable "i" and then declare "this" which simply points to "i", is there a reason for it?

On ForEachBuff module "this" refers to current buff of the loop.

@edit
Thanks for the suggestion =) I didn't know that the JassHelper ignores the second implement

 

[Magtheridon96]

I have an awesome idea!
Instead of having a static integer inside the module, you can keep an integer array inside the library and do this:

module BuffStruct

    static method operator buff takes nothing returns integer
        return Buff.index__q[thistype.typeid]
    endmethod

    // code bla bla ble

    private static method onInit takes nothing returns nothing
        set Buff.index__q[thistype.typeid] = InitBuff(AURAID, BUFFID, function thistype.m)
    endmethod
endmodule

typeid is a unique integer that is generated when you create a struct.

 

[LuizBills]

Some advantage?

 

[Magtheridon96]

The smaller the module, the less the generated code ^.^

 

[LuizBills]

smaller module? You're just increasing it.

 

[Magtheridon96]

How? I'm removing an integer from your module and putting it in the struct so it doesn't get copied a bunch of times.
Plus, you already have an onInit method in there and onInit methods all get combined into one function after compiling.
And, that method operator is already present in your module and the typeid is there by default.

 

[LuizBills]

typeid? Oh I had never seen

ok... I will implement soon. And sorry =)

 

[Magtheridon96]

^.^
getType does the same thing too

 

[LuizBills]

Updated =)
Oh thanks again mag¹¹²¹³¹

 

[Bribe]

@Mag, each module/struct/library init is called via ExecuteFunc, they are not merged with each other. Unless you were referring to something else.

 

[Magtheridon96]

Oh- right.

I somehow knew that but said something else

 

[LuizBills]

Updated.

- Improved the method GetBuffDuration() and code generated by BuffStruct module.

 

[Bribe]

This uses an invalid requirement (TimerUtilsEx).

 

[Magtheridon96]

Luiz, you should make this require TimerUtils, and if you want, add static ifs that check if TimerUtilsEx is in the map (Although it's not an official resource, there are a lot of people I know that use it )

 

[LuizBills]

Fixed. Thanks =)

 

[Bribe]

API is a bit vague:

CountBuffs -> CountBuffsOnUnit

AddBuff -> UnitAddBuff

RemoveAllBuffs -> UnitRemoveBuffs

    //You can change these to let the unit ID be the variable stored, and "GetBuffEventUnit" can just do "GetUnitById(buffID)".
    function GetBuffEventUnit takes nothing returns unit
        return eu
    endfunction

    function GetBuffEventUnitId takes nothing returns integer
        return GetUnitUserData(eu)
    endfunction

This provides a unique way of handling buffs. What I suggest for adding some utility is to maybe make a function to create a unique buff without having to use the module (ie. to split InitBuff into a public function without the code argument).

 

[LuizBills]

really need it?

 

[shadowvzs]

nice, i use similiar thing too just that jass not vjass, but i must agree it is really usefull

 

[Bribe]

I don't like when API gets heavily changed after it's been approved (one reason why Nestharus and I clash a lot). So yes, I would consider it pretty contingent on approval to make the function names more specific to the fact you are dealing with a UNIT and a BUFF, not just a buff.

 

[LuizBills]

ok so let's break the compatibility.

@Update D:
Many functions renamed:
AddBuff => UnitAddBuff
RemoveBuff => UnitRemoveBuff
GetBuffDuration => GetUnitBuffDuration
...

 

[Bribe]

Works cool. Your demo map could do a nicer job of preloading the abilities to prevent first-cast lag, but it otherwise not bad.

 

[LuizBills]

I always forget '-'

 

[mysteriis]

amazing system =)

 

[Magtheridon96]

Who could blame ya, I once forgot to preload abilities in my map.
When I released it, people were complaining for hours ;p

 

[Nestharus]

I only see a couple of problems with this.

You aren't handling recursive events. As this stands, events fired inside of events will break =).

With vex's new optimizer released, the variable names should be changed to be readable. I plan to do this with all of my own resources at some point ;o.

This appears to be an excellent resource. I don't see any other problems for now ^_^.

+rep

edit
Ah, I now see problems o-o. Buffs != Abilities ; ). You have to add buffs using something like DummyCaster. Because of this, you'll have to store the buff levels and what not.

So this needs a little bit of rewriting to work correctly : ).

Well, actually, you could do abilities inside of spell books to make them appear as buffs, but that just seems like extra object editor data that you don't need. Also, keep in mind that auras do break EVENT_UNIT_IN_RANGE, so you shouldn't use them to apply buffs.

I think that this needs a bit more work and some more thought put into the design. It's almost there =o.

 

[Troll-Brain]

You aren't handling recursive events. As this stands, events fired inside of events will break =).

With "real" events recursivity problem is really easy to handle :

call DisableTrigger(GetTriggeringTrigger())
// your code
call EnableTrigger(GetTriggeringTrigger())

Can't the same be used with Event ? (Note that i have not really thought about it)

 

[Nestharus]

Yea, but you want the recursive events to run. I wrote a tutorial on recursive data ;o. It's not a big mystery on how everyone handles recursive data now ^_^.


I think LuizBalls just forgot to put it in, it's not a big deal. It is like a 5 minute fix anyways.

 

[Troll-Brain]

I don't get the need of recursive events, each time there was one i just used the mentionned script, as i didn't want the event to fire again anyway.
Have you an example ?

 

[Nestharus]

unit indexing events?

You don't want removed units not to deindex within a unit deindex event do you? What if you create a unit within a unit index event?

Also damage events. If you issue damage to a unit from an effect of the current damage, you want that damage to be modified by the triggers :\.

If DAMAGE_TYPE == fire, issue extra damage

like that ;o.


What if a buff removes other buffs, like negative buffs, and keeps negative buffs from being applied? You'd need the events to fire ; ).

 

[Troll-Brain]

Hmm, disable a trigger doesn't stop it, so it would work fine, or i'm missing something.

Damage events : for me the real good one is just to make all abilities cause 0 damage, and code every spells which make damage, this way you obvioulsy know what happens.

 

[Nestharus]

Troll-Brain, so you want to remove a unit and have it not properly be deindexed?

 

[Troll-Brain]

I still don't get it, if you remove an unit on an deindexing event, it's like you shot an already dead people, nothing more will happen, the unit will be removed properly.

An example plz.

 

[Nestharus]

RemoveUnit(u1) //fires event

Inside of the event

//Remove all units owned by deindexed unit
loop
    //get next unit owned by u1
    call RemoveUnit(ownedUnit)

    //if you disable the deindexer here, the owned unit will not
    //be deindexed, meaning that you will leak indexes. The events
    //have to fire in order for the owned unit to be deindexed. This
    //code runs inside of a fire event, meaning that it needs proper
    //recursion handling so that the event can be recursively run within
    //itself like this example.

    //without recursion, you will not be able to remove any units within
    //a deindex event and you will not be able to create any units within
    //an index event
endloop

edit
In this case, you would not be able to remove buffs within remove events or add buffs within add events.

If you have buffs that depend upon other buffs and a key buff is removed, then you wouldn't be able to remove those other buffs w/o bugging up your events.

 

[qq1587458539]

try pls with return bug, it must be better