Very Basic vJASS Spell: Looking For Comments/Criticisms

[WolfieeifloW]

Hey there.
I just got back into WE'ing, and made a very, very basic DoT spell.

I'm just wondering if anyone can give me comments and/or constructive criticisms to help me re-improve my JASS as I'm very rusty.

Without further ado:

// +----------------------------------------------------------+
// | |
// | -=-=- Wolfie's Bite [v1.0] -=-=- |
// | -=-=- By: WolfieeifloW -=-=- |
// | Requires JASS NewGen and TimerUtils |
// | |
// +----------------------------------------------------------+
// | |
// | Strike the enemy with a fierce bite, leaving a deep |
// | wound that will bleed over 3 seconds. |
// | |
// +----------------------------------------------------------+
// | |
// | -=-=- How To Implement -=-=- |
// | 1. Copy this trigger into your map |
// | 2. Copy TimerUtils into your map |
// | 3. Copy the buffs into your map |
// | 4. Copy the abilities into your map |
// | 5. Make sure the 'Rawcodes' in the trigger match |
// | your buffs/abilities/units in Object Editor |
// | 6. Customize the spell |
// | 7. Enjoy! |
// | |
// +----------------------------------------------------------+
// | |
// | -=-=- Credits -=-=- |
// | Credits are not needed, but appreciated |
// | Just don't claim this as yours |
// | |
// +----------------------------------------------------------+
// | -=-=- Version History -=-=- |
// | |
// | Version 1.0 |
// | - Initial release |
// | |
// +----------------------------------------------------------+
scope WolfiesBite
 
globals
// +-------------------------------------+
// | -=-=- MODIFY HERE -=-=- |
// | -=-=- MODIFY HERE -=-=- |
// | -=-=- MODIFY HERE -=-=- |
// +-------------------------------------+
private constant integer SPELLID = 'A000'
 
 
endglobals
 
private function Damage takes integer level returns real
return 25. * level
endfunction
 
// +----------------------------------------------+
// | -=-=- NO TOUCHIE PAST HERE -=-=- |
// | -=-=- NO TOUCHIE PAST HERE -=-=- |
// | -=-=- NO TOUCHIE PAST HERE -=-=- |
// +----------------------------------------------+
struct Data
unit caster
unit target
integer level
integer i
timer t
 
method onDestroy takes nothing returns nothing
call ReleaseTimer(.t)
call .destroy()
endmethod
 
// -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
static method Callback takes nothing returns nothing
local Data d = GetTimerData(GetExpiredTimer())
 
if (d.i < 3) then
call UnitDamageTarget(d.caster, d.target, Damage(d.level), true, true, ATTACK_TYPE_HERO, DAMAGE_TYPE_FIRE, WEAPON_TYPE_WHOKNOWS)
set d.i = d.i + 1
else
call d.destroy()
endif
endmethod
 
static method Conditions takes nothing returns boolean
local Data d
 
if (GetSpellAbilityId() == SPELLID) then
set d = Data.create()
set d.i = 0
set d.caster = GetTriggerUnit()
set d.target = GetSpellTargetUnit()
set d.level = GetUnitAbilityLevel(d.caster, SPELLID)
set d.t = NewTimer()
call SetTimerData(d.t, d)
call TimerStart(d.t, 1, true, function Data.Callback)
endif
return false
endmethod
 
static method onInit takes nothing returns nothing
local trigger t = CreateTrigger()
 
call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
call TriggerAddCondition(t, Condition(function Data.Conditions))
set t = null
endmethod
 
endstruct
 
endscope

Thanks for your time.

 

[PurgeandFire]

A few things:

• Make sure you get into the habit of making your structs, functions, and methods private. This will help avoid collisions with other systems. For example, "Data" is a very common name, so you should make it private. Although, even if it is an obscure name, making something private is just common coding practice.
• The method [ljass]onDestroy[/ljass] is not really used unless you need polymorphism. The reason for this is that it will use a trigger + conditions to evaluate the method every time upon destroy. You can either just perform ReleaseTimer(.t) when you destroy the method or you can rename the method to [ljass]destroy[/ljass] and change the .destroy() within that method to .deallocate(). For example:
  

  method destroy takes nothing returns nothing
      call ReleaseTimer(.t)
      call .deallocate()
  endmethod

• This is just something to consider. Usually method names start with lower case (they are camelCase) and functions and structs start with upper case. These conventions usually look pretty nice. This is totally optional but most coders like it.
  For more info:
  http://www.hiveworkshop.com/forums/...80/jpag-jass-proper-application-guide-204383/
• There also seems to be a problem with the indenting. Usually you should indent to make the code more readable. Although, this might've just been from the forum bugging out. I know it happens whenever you edit the post because it trims leading whitespace.