Your analysis of this Trigger

hdm · Jun 5, 2014

This is my first attempt to create a MUI Spell alone (no help, only me and my little and fucked knowleged). I was intended to create an ability that damages over time enemies in the Area by their Max HP and this is what I made. (I am not a expert GUI, nor nothing, just the basic one.)

Ability Start
- Events
  - Unit - A unit Starts the effect of an ability
- Conditions
  - (Ability being cast) Equal to Ability Actions
  - Set Ability_MaxIndex = (Ability_MaxIndex + 1)
  - Set Ability_caster[Ability_MaxIndex] = (Triggering unit)
  - Set Ability_AoE[Abilty_MaxIndex] = (Target point of ability being cast)
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
      - Abilty_MaxIndex Equal to 1
    - Then - Actions
      - Trigger - Turn on Ability Periodic <gen>
    - Else - Actions

Ability Periodic
- Events
  - Time - Every 0.03 seconds of game time
- Conditions
- Actions
  - For each (Integer Ability_CurrentIndex) from 1 to Abilty_MaxIndex, do (Actions)
    - Loop - Actions
      - Set Ability_Period[Ability_CurrentIndex] = (Ability_Period[Ability_CurrentIndex] + 0.03)
      - Set Ability_UnitGroup = (Units within 500.00 of Ability_AoE[Ability_CurrentIndex] matching (((Matching unit) belongs to an enemy of (Owner of Ability_caster[Ability_CurrentIndex])) Equal to True))
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Ability_Period[Ability_CurrentIndex] Less than 5.00
        
        Then - Actions
        
        Unit Group - Pick every unit in Ability_UnitGroup and do (Actions)
        
        Loop - Actions
        
        Unit - Set life of (Picked unit) to ((Percentage life of (Picked unit)) - ((10.00 + (0.02 x (Real((Intelligence of Ability_caster[Ability_CurrentIndex] (Include bonuses)))))) x 0.03))%
        
        Else - Actions
        
        Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
        
        Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]
        
        Set Ability_CurrentIndex = Ability_MaxIndex
        
        Set Ability_CurrentIndex = (Ability_CurrentIndex - 1)
        
        Set Ability_MaxIndex = (Ability_MaxIndex - 1)
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Ability_MaxIndex Equal to 0
        
        Then - Actions
        
        Trigger - Turn off (This trigger)
        
        Else - Actions
  - Custom script: call RemoveLocation(udg_Ability_AoE[udg_Ability_CurrentIndex])
  - Custom script: call DestroyGroup(udg_Ability_UnitGroup)

I want you, Pros, if you want and if I am not asking too much from you, to analyse this and tell me what's wrong, how can I fix, if I did it well or not, what I did wrong, etc. But remember, this is my first attempt ALONE, meaning, all my other MUI Triggers I got your help (thx you all alot), except this.

stan0033 · Jun 5, 2014

there is a standalone unit trigger damage units in area, you don't need to pick them.

Solu9 · Jun 5, 2014

stan0033 said:
there is a standalone unit trigger damage units in area, you don't need to pick them.

If he want to ensure that only enemies are damaged he needs to set the group.
The "Damage area" will damage every unit regardless of owner.

hdm · Jun 5, 2014

I think I didn't get what you're saying. Which line has this issue ? And about the indexing ? What do you think ?

Solu9 · Jun 5, 2014

hdm said:
I think I didn't get what you're saying. Which line has this issue ? And about the indexing ? What do you think ?

I'm crazy tired right now, but in my drugged like state the triggers seem fine.

hdm · Jun 5, 2014

Solu9 said:
I'm crazy tired right now, but in my drugged like state the triggers seem fine.

I don't know if it is a praise or not

Solu9 · Jun 5, 2014

hdm said:
I don't know if it is a praise or not

I'm not going to praise you. It means I think the trigger looks fine to me.
Tell me, have you even tried it ingame yet?

