How efficient is this spell.

[Wezthal]

I think there's still something wrong with this spell's code, but I'm not experienced enough to spot it right then and there, so I'm asking you people. How could this be improved with couple of simple changes?

"Enter map-specific custom script code below. This text will be included in the map script after variables are declared and before any trigger code."

DELETED (outdated + case closed)

FireBall

DELETED (outdated + case closed)

 

[Pharaoh_]

What is more, why do you use private hashtable? I still believe a global one is fine.

 

[Wezthal]

I saw an example when I started using JASS and it used global groups which, if I remember correctly, weren't destoroyed for some reason. Otherwise I would've destroyed them.
But you think the spell is fine otherwise? I haven't done a projectile spell in jass before, so this was a kind of a test if I'm able to.

Edit: This was the spell I once used as an example.
Instant Justice

DELETED (outdated + case closed)

 

[watermelon_1234]

You may want to initialize your variables more. (In declaring your globals and local variables. For example you could have private group FBG_MISSILE = Create Group())

Don't use GetSpellTargetLoc, use GetSpellTargetX and GetSpellTargetY.

You don't really need to have the player variable in FireBall_Start.

I think you can set the filter to null for Group Enumeration since it should no longer cause a leak...

You don't need to inline TriggerRegisterAnyUnitEventBJ.

If you want, you could try to remove those FirstOfGroup loops (since they're kind of slow) and use ForGroup or the filter to do the actions directly. You'll need to have temp globals for that though.

You could consider using TimerUtils so you don't have to use a hashtable.

I don't see the need to have your own custom BJ's except that it makes the code look nicer. You may want to simplify the calculations, like having 0.017453293 instead of Pi/180.

 

[Septimus]

private function Actions takes nothing returns nothing
local location spellLoc = GetSpellTargetLoc()
local real spellX = GetLocationX(spellLoc)
local real spellY = GetLocationY(spellLoc)

why not use getspelltargetx and getspelltargety?

 

[watermelon_1234]

private function Actions takes nothing returns nothing
local location spellLoc = GetSpellTargetLoc()
local real spellX = GetLocationX(spellLoc)
local real spellY = GetLocationY(spellLoc)

why not use getspelltargetx and getspelltargety?

Yeah, I sort of mentioned that. ;D

 

[Wezthal]

Yeah, well I kind of lived in the belief that for getting SpellTargetPoint the only way is by using locations. Either the tutorial I read before wasn't accurate because it said that "...blizzard messed up. Only way to get spelltargetpoint is by using GetSpellTargetLoc()...", so either Blizzard updated or it's the tutorial that messed up, heh. Anyways thanks for pointing that out.

Edit: Umm, but there is no such functions. Did you mean creating custon functions for that? In that case wouldn't it simply add one extra code...

DELETED (outdated + case closed)

Edit2: Tested 'GetSpellTargetX()' and 'GetSpellTargetY()' ingame. Like hashtables, JNGP nor JassCraft won't recognize them so they won't get colorcoded, but they work.
(I aslo updated the spell, so all things you guys reported, should now be fixed/changed)

 

[watermelon_1234]

No, the tutorial is outdated. One of Blizzard's patch gave us the new natives to get the coordinates. I'm sure it will compile correctly if you just put GetSpellTargetX() and GetSpellTargetY().