Javelin Toss v1.00

[Moderator]

12th Dec 2015
IcemanBo: For long time as NeedsFix. Rejected.

17:24, 7th Jan 2015
IcemanBo: http://www.hiveworkshop.com/forums/spells-569/javelin-toss-v1-00-a-259254/#post2637135

 

[Vinz]

Reserved

 

[mogulkhan1Axe]

• For each (Integer A) from 1 to JT_Index, do (Actions)

Ha nice joke with that ''A'' Variable.
I think this is good spell but i think spear height should be reduced by 50%
It just seems too high.

 

[Vinz]

Haha, at first I used X/Y/Z but when I needed more, I used A/B/C. Mostly A, since it appears first on the list and is easy to select.

I'll see about spear height.

 

[mogulkhan1Axe]

Yea spear sometimes look like its gonna hit but it doesn't or otherwise so height should be reduced due to that effect. But that's just my opinion.

 

[chobibo]

Having the starting fly-height configurable would be better too, if that's possible. Also, if you can, also factor in the target's height and adjust the spear height so that it wouldn't look like what mogul just described, at least with regards to the fly height (the model height isn't that easy to determine by triggers, or impossible, dunno really).

 

[GywGod133]

• Unit - Create 1 Cnfg_JT_DummyType for (Triggering player) at TempPoints[1] facing JT_Angle[A] degrees

->>>

• Unit - Create 1 Cnfg_JT_DummyType for (Neutral Passive) at TempPoints[1] facing JT_Angle[A] degrees

to keep score of the scorescreen (right? lol)

 

[Vinz]

@mogul oh, that. It's hard because units, unlike default spell missiles, follow Terrain height. The latter can sink below terrain at times.

@chobibo I'll see. Model height is 0.

@GywGod oh, right. I never thought of that.

 

[GywGod133]

opps

you forgot something ^^

• Unit - Create 1 Cnfg_JT_DummyType for (Neutral Passive) at TempPoints[1] facing JT_Angle[A] degrees

->>

• Unit - Create 1 Cnfg_JT_DummyType for (Neutral Passive) at TempPoints[1] facing JT_Angle[JT_Index] degrees

 

[Vinz]

Oh that must be why it faces the wrong angle at times. Thanks!

 

[IcemanBo]

Should be changed:

• Your iteration through nearby enemies may cause unwanted results, because you don't null "TempUnits[1]" in each iteration of your instances.
  Your condition to damage a unit is "TempUnits[1] Not equal to No unit", but you only null it only once in the end of your periodic.
• Your unit groups leaks. You never destroy it.
• Remove your points after usage, or at end of your iteration. But not only once in end of periodic trigger. Potential location leaks. (If index > 1)
• Why do you use custom script for: Kill Dummy, null caster? .. You can do both with GUI. (when using GUI there should be as less as possible custom scripts)
• Even if you don't damage an enemy you do still remove "TempPoint[3]" afterwards. No need to do so.

Optional performance optimizations:

• variable[array] is not necessarily needed with so few indices, but may be your personal taste.
• You can check the unit's position at top of loop, and then immediatly deinex if the dummy is out of PlayableMapArea.
• OwnerOfCaster could be stored into a global variable.
• PlayableMapArea could be stored into a global variable.

• Custom script: set udg_TempUnits[0] = null

  ^This nulling seems redundant, as it will be used again, and again.

Configuration possibility looks very good.
Maybe it would be cleaner to post the code in spell description.

 

[Vinz]

Your unit groups leaks. You never destroy it.

I missed that.

Remove your points after usage, or at end of your iteration. But not only once in end of periodic trigger. Potential location leaks.

Oh, right, I didn't notice that.

Why do you use custom script for: Kill Dummy, null caster? .. You can do both with GUI. (when using GUI there should be as less as possible custom scripts)

It's been like a custom for me to null/kill things in JASS. xD

But I will GUI-fy it.

Thanks for the review!

 

[Lvthnn]

Maybe my Warcraft is a bit old, but when I tested the map, it first turned out awesome. After quite a while, when the spear hit one of the enemies, the dummy suddenly stopped appearing. Very nice anyway 5/5!

 

[Vinz]

I don't know, every update I kill all the Furbolgs while testing. :[

 

[IcemanBo]

• Null Caster- and Dummy[MaxIndex] onDeindex.
• Null TempUnits[1] must be nulled after usage. Else it brings unwanted results.
• Only remove locations if they exist. And if they exist and you don't need them anymore remove them in same code-block.
• Your temp variables don't really need to have an array.
• Spells with not needed imports are prefered. Could you make the imports optional and just suggest to use them here in spell description?
• Please add import instructions.

Needs Fix