Esdo's "Dwarven Grenade" spell v1.2a

[Moderator]

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

20:34, 3rd May 2014
BPower:
http://www.hiveworkshop.com/forums/spells-569/esdos-dwarven-grenade-spell-v1-2a-249250/index2.html#post2521753

 

[Tal'Rarity]

Post your triggers/code so everyone can view it.

 

[esdo]

Is there any easy way to import all my triggers here? All is in GUI, and using [./trigger] could take quite a while. I got like seven triggers on this one single spell.

 

[IcemanBo]

http://www.hiveworkshop.com/forums/miscellaneous-tutorials-456/how-easily-post-triggers-163285/

 

[esdo]

Thanks for that link. Got all the code posted now.

 

[IcemanBo]

Had quick look on triggers.

- for what you use local integer in 1st trigger?
- read index/deindex methode in a tutorial. you will see what I mean (GUI-tutorial in my signature for example)
- if you have a loop from 1-100 you have to move the "RemoveLocation" action into the loop and not only once after the loop (in more than 1 trigger as I saw)
- one loop starts with 0, all others with 1
- do not use IntegerA for loop, use your own custom integer for it. it's more efficient, and your system should have it's own specific integer
- in first 3 trigger your variables are called something with "dwarven", in other triggers they have prefix "BS_" .. why?

These should be enough for next update. I see this is your first spell here, I wish you good luck with it!

Don't surrender, almost no ressource is at it's best at first few uploads. It always take some time to optimize it.

 

[esdo]

Had quick look on triggers.

- for what you use local integer in 1st trigger?
- read index/deindex methode in a tutorial. you will see what I mean (GUI-tutorial in my signature for example)
- if you have a loop from 1-100 you have to move the "RemoveLocation" action into the loop and not only once after the loop (in more than 1 trigger as I saw)
- one loop starts with 0, all others with 1
- do not use IntegerA for loop, use your own custom integer for it. it's more efficient, and your system should have it's own specific integer
- in first 3 trigger your variables are called something with "dwarven", in other triggers they have prefix "BS_" .. why?

These should be enough for next update. I see this is your first spell here, I wish you good luck with it!

Don't surrender, almost no ressource is at it's best at first few uploads. It always take some time to optimize it.

Thanks for giving a look at it! Working on your suggestions right now.

The BS_ vars are from a "Arc Projectile System" from NFWar ( dunno if it is a HiveWorkshop user ). Good thing you noted that. I forgot about the system. Im gonna give him proper credit on the next release.

But hey i got one question for you, who knows how my system works.

I give every spell an "ID" that ranges from 0 to 100. That means I can have up to 101 of this spell's type cast at once, and the triggers will still work fine.

But that method forces me to use a bunch of "Periodic Event" triggers on a "For variable x from 1 to 100".

I want to know if that may cause any lag. The looping triggers.

Thanks!

 

[IcemanBo]

On it's own a loop with 100 iterations is not a big deal at itself. But it's not needed here.

You should only loop from 1-MaxIndex.
If you read index tutorials where is explained how to solve this, and you will also be able to have more than 100 instances. (usually 100 is enough, but it will be also in a more efficient way)

Then it's also important what you do in your loop.
For example if you loop from 1-100 each 0.02 seconds and also create some unit groups in each iteration inside the loop, it might be to heavy operations in too short time.

 

[esdo]

Updated the map.

I read the tutorial there. But I've read that the MAX index size an array variable could take is 8192. So, using that index method, and if the limit truly exists, one would only be able to cast the spell 8192 times before the triggers crashed. Changing the limit from 100 to 8192 in my map also caused heavy LAG.

But if the limit doesn't exist, why not to make a...

For RandomInt from "MaxIndex - (Quantity of spell indexes currently in use)" to "MaxIndex"...

So every time a spell starts it increases an integer counter for (Quantity of spell.... bla bla) and every time the sell ends, the counter decreases.

That way, the FOR would run on like 10 vars max, potentially decreasing memory usage.

I don't know if that would make any sense, and I'm kind of a noob in WE right now, but would that work?

 

[IcemanBo]

No you can only run 8192 instances at once, not at all. Of course you need a proper deindex for that.

And as I said dont loop from 1 to 100 but only from 1 to MaxIndex. As was did in your 2nd system "For each (Integer BS) from 1 to BS_Times, do (Actions)".

Read this: http://www.hiveworkshop.com/forums/...en-using-triggers-gui-233242/#Section 2 Ch 12

 

[esdo]

No you can only run 8192 instances at once, not at all. Of course you need a proper deindex for that.

And as I said dont loop from 1 to 100 but only from 1 to MaxIndex. As was did in your 2nd system "For each (Integer BS) from 1 to BS_Times, do (Actions)".

Read this: http://www.hiveworkshop.com/forums/...en-using-triggers-gui-233242/#Section 2 Ch 12

I read that tutorial. But there's a problem. In that spell, the duration is fixed, so the spell cast first will always be the first to end. In my system, the fuse timer is random, so the first spell cast may or may not be the first to end. If it isn't, that being if the second spell cast has a shorter fuse time, the trigger would crash.

 

[esdo]

Now, take a look at new code in v1.1c:

See if that's the correct method.

 

[deathismyfriend]

Switch these around.

• Set DwarvenGrenade_CasterPoint = (Position of (Triggering unit))
• Set DwarvenGrenade_Caster[DwarvenGrenade_Index] = (Triggering unit)

And use DwarvenGrenade_Caster[DwarvenGrenade_Index] instead of triggering unit.

Store this in a variable and use the variable for efficiency.

• (Last created floating text)

Anything used twice or more should be stored.

After decreasing this.

• Set DwarvenGrenade_Index = (DwarvenGrenade_Index - 1)

You also need to decrease your looping integer or it skips a spell instance whenever a spell instance ends.

Don't use integer A / B. They are slower and less efficient than using your own custom integer.

Looping from 1 to 100 every time is very inefficient.

I don't see what you mean by this.

if the second spell cast has a shorter fuse time, the trigger would crash.

If it crashes because of that then this spell is not MUI.

 

[ILH]

Will the spell's cooldown removed or not?

 

[esdo]

Will the spell's cooldown removed or not?

Just updated the spell to v1.1d. Now has cooldown that starts when the grenade explodes.

Also fixed some code like the loop skipping (DwarvenGrenade_LoopInteger = DwarvenGrenade_LoopInteger -1).

 

[esdo]

Switch these around.

• Set DwarvenGrenade_CasterPoint = (Position of (Triggering unit))
• Set DwarvenGrenade_Caster[DwarvenGrenade_Index] = (Triggering unit)

And use DwarvenGrenade_Caster[DwarvenGrenade_Index] instead of triggering unit.

Store this in a variable and use the variable for efficiency.

• (Last created floating text)

Anything used twice or more should be stored.

After decreasing this.

• Set DwarvenGrenade_Index = (DwarvenGrenade_Index - 1)

You also need to decrease your looping integer or it skips a spell instance whenever a spell instance ends.

Don't use integer A / B. They are slower and less efficient than using your own custom integer.

Looping from 1 to 100 every time is very inefficient.

I don't see what you mean by this.


If it crashes because of that then this spell is not MUI.

Those are all fixed, but whats up with storing last created text? Why should I? Thats the only thing you said that i didn't fix.

 

[deathismyfriend]

As i said above. It is for efficiency.
Function calls ( The things you see in parenthesis are costly)

Store this in a variable and use the variable for efficiency.

• (Last created floating text)

Anything used twice or more should be stored.

 

[ILH]

Just updated the spell to v1.1d. Now has cooldown that starts when the grenade explodes.

Also fixed some code like the loop skipping (DwarvenGrenade_LoopInteger = DwarvenGrenade_LoopInteger -1).

Nice to hear that.
I'll test this on Saturday

 

[deathismyfriend]

You need to add a config trigger. So people can easily change things like damage / time / whatever else.

 

[ILH]

BUG
when i upgraded this skill to levl 3, i use it, and when i got my skill back, it turned into lvl 1

 

[BPower]

The configuration trigger is lacking. You should add variables like ability, min/max explosion timeout, explosion aoe, used effects, ...

Instead of checking if the periodic triggers are on, you could do a simple integer comparison --> DwarvenGrenade_index == 1

The tooltips are lacking, you can use the blizzard default tooltip for spells as template.

Remove all game text meesages (boom, follow, ...)

You could merge for example the grenadeboom and grenadefollow trigger.

Why do you check wether the grenade is alive or not? If it would be dead, the deindexing wouldn't work.

Last created unit is not safe to use in this case. It could be any unit.

• Unit - Add Storm Crow Form to (Last created unit)

Store in GrenadeArc TriggeringUnit into a variable. You can use a local unit variable via custom script. Please null this variable at the end of the trigger to avoid potential unit handle leaks.

I'm sure you don't need 4 variables of type point plus one point array for your code.

I didn't check the whole code, yet. I will once you fixed some of the issues I mentioned.

 

[GywGod133]

i have suggestion...

use 0.03sec to all trigger looper

0.03sec is basically is a minisec
means this is a good timer to your spell

if you use this
you should add some real variables to this

 

[Daffa]

I'm an anti 0.03 to be honest, I would go with a little Jass and use 0.03125
It's just a variable and 2 actions needed.

 

[deathismyfriend]

Ya going with Jass is better but is sadly not required in GUI spells.