HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

Cyclone

12-06-2008, 10:42 PM#1
Zerzax
This spell was made for XeNiM666, who requested it in Pyrogasm's Spell Request Thread. It follows the JESP Manifest.

The Spell Uses:
TimerUtils

It was coded using vJASS and the JNGP.

Cyclone
The caster becomes a destructive cyclone that rushes to the target unit and bounces from enemy to enemy, destroying trees in its path and damaging nearby units. Deals 20 damage per level to units in a small range and 30 damage per level to each main target. Each level increases the target limit, starting with 3 targets.

This is my first JESP spell and I would appreciate any corrections to the script or just comments and suggestions. Enjoy!

Expand Script:

Version A has now been updated for more efficiency. I've also added a smaller screenshot that perhaps will do the spell justice. The cyclone is hard to see. Oh well.
Attached Files
File type: w3xCyclone.w3x (115.1 KB)
12-06-2008, 11:36 PM#2
Pyrogasm
I didn't really read the full code, nor did I test the spell, but I noticed this right off the bat:
  • REG_TARGET_DAMAGE, CHAIN_TARGET_DAMAGE, and BASE_CHAIN_COUNT are pretty much superfluous and don't really help anything. The user should just change the values directly in each of the specific configuration functions.
  • JassHelper removes the need for InitTrig functions
  • When would PRELOAD_ID ever be different from WIND_UNIT?
  • Instead of destroying .hitTable in .onDesroy, you can simply leave it (call .reset() if you want) and in the .create method check and only create .hitTable if it doesn't already exist.
12-07-2008, 12:07 AM#3
Zerzax
I put in the InitTrig because of JESP, it stated that "the initTrig function must be named "initTrig_<Spellname>". I took that to mean it required the function, guess not.

I will update those constants, and merge the two unit constants.

Good idea about the tables. I always wondered what the best way to recycle them was.

EDIT: Posted updates.
12-07-2008, 05:06 AM#4
Pyrogasm
The JESP standard is really kind-of irrelevant now because it hasn't been updated to account for the new syntax now.

Essentially, if you make a scope with your spell's name and all of the functions and globals are either public or private, then it follows the JESP standard :P
12-07-2008, 02:09 PM#5
Zerzax
Haha, well I recommend trying out the spell ASAP and seeing what it's like.

Apparently Version B (maybe A too?) has a bug. I'll try to fix it by the end of the week.

EDIT: I think the bug does not exist in this testmap, so worry not about it.

EDIT2: Found some extra junk lying around that won't impact the spell, such as an extra "show unit". Also, I think locust removes the ability to target the affected unit permanently so I will fix that too as soon as someone looks at the script.
01-05-2009, 11:53 PM#6
Zerzax
I'm bumping this because it's been atrophying for almost a month. Is the spell approveable, besides the fact it needs an extraneous function call removed? I'm ready to fix anything that needs it.
02-01-2009, 06:53 PM#7
Rising_Dusk
This looks good enough for me. Nothing jumped out at me, really.
Quote:
Originally Posted by Zerzax
Also, I think locust removes the ability to target the affected unit permanently so I will fix that too as soon as someone looks at the script.
Pretty sure it doesn't so long as it's managed via triggers. You can of course test this.
02-01-2009, 10:08 PM#8
Pyrogasm
I looked at the code too and didn't see anything else, so let's approve it!
02-02-2009, 12:18 AM#9
Zerzax
Hey Hey Hey! Thanks guys :D. I wanted to get a least one submission in, for the experience of it. I'll start using global timers + array stack from now on for my structured spells.
02-02-2009, 03:13 PM#10
Anitarf
You could inline your group recycling like this since the struct itself works like a stack anyway.

You should flush a gamecache value, not just set it to 0, furthermore the units should be removed from the hit group when their time runs out. Well, I don't really like the heavy gamecache calls you're doing anyway. I'd likely be more efficient to just have a timer per hit unit per spell instance instead.

Your description of the COLLISION_DIST calibration constant is incorrect (you're using InRange natives which take collision size into account) and useless (it doesn't really describe what the constant is used for). Also, the enum call should enum units in COLLISION_DIST+ENUM_SIZE range. The ENUM_SIZE constant also has a fairly poor description of what it does.

The line that checks if the destructable is a tree should be a calibration function, in case users have custom trees in their map.
02-03-2009, 07:30 PM#11
Zerzax
Thanks Ani, I'll get right on it. These are things I've addressed in other scripts but never went back to cyclone to fix. I guess the timer per unit idea is soundest as it would indeed wipe out the gamecache usage.

EDIT: Without using the 2-dimensional functionality of gamecache, how would I be able to (a have a unique counter for each unit per spell instance and (b associate the counter to the unit so that it can be referenced by the main group enum?

Also, I'll take a better screeny for each spell type.
02-03-2009, 08:47 PM#12
Anitarf
Quote:
Originally Posted by Zerzax
EDIT: Without using the 2-dimensional functionality of gamecache, how would I be able to (a have a unique counter for each unit per spell instance and (b associate the counter to the unit so that it can be referenced by the main group enum?
For a), I don't really understand the problem, youre already using TimerUtils so getting a timer per unit per spellinstance should be no problem, all you'll need is an extra struct that you attach to the timer and that holds the unit.

You don't really need b).
02-03-2009, 09:42 PM#13
Zerzax
Okay. I can understand how an individual timer per unit would run and eventually satisfy the time duration. OHH I just figured it out. I could check whether or not the unit was in the struct's main group. The individual struct instances would have references to the "parent" spell instance's group, and remove itself once duration is up. I'm guessing that's a good way to do it, so I'll go ahead with it. Thanks.

EDIT: New Version of A up. I'm too lazy at the moment to update B also, I prefer A anyway.
02-08-2009, 11:28 PM#14
Anitarf
Quote:
Originally Posted by Zerzax
EDIT: New Version of A up. I'm too lazy at the moment to update B also, I prefer A anyway.
You should either update it, or remove it. Right now your post still lists Table as a requirement even though version A no longer needs it just because you still have the old B.

I think you should use something like max collision size constant from xe for determining the GroupEnum range instead of having it as a separate poorly explained ENUM_RANGE constant. Enuming units in a larger range is done because the IsUnitInRange* natives take the collision size into account to get proper War3 area-of-effect, yet you describe COLLISION_DIST as a distance between unit centers, which would leave the user in confusion as to why they need to specify a separate ENUM_RANGE to begin with.
02-09-2009, 02:00 AM#15
Zerzax
B has been removed. I'll fix up the script and alter the constants in the desired manner. This should be complete by tomorrow night.

EDIT: Changed the description of the constants as well as their names. I hope it's clear.