Ceday · Jun 5, 2014

You should set Ability_Period[Ability_CurrentIndex] to 0 when deindexing.

IcemanBo · Jun 5, 2014

I don't see you set "Ability_Period" anywhere in the cast trigger.

I would also index the OwnerOfCaster, so you don't need to call a new function each time in period trigger.

In case you don't want the damage to be dynamic you also should index the amount of damage, so you don't need to calculate it each time.

Store "PickedUnit" into a variable to reduce amount of function calls. (each time you use "PickedUnit" you call a function)

The max PeriodTime should be stored into a variable, so user can config it.

Custom script: call RemoveLocation(udg_Ability_AoE[udg_Ability_CurrentIndex])

^You only need this in your deindex part.

Custom script: call DestroyGroup(udg_Ability_UnitGroup)

^You need to destroy it only and each time inside your loop through indixes, because you create a new UnitGroup each iteration.

Dr Super Good · Jun 5, 2014

Unit - Set life of (Picked unit) to ((Percentage life of (Picked unit)) - ((10.00 + (0.02 x (Real((Intelligence of Ability_caster[Ability_CurrentIndex] (Include bonuses)))))) x 0.03))%

You may want this to be done as damage that ignores resistance and reductions. Currently it could possibly kill a unit (hard to tell, stupid unreadable GUI brackets...) which would result in incorrect kill credit.

Set Ability_CurrentIndex = Ability_MaxIndex
Set Ability_CurrentIndex = (Ability_CurrentIndex - 1)

You sure you do not just mean...

Set Ability_CurrentIndex = (Ability_CurrentIndex - 1)

Setting it to max will cause the instance loop to end pre-maturely I would imagine and skip the remaining instances for that cycle. The idea is that you want to recycle through the current index on the next iteration of the loop as the current index has been changed to be what was the max index before so needs to be processed again. Skipping to max index fulfils the end requirements of the loop so it should act similarly to a break statement.

hdm · Jun 5, 2014

Ceday said:
You should set Ability_Period[Ability_CurrentIndex] to 0 when deindexing.

Else - Actions
- Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
- Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]
- Set Ability_Period[Ability_CurrentIndex] = 0.00
Everything Else...

You mean this ?.

IcemanBo said:
1 - I don't see you set "Ability_Period" anywhere in the cast trigger.

2 - I would also index the OwnerOfCaster, so you don't need to call a new function each time in period trigger.

3 - Store "PickedUnit" into a variable to reduce amount of function calls. (each time you use "PickedUnit" you call a function)

4 - The max PeriodTime should be stored into a variable, so user can config it.

5 -

Custom script: call RemoveLocation(udg_Ability_AoE[udg_Ability_CurrentIndex])

^You only need this in your deindex part.
6 -

Custom script: call DestroyGroup(udg_Ability_UnitGroup)

^You need to destroy it only and each time inside your loop through indixes, because you create a new UnitGroup each iteration.

Read this QUOTE /\, I enumerated my doubts

1 - What you mean ? You mean nulling the Period ?

2 - Didn't I do this already ?

3 and 4 - should I do this on another trigger or in this loop trigger?

5 - Done

6 - I didn't understand this part.

EDIT: Because of Double Post.

Dr Super Good said:
Setting it to max will cause the instance loop to end pre-maturely I would imagine and skip the remaining instances for that cycle. The idea is that you want to recycle through the current index on the next iteration of the loop as the current index has been changed to be what was the max index before so needs to be processed again. Skipping to max index fulfils the end requirements of the loop so it should act similarly to a break statement.

Fixed.

JCarrill0 · Jun 5, 2014

hdm said:
EDIT, Shit Double post

you can delete your own posts by the way.

What is the issue with the trigger Ingame? Have you tried it?

hdm · Jun 5, 2014

JCarrill0 said:
you can delete your own posts by the way.

What is the issue with the trigger Ingame? Have you tried it?

