Awesome spell! There are some issues though:
- Please keep map demo stuff in their own folder (TreeRevival)
- TreeRevival was not letting me launch the map because of an "unexpected: elseif"
- I would move the RH_ChannelOrder to the top of the list with RH_Ability and RH_DummyType. Not sure what it is doing so far down the configuration
- When you are preloading, why have users input the strings manually when you can just directly refer to their variables?
- I find the 0.02 loop interval to have very little visual improvement from 0.03
- I'm not familar with Jad's KB system, but is KB3D_LoopDamage the damage interval? If so, it's currently hardcoded
- Instead of having to recalculate
360. / RH_HammerCount[RH_Level]
for every unit on cast, you can calculate it in map init and then just RH_SpreadAngle + SpawnAngle[AbilityLevel][
- RH_HammerModel should have an attachment point configurable as well
- The "prevent unit stacking" If/Then/Else looks pretty useless. How can a newly created unit already be in the group?
- Why do you have two conditions to check if it is time to turn on the loop? You only need one of the other
- You don't have to check to see if RH_Tree is dead before killing it. I believe the harvester unit will still have the stop order when ordered to harvest a dead tree
- From the looks of it, Cos and Sin can be stored on cast instead of having to recalculate every loop iteration
call DestroyEffect( AddSpecialEffectTarget( udg_RH_HammerStartRevolveEffect, udg_RH_Unit, "origin" ) )
should have an attachment point configurable! Some effects look weird when applied to origin, so you should give users the freedom to choose where it goes
- Do you intend for the caster to be able to channel multiple instances of this spell? It also appears that the caster immediately begins rechanneling the spell even after they are stunned (I've had this problem before)
- This spell creates a lot of dummy units! I recommend you take a look at DummyRecycler to help with performance
You use way more Jass than you do GUI. It made your trigger pretty unreadable since Custom script doesn't have any indentation or colored syntax. Why did you chose to make this spell in GUI...? The spell is extremely promising, but for now it will be set to
Awaiting Update.
^ Updated to
v1.1
* Removed unused systems on the demo map.
* SPELL DATA section moved from the top of the configuration field.
* Preloads now the string effect variables instead of manually touching the preload custom script lines.
* Removed the kb loop damage. ( It is not intended to damage on kbed units )
* Added initialization calculations. ( Prevents recalculating issues )
* Attachment points are now configurable.
* Added a trail effect to the hammers.
* the prevent unit stacking if block is now removed lol didn't notice that it is useless xD.
* Now only uses 1 checking group to turn off the looping trigger.
* Removes the checking of dead trees condition.
* The caster nows stops rechanneling the spell when stunned.
* All effects variable are now merged into 1 effect array variable.
* Now the hammers prevent to go outside the playable area or the map.
* Added a safe filtering function, ( removes null boolexpr leaks in 1.24version below ) ( leakless )
* Added a playable area boundaries function.
* Removed some useless configuration values.
* Added a note that the hammers amount cannot be 0.
* Added a configuration of ability's max level.
* Added a dummy hammers player configuration boolean. ( Hammer units now supports player colors )