Nether Barrage v0.02

[Moderator]

12.12
IcemanBo: For long time as NeedsFix. Rejected.

03:44, 12th Dec 2015
IcemanBo:

 

[G00dG4m3]

Hope you will enjoy this.
Suggestions, flaming and all that stuff is all welcome here,

 

[Adiktuz]

Note: just took a quick look at the code, might test this later...

instead of using the integer array (Integer[]), just use two integer variables since you only use two members of the array (Integer[1] and Integer[2])...

also with the other arrays like Real[], use different variables with more fitting name rather than creating an array with less than 10 members...
ex. change Real[1] to BoltDamage, Real[2] to OrbitSpeed, etc...

its always better to have the variables names to be self-explanatory...

 

[G00dG4m3]

Well, I always create variables with not-self-explanatory names, as I am using Hashtable,
and all spells I release, I try to put into a single spellpack. This way it is just simplier, sorry if any1 got problems because of this.

 

[Adiktuz]

it would be better if you get rid of that bad habit of naming variables at least when creating public resources...

and please turn those arrays into separate variables coz on this case, using arrays is not required and is less efficient than having multiple variables...

 

[Maker]

First of all, I have to say I like the spell.

I'm against adding abilities like that. The command card slots are limited enough, many custom maps use five hero abilities plus maybe backpack icon etc. and the extra icon won't fit there. Maybe you could replace the summon ability with the shoot ability, and when the orbs are used, give the summon ability back.

This is unnecessary since the unit has locust. You can set collision size to 0 in object editor.

• Unit - Turn collision for (Last created unit) Off

In Nether Bolt Load, you are loading stuff and then you do <Is caster alive> check. But you don't need all the stuff if the caster is dead.

Unit group leak:

• Unit Group - Pick every unit in (Random 1 units from PosTargets) and do (Set Target = (Picked unit))

add

• Cusom script: set bj_wantDestroyGroup = true

before that line.

These checks should be right after "remove unit from group" action:

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • (FloatingBolts is empty) Equal to True
  • Then - Actions
    • Trigger - Turn off (This trigger)
  • Else - Actions

In Nether Bolt Slide, you calculate Point[3], but you don't necessarily need it.

I'd modify the tooltip a bit. Using "you" in a tooltip isn't a good idea.

I'm puzzled as to why you'd name the ability v0.01, since clearly this isn't v0.01. The rules state that only "finished" spells should be uploaded (meaning v1.00).

The effects are nice and the coding is good in general.

 

[Bribe]

Bribe

With arrays, if you find yourself using "[1], [2], [3]" for things, those should be converted to regular variables because arrays are slower and consume more memory than regular variables.

(Key (Picked unit)) should be stored into a variable as it involves three function calls.

Nether bolt float
- PosTargets does not need to be set to a variable. You can use "set bj_wantDestroyGroup = true" and the "Unit group is empty" check will destroy the group internally.

Special effects would be nicer if their string-art could be configured during initialization.

 

[Maximum_Evil]

looks like a great spell, but why not make a max amount of bolts?