Yes I did, I'll tell in a while what happened, I can't now

EDIT: The problem is.. It works fine, two/more units casting the skill, the damage is apllied correct, etc etc. But when the effect finishes for one unit it finishes for ALL. Any help ?

This is my first attempt to create a MUI spell alone.

JCarrill0 · Jun 5, 2014

Just edit, and delete your double post. it happens to me too

IcemanBo · Jun 5, 2014

hdm said:
Read this QUOTE /\, I enumerated my doubts

1 - What you mean ? You mean nulling the Period ?

2 - Didn't I do this already ?

3 and 4 - should I do this on another trigger or in this loop trigger?

5 - Done

6 - I didn't understand this part.

EDIT: Because of Double Post.

Fixed.[/QUOTE]

1. Actually ceday also said a method:

[quote]You should set Ability_Period[Ability_CurrentIndex] to 0 when deindexing.[/quote]

2. No, I mean at cast: Set Ability_Player[Ability_MaxIndex] = OwnerOfUnit

3. Would need to create a new variable[array] like in step 3.

4. Set tmpUnit = PickedUnit in loop trigger ... and then use the variable instead of PickedUnit again.

5. ...

6.

[TRIGGER]Set UnitGroup = PickAllUnits....

Unit Group - Pick Every Unit in UnitGroup And Do Actions: ...

Custom script: call DestroyGroup(udg_UnitGroup)

So destroy it each time you create one new group.

JCarrill0 · Jun 5, 2014

IcemanBo said:
6.

Set UnitGroup = PickAllUnits....

Unit Group - Pick Every Unit in UnitGroup And Do Actions: ...

Custom script: call DestroyGroup(udg_UnitGroup)

So destroy it each time you create one new group.

But Doesn't that still leak?

Ralle said:
Things That Always Leak

Never use these functions:

Unit Group - Pick all units of type _ and do...

hdm · Jun 5, 2014

IcemanBo said:
3. Would need to create a new variable[array] like in step 3.

Something like Ability_MaxPeriod = 5 (This is a Real Variable). Should it use array ? Current_index and Max_Index ?

JCarrill0 said:
But Doesn't that still leak?

That's why he Custom Script to destroy

PurgeandFire · Jun 5, 2014

Moved to Triggers & Scripts.

JCarrill0 · Jun 5, 2014

hdm said:
That's why he Custom Script to destroy

So it wont still leak anyway? (Ralle you lied!)

IcemanBo · Jun 5, 2014

Idk if you know JASS, but the PickAllUnitsOfType is worse because it creates 2 local UnitGroups and none of them gets nulled. And local agents should be nulled.

JASS:

function GetUnitsOfTypeIdAll takes integer unitid returns group
    local group   result = CreateGroup()
    local group   g      = CreateGroup()
    local integer index

    set index = 0
    loop
        set bj_groupEnumTypeId = unitid
        call GroupClear(g)
        call GroupEnumUnitsOfPlayer(g, Player(index), filterGetUnitsOfTypeIdAll)
        call GroupAddGroup(g, result)

        set index = index + 1
        exitwhen index == bj_MAX_PLAYER_SLOTS
    endloop
    call DestroyGroup(g)

    return result
endfunction]

Edit:

@hdm, no it does not need to be an array. You only need array if you want different values for different units.

hdm · Jun 5, 2014

hmm, no, I suck Jass"ing".

EDIT: The old issue has been fixed, it doesn't cancel the effect anymore. But a new has appeared. If two/more units cast this the effect is longer for the last unit. I think it resets the time, like if I was using Waits

Dr Super Good · Jun 5, 2014

I think it resets the time, like if I was using Waits

Um what?!

You do not even use a timer... And TriggerSleepAction cannot be reset as far as I know.

hdm · Jun 6, 2014

So what the hell is wrong, i'll give to you the map with trigger (not the original ofcourse)

EDIT: I think it's not needed, the triggers are here

