Torrent Slash v1.02

[Moderator]

BPower: Approved.

Old:

Torrent Slash v1.01 | Reviewed by Maker | 30th Jun 2013

NEEDS FIX

You are leaking special effects in the looping trigger You could index the blade unit / degree / effect into single arrays to avoid long if/then/elses in the cast and looping trigger

[tr]

 

[Vinz]

~Reserved. Feel free to give constructive criticism and tips.

Remember to use Ctrl + F5, since I updated it.

 

[Maker]

I see you use arrays when you don't need to. I just makes things more difficult to read and has no real benefit

 

[Vinz]

They're for levels of the spell..

 

[Maker]

I mean those TempLevel, TempPoint, TempReal... variables.

 

[Vinz]

Ok, I'll try to remove their arrays once I get back to the computer.

 

[Almia]

In the configuration trigger,there is a for loop action. In configurations,we don't use this,we put the values one by one in-case the user wants to have damage not relative to each other(e.g level 1 = 100, level 2 = 54.5, level 3 = 184.5,etc)

and also, as I have mentioned to your spells and the other rejected spells that are in GUI must have their variables have a prefix,some of your variables are missing it.

 

[Vinz]

Wha-- Missing prefix..?

 

[Almia]

TempPoint ,Temp Unit, etc.
Btw, for loop must start at 1.

and also, because of its current state about the glaives,the glaive number is not recommended as configurable anymore. It will be hard for those beginners to CnP the codes just to index the glaives 5>.

Btw, Unit group range should be configurable.

Replace UnitApplyTimedLifeBJ with call UnitApplyTimedLife(whichUnit, buffId, duration)
and also, doing it in custom script is useless because GUI 99% BJS.

//===========================================================================

function UnitApplyTimedLifeBJ takes real duration, integer buffId, unit whichUnit returns nothing

    call UnitApplyTimedLife(whichUnit, buffId, duration)

endfunction

For the unit scaling(actions), set the other two values to 0. X is enough for scaling.
So its (size, 0, 0)

Ability Level of Caster must be indexed.

Your indexing has a problem. You are not moving the last index to the removed index,you are just nulling the values of the removed index.
It must be, ex.:
set unitData[tempIndex] = unitData[lastIndex]

Damage effect attachments should be configurable.

TempPoint[0] = (TS_CasterPos[TempInteger[0]] offset by 400.00 towards 270.00 degrees)

400 and 270 must be configurable as well.

 

[Maker]

TempPoint ,Temp Unit, etc.

Temp variables are better off without prefixes. That way there is a chance that the map already has variables with the same name and no new ones are created. It would be great if everyone used the same temp variables across all spells.

 

[Almia]

The scary thing is that he uses arrays for them

 

[Vinz]

Oh and that last thing you mentioned, the height of the dummy is uncontrollable with that model, so I create it 400 range south of the hero to make it look normal. Also, I use tempvars many times..

 

[deathismyfriend]

i dont like the arrayed lvls for this. i rather it this way although it kills what almia suggested. about lvl 2 being lower than lvl 1 for damage.

i have 2 variables for damage one is called damageBase. the other is called damagePerLvl. the formula then goes like this. damageBase + ( abilityLvl * damagePerLvl)
lets say my base is 500 and my per lvl one is 250.
this means lvl 1 is 750. lvl 2 is 1000. lvl 3 is 1250 and so on.
The best thing about this is those 2 variables work for all the lvls. As many lvl as u want. so if i want to change my ability to have 100 lvls then i dont have to have 100 lines of the damage. Those 2 variables already do it. example: lvl 100 would be like this 500 + ( 100 * 250)
which would come out to be 25500 damage.

ur indexing is also old style. i thought u learned the way i did it ?

 

[Vinz]

Okay I missed some lines in your trigger. I'll fix it soon.

 

[defskull]

@deathismyfriend
Level 1 - 300 Damage
Level 2 - 330 Damage
Level 3 - 420 Damage
Level 4 - 760 Damage

Solve this.

@OT
It is better to do it like this;

• Actions
  • Set Damage[1] = 300.00
  • Set Damage[2] = 330.00
  • Set Damage[3] = 420.00
  • Set Damage[4] = 760.00

It gives more freedom to the user and bend the spells' configuration to his/her will.

In the cast trigger, you just need to do this;

• Set DamageDealt = Damage[(Level of (Ability being cast) for (Triggering unit))]

Or if you have multiple setup;

• Actions
  • Set TempInt = (Level of (Ability being cast) for (Triggering unit))
  • Set DamageDealt = Damage[TempInt]
  • Set AreaOfEffect = AOE[TempInt]
  • Set Range = Distance[TempInt]

But yes, everything in this world has pros and cons, right ?

Pros
- Works with any kinds of level
- No need extra work - just to need to find the right formula

Cons
- Value is in scale

Pros
- You can alter its value per level without being attached to a certain scale
- Easy to understand and get what level does what damage

Cons
- If you have 100 levels = 100 lines of code

So you just have to find which situation fits you the best.

If your spell only has 5 levels or at least 10, just use Indexing-based.

If your spell has 20 level or more, just Formula-based.

 

[Vinz]

Update, update, update. Hello world.

 

[BPower]

In order to save computation time you should store TriggeringUnit into a variable. Variables lookups are much faster than function calls, consider the loop.

• (Life of TempUnit) Greater than 0.41

--> Change this IsUnitAlive, in a narrow sense units with 0.41 life are not dead. The excact border would be 0.405

Damagetype and Attacktype could be configurable.

 

[Vinz]

You're right, I guess they are both BJs so no need to stay away from IsUnitAlive.

Will do.

 

[Zed]

You're right, I guess they are both BJs so no need to stay away from IsUnitAlive.

Will do.

What BJ?
(iseewhatyoudidthere)

 

[Vinz]

Blizzard JASS.