Arrmageddon 1.3

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

Arrmageddon 1.3 | Reviewed by Maker | 2nd May 2013

NEEDS FIX

Target point leaks in Armageddon trigger Make the terrain deformation optional since it can cause an FPS drop Dummies should not use the default upgrades and their death type should be can't raise, does not decay. They don't need to give vision either. Disable their attacks The duration data field of the ability affects the animation the caster plays It looks better when not set to 0.01, but to a higher value I don't favour using dummy units instead of timer to be used for delays You only need one dummy unit type, not three

[tr]

 

[Tal'Rarity]

Shouldn't you fix your spellpack first?
Please learn how to use only one dummy unit, if you want people useing your stuff... it makes it much MUCH better. All you need a blank dummy unit where you can find on from vexorian I think and then just learn to use special effects.

 

[Maker]

You leak the offset point in the first trigger.

 

[Elprede]

Did you paste the incorrect trigger for the third, daRk_sParroW?

 

[daRk_sParroW]

Did you paste the incorrect trigger for the third, daRk_sParroW?

oops, my bad..

You leak the offset point in the first trigger.

oh yeah.. I missed it.

Shouldn't you fix your spellpack first?
Please learn how to use only one dummy unit, if you want people useing your stuff... it makes it much MUCH better. All you need a blank dummy unit where you can find on from vexorian I think and then just learn to use special effects.

I'm working on it...

v.1.1 Update

 

[Almia]

replace Key(Triggering Unit) with an integer variable then do this : set Key = GetHandleId(udg_SomeUnit). SomeUnit is your triggering unit.That is just an example,replace all the parameters i have given.
This same goes to Key(last created unit).

Cache Owner of Caster

Terrain deform times , size, and depth should be configurable.

The "300" in the unit group should be configurable.

Because you are using so many dummies,i suggest using dummy.mdx. This will make the dummy data's very configurable.

Spell's variable prefix is too simple. Make it Armg or just AR. A is not enough to prevent variable collision.

You can merge Armageddon 2 and Armageddon Effect as one trigger by using If conditions.

The generic expiration timer value should be at least configurable.

 

[mckill2009]

Armageddon INIT:
- it's best to make the A_Damage[1], etc...into non-arrayed variables coz it's harder to configure that way and it's also slower, imo.

Armageddon:
- Use "Starts the effect of an ability" instead of begins casting
- You leak A_TargetPoint and offset point
- Use triggering player instead of owner of triggering unit
- key caster and alike (AKA StringHash is slow, use normal integers, like "Hashtable - Save Handle OfA_Caster as 1 of (Key (Last created unit)) in A_Hashtable"
- Imo, kill the meteors when 'it lands on the ground', not when the timer expires

Armageddon 2:
- When choosing color, integer is better then convert it to real

Armageddon Effect:
- Terrain deformation always leak, remove it

 

[daRk_sParroW]

Updated v1.2

- it's best to make the A_Damage[1], etc...into non-arrayed variables coz it's harder to configure that way and it's also slower, imo.

I would recommend my settings to be at this way to make it more configurable, for example what if a user would like to make the level 1 as 223 and level 2 as 346 or something like that would definitely hard for them to configure it.

- Use "Starts the effect of an ability" instead of begins casting
- You leak A_TargetPoint and offset point
- Use triggering player instead of owner of triggering unit

Done

- When choosing color, integer is better then convert it to real

The color is randomized so why change?.

 

[Almia]

StringHash also leaks. Also,read my review above his post.

 

[mckill2009]

I would recommend my settings to be at this way to make it more configurable, for example what if a user would like to make the level 1 as 223 and level 2 as 346 or something like that would definitely hard for them to configure it.

simple:
var1 = 223
var2 = 346
var3 = 1

although it's relatively the same, arrays should be used in a dynamic way like indexing not constants...

The color is randomized so why change?.

why? coz reals are 0.1, 0.23, 0.35 etc.....colors usually is 5, 15, 1 etc...
although conversion is slow but it's better...

@Almia
integers don't leak

 

[daRk_sParroW]

Updated v.1.3

simple:
var1 = 223
var2 = 346
var3 = 1

although it's relatively the same, arrays should be used in a dynamic way like indexing not constants...

Oh, i see. my mistake.

why? coz reals are 0.1, 0.23, 0.35 etc.....colors usually is 5, 15, 1 etc...
although conversion is slow but it's better...

done.

 

[Almia]

@mckill2009
The "string"

 

[mckill2009]

@mckill2009
The "string"

if it's not a handle then it doesnt leak...string, real, integer doesnt leak...

 

[Maker]

Each unique string is stored internally until the game ends. So in that sense they leak.

 

[mckill2009]

if we're talking about addition in memory, everything in wc3 leaks even integers and reals, I've tested it for 10k instance and my memory went up...anyway Im talking about handle leaks...