IcemanBo · Jun 6, 2014

Have you made the changes we mentioned? Because with currently shown trigger it can't work properly.

aple · Jun 6, 2014

Solu9 said:
If he want to ensure that only enemies are damaged he needs to set the group.
The "Damage area" will damage every unit regardless of owner.

Not true, he can just check if the units are enemies of the player, and if they are damage them. Otherwise, if they are allies of the player, nothing will happen.

In an If/Than/Else, just do

If Picked Unit is an Enemy of (Player) Equal to True

Solu9 · Jun 6, 2014

And how exactly would you do that?

Unit - Cause (Triggering unit) to damage circular area after 0.00 seconds of radius 500.00 at (Center of (Playable map area)), dealing 100.00 damage of attack type Spells and damage type Normal

hdm · Jun 6, 2014

IcemanBo said:
Have you made the changes we mentioned? Because with currently shown trigger it can't work properly.

Why It can't work properly ? Point the issues.

And Yes I made your changes. But if there are more suggestions about fixing this trigger, please put between [trigers]. It may help alot.

The trigger works for more than one unit and deals the correct damage without issues. But the effect lasts longer for the last caster. meaning it should deal %hp damage for 10 seconds (example) and for the last caster it is like 15/20 depending on how many units cast the spell

Dr Super Good · Jun 6, 2014

Why It can't work properly ? Point the issues.

We cannot unless you post the current trigger (after some of the above changes were applied). We cannot be sure you applied all the changes correctly unless we can see what you currently have.

And Yes I made your changes. But if there are more suggestions about fixing this trigger, please put between [trigers]. It may help alot.

Please put your current trigger in trigger tags so we can check for anything else that requires fixing. If you want us to post fixes in trigger tags you will need to give us a map with it in as we cannot write GUI code easily. If you do give us a map make sure the problem is easily re-creatable so we can confirm if a fix works.

The trigger works for more than one unit and deals the correct damage without issues.

So you have changed it so that it now deals damage and not removes life? As mentioned earlier removing life can result in improper death credit resulting in no hero XP being rewarded or bounty being given as well as improper kill counts.

But the effect lasts longer for the last caster. meaning it should deal %hp damage for 10 seconds (example) and for the last caster it is like 15/20 depending on how many units cast the spell

Sounds like the removal of instances is not being done correctly as a rough guess. We will need to see what you currently have to look for the problem.

hdm · Jun 6, 2014

Ability Start
- Events
  - Unit - A unit Starts the effect of an ability
- Conditions
  - (Ability being cast) Equal to Ability
- Actions
  - Set Ability_MaxIndex = (Ability_MaxIndex + 1)
  - Set Ability_caster[Ability_MaxIndex] = (Triggering unit)
  - Set Ability_Owner[Ability_MaxIndex] = (Owner of Ability_caster[Ability_MaxIndex])
  - Set Ability_AoE[Abilty_MaxIndex] = (Target point of ability being cast)
  - Set Ability_MaxPeriod = 5.00
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
      - Abilty_MaxIndex Equal to 1
    - Then - Actions
      - Trigger - Turn on Ability Periodic <gen>
    - Else - Actions

Ability Loop
- Events
  - Time - Every 0.03 seconds of game time
- Conditions
- Actions
  - For each (Integer Ability_CurrentIndex) from 1 to Ability_MaxIndex, do (Actions)
    - Loop - Actions
      - Set Ability_Period[Ability_CurrentIndex] = (Ability_Period[Abilty_CurrentIndex] + 0.03)
      - Set Ability_UnitGroup = (Units within 400.00 of Ability_AoE[Ability_CurrentIndex] matching (((Matching unit) belongs to an enemy of Ability_Owner[Ability_CurrentIndex]) Equal to True))
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Ability_Timer[Ability_CurrentIndex] Less than Ability_MaxPeriod
        
        Then - Actions
        
        Unit Group - Pick every unit in Ability_UnitGroup and do (Actions)
        
        Loop - Actions
        
        Set Ability_TempUnit = (Picked unit)
        
        Unit - Set life of Ability_TempUnit to ((Percentage life of Ability_TempUnit) - ((10.00 + (0.02 x (Real((Intelligence of Ability_caster[Ability_CurrentIndex] (Include bonuses)))))) x 0.03))%
        
        Custom script: call DestroyGroup(udg_Ability_UnitGroup)
        
        Else - Actions
        
        Custom script: call RemoveLocation(udg_Ability_AoE[udg_Ability_CurrentIndex])
        
        Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
        
        Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]
        
        Set Ability_Timer[Ability_CurrentIndex] = 0.00
        
        Set Ability_MaxIndex = (Ability_MaxIndex - 1)
        
        Set Ability_CurrentIndex = (Ability_CurrentIndex - 1)
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Ability_MaxIndex Equal to 0
        
        Then - Actions
        
        Trigger - Turn off (This trigger)
        
        Else - Actions

These are the triggers with changes. If I did something wrong (of course I did. but aren't we humans ?) point

Dr Super Good · Jun 6, 2014

Set Ability_MaxPeriod = 5.00

This is constant and should be set at map initialization not every cast. This should not cause any problems and I am only mentioning it as an optimization.

Set Ability_Period[Ability_CurrentIndex] = (Ability_Period[Abilty_CurrentIndex] + 0.03)
Ability_Timer[Ability_CurrentIndex] Less than Ability_MaxPeriod
Set Ability_Timer[Ability_CurrentIndex] = 0.00

Ability_Timer is never incremented. You are instead incrementing a variable called "Ability_Period" which is never used or accessed out side of the increment. This means that the condition "Ability_Timer[Ability_CurrentIndex] Less than Ability_MaxPeriod" will always pass as true and the destructor code (in then else) is equivalently unreachable. Maybe you meant to increment Ability_Timer instead?

Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]
Set Ability_Timer[Ability_CurrentIndex] = 0.00

Incorrect destructor logic. You need to copy everything from the Ability_MaxIndex to the Ability_CurrentIndex. It is difficult to emphisise how important that you copy absolutely, totally and utterly everything. This includes Ability_Timer (and Ability_Period? maybe you do not need to use that variable at all as explained above...).

From your script you should be copying the following.
Ability_caster
Ability_Owner
Ability_AoE
Ability_Timer
Where red means not being copied (at the moment) and green means you are copying.

This is what I have noticed with this pass of revision so am not sure if this is all the problems. Please fix the above and if problems remain post the revised trigger so I can check fixes were applied correctly and look for any more problems.

If you have problems understanding how to solve any of the problems I raised feel free to ask (quote the problem as well so I know exactly what needs explaining).

hdm · Jun 7, 2014

Dr Super Good said:
This is constant and should be set at map initialization not every cast. This should not cause any problems and I am only mentioning it as an optimization.

Ability_Timer is never incremented. You are instead incrementing a variable called "Ability_Period" which is never used or accessed out side of the increment. This means that the condition "Ability_Timer[Ability_CurrentIndex] Less than Ability_MaxPeriod" will always pass as true and the destructor code (in then else) is equivalently unreachable. Maybe you meant to increment Ability_Timer instead?

Incorrect destructor logic. You need to copy everything from the Ability_MaxIndex to the Ability_CurrentIndex. It is difficult to emphisise how important that you copy absolutely, totally and utterly everything. This includes Ability_Timer (and Ability_Period? maybe you do not need to use that variable at all as explained above...).

From your script you should be copying the following.
Ability_caster
Ability_Owner
Ability_AoE
Ability_Timer
Where red means not being copied (at the moment) and green means you are copying.

This is what I have noticed with this pass of revision so am not sure if this is all the problems. Please fix the above and if problems remain post the revised trigger so I can check fixes were applied correctly and look for any more problems.

If you have problems understanding how to solve any of the problems I raised feel free to ask (quote the problem as well so I know exactly what needs explaining).

The ability Period / Timer is a mistake. I edited it wrong, Timer = period

Also I didn't understand this last part, of copies

Dr Super Good · Jun 7, 2014

The ability Period / Timer is a mistake. I edited it wrong, Timer = period

Which would certainly cause incorrect results.

Also I didn't understand this last part, of copies

The sort of instance system you are using can be viewed as an array of structures.

Each structure consists of the following elements in parallel...
Ability_caster
Ability_Owner
Ability_AoE
Ability_Timer
Each structure represents an instance of an ability. Each instance is represented by a unique storage location in the array of structures.

Since your ability does not utilize references and your periodic update does not require order can get away with defining the use of the array of structures as a set of structures. This means that there is no idea of ordering (where an instance is physically does not mater).

Allocating and removing instances from the set of structures is the same as popping and pushing with a stack. Allocations increase a pointer to the top of the stack by 1. Popping decreases the pointer to the top of the stack by 1.

Obviously since it is a set of structures it is possible that any instance from it may be removed at any time so the standard stack pop operation will not suffice as that only removes the top. However since it is a set the ordering does not mater leaving you the capability to move where individual structure instances are located in the array. By transforming an instance you want to delete from somewhere in the array to the top of the stack you can then pop it just by decrementing the stack pointer.

Obviously there are a lot of optimizations one can do. Since we are dealing with a set of elements you only need to move the top of stack instance with the instance you want to remove in the array. Since the top of stack is being discarded anyway there is no need to move the instance being destroyed to the top of the stack and instead you only need to move the instance at the top of the stack to the instance that is being destroyed.

Together all this results in the following deallocation procedure.

1. Finalize instance (destroy sub-instances and any leakable objects like locations, groups, forces, etc).
2. Relocate top of stack instance with instance being destroyed. This requires copying all structure members in parallel. There is no shortcut here, you have to copy all member variables together otherwise the result will be incorrect.
3. Decrement top of stack pointer.

Example
A set of instances inside the underlying structure arrays is defined below. Imagine that A is the oldest instance and D is the youngest.
[A B C D]
Stack pointer is 4
Instance B expires and is to be destroyed / de-allocated.
1. B is finalized converting it into garbage space (-).
[A - C D]
Stack pointer is 4
2. D is moved to where B used to be.
[A D C D]
Stack pointer is 4

Notice how there is now two copies of D.

3. Stack pointer decremented.
[A D C]
Stack pointer is 3

Notice how the top of stack copy of D has been poped away. At the time it is still a copy of instance D but after this operation we consider it as empty space.

We can then go on to allocate a new instance, E.
[A D C E]
Stack pointer is 4

If you perform this deallocation while inside an iteration of the set of structures you will need to decrement the current pointer by 1 as the instance at the element you just processed has changed.

hdm · Jun 7, 2014

/\ er.... what ? I am not the "English - Pro", so there are somethings that I didn't understand, resulting in a great crap to understand your post. Since I don't know what means Array (I just know how to use), Instance and other hard words. Can you "simplify" as much as you can ?

Keep in mind, this is my first attempt to MUI a trigger without no help (unless this help of you)

Also, out of this copy part, all the rest seems OK for you all ?

Dr Super Good · Jun 7, 2014

Since I don't know what means Array

An Array is a programming construct. JASS (produced by the GUI you are using) uses dynamic arrays which automatically expand up to 8192 indices.

Instance and other hard words

An instance is similar to the MMORPG meaning. It simply means an occurance of something. Several instances (occurances) can exist at the same time. If you think of each cast of an ability as an instance, you can have many instances of the ability at the same time if more than one unit casts it or the ability instances last longer than the cooldown. You do not have to use that relationship, you could have it that ever unit affected by an ability is a separate instance (each cast makes several instances), which is best depends on situation and is entirely up to you.

The meanings are not that important which is why I gave an example. In the trigger you posted a while back you did not perform step 2 properly, failing to copy across several pieces of data that was related to that one instance of the ability.

Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]

This is you performing step 2. However each instance has 4 members, not 2.

Ability_caster
Ability_Owner
Ability_AoE
Ability_Timer

Thus you should have this instead...

Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]
Set Ability_Owner[Ability_CurrentIndex] = Ability_Owner[Ability_MaxIndex]
Set Ability_Timer[Ability_CurrentIndex] = Ability_Timer[Ability_MaxIndex]

Obviously this then needs to be removed.

Set Ability_Timer[Ability_CurrentIndex] = 0.00

Because you were not doing this, there were two bugs.
- Incorrect timeout of the instances. The timer progression for the instance at the top of the allocation stack was lost and reset to 0 every time another instance expired. This manefests itself in that the last cast instance of the ability during a multi-instance period of time would last longer than intended.
- If more than 1 player had instances of the ability in a multi-instance period of time the owner of the instances could become corrupt (wrong) resulting in incorrect area search results.

hdm · Jun 7, 2014

Aw, about copy, you mean, copy all the members of the instance, not ONLY the Caster and AoE. That's why you colored with green and red. So I'll do this

Set Ability_AoE[Ability_CurrentIndex] = Ability_AoE[Ability_MaxIndex]
Set Ability_caster[Ability_CurrentIndex] = Ability_caster[Ability_MaxIndex]
Set Ability_Owner[Ability_CurrentIndex] = Ability_Owner[Ability_MaxIndex]
Set Ability_Timer[Ability_CurrentIndex] = Ability_Timer[Ability_MaxIndex]

.

And I'll test.

Question: This what you said is applied to ALL MUI triggers ? If I'll make another MUI Trigger, I have to keep this in mind. and How do you know all this stuff ? Unless you worked with Blizzard, it's impossible to know this.

Dr Super Good · Jun 7, 2014

This what you said is applied to ALL MUI triggers ? If I'll make another MUI Trigger, I have to keep this in mind.

It is applied to all MUI trigger systems you make that do not require static indexes (no references to them).

Some require attaching instances to objects such as units in which case you cannot easily change the index an instance is at and so a different type of system is needed. In that case you use a dis-used member (or dedicated member if you have no integer members) to keep a linked stack of free indexes. Fragmentation is not a problem as once the JASS interpreter expands an array it never shrinks it.

This means the allocation process goes in two steps.
1. Remove top of linked stack (you may need a dedicated non-array variable for this). If linked stack is empty proceed to 2.
2. Allocate from top of stack (what you do here). Can be merged into 1 where the bottom of the linked stack is always the top of stack and when that is used it increments the top of stack and places it back in the queue.

De-allocation is more simple as you just have to place the index in the linked stack.

Complicated? Yup and this is what vJASS structs do (well something like this). The extra complexity is required to keep instances static which is vital if you ever reference an instance directly. The sort of instance system you are using does not support references.

and How do you know all this stuff ?

I studied programming at University.

Unless you worked with Blizzard, it's impossible to know this.

Simple algorithms and data structure principles that apply to all programming languages.

hdm · Jun 8, 2014

hdm said:
EDIT: The old issue has been fixed, it doesn't cancel the effect anymore. But a new has appeared. If two/more units cast this the effect is longer for the last unit. I think it resets the time, like if I was using Waits

Solved Finally. And Also I indentified this /\ issue. When I said "The damage lasts longer for the last caster" is because both casters are different, with different spells and the last caster has an ability "Break Aura" which passively deals 2% of the nearby enemie's max HP per second. So it seemed that the damage of this ability was lasting longer, but the additional damage was actually the Aura's damage.

How dumb am I. Thx to you all, I'll +REP you all.

Your analysis of this Trigger

Similar threads