Spell Deadly Survival

manofsteel · Aug 5, 2008

This is my first submitted spell and also one of my first spells done in JASS.

Name: Deadly Survival
Description Absorbs energy from nearby corpses and gives it to you. If the corpse is an ally corpse you gain mana if it's an enemy corpse you gain health.
GUI/JASS: JASS
MUI/MPI: Both
Laggfree: Yes
Leakfree: Yes
Import difficulty: Very Easy
, just follow the instructions in the map.

A didn't think a screenshot was needed since it's really nothing to see(and I can't take screenshots on this computer...).

Code:

JASS:

function Trig_DS_Conditions takes nothing returns boolean
     return GetSpellAbilityId() == &#039;A000&#039;     //the ability that&#039;s being casted
endfunction

function Trig_dead takes nothing returns boolean
    return IsUnitType (GetFilterUnit (), UNIT_TYPE_DEAD)
endfunction

function Trig_DS_Actions takes nothing returns nothing
     local unit c = GetTriggerUnit()
     local real x = GetUnitX(c)
     local real y = GetUnitY(c)
     local unit pu
     local group g = CreateGroup()
     local real r = (R2I(GetUnitAbilityLevel(c, &#039;A000&#039; ))) *5 + 20
  
     call GroupEnumUnitsInRange(g, x, y, 800.00, Condition(function Trig_dead))
     loop
          set pu = FirstOfGroup(g)
          exitwhen pu == null
          call GroupRemoveUnit(g, pu)
          if IsUnitEnemy(pu, GetOwningPlayer(c)) == true then
                call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
                call RemoveUnit( pu )
           else
                call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
                call RemoveUnit( pu )
           endif
       endloop
      call DestroyGroup(g)
      set g = null
      set c = null
endfunction

//===========================================================================
function InitTrig_DS takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_DS_Conditions ) )
    call TriggerAddAction( t, function Trig_DS_Actions )
endfunction

Seannny · Aug 5, 2008

Lol at the Cliche name, anyways ill take a look at it. =)

Flare · Aug 5, 2008

1)

Just a minor lagg on first cast, but nothing that you notice.

Then preload stuff in your InitTrig - if it's an ability, create a dummy unit, add the ability to the unit, remove the unit (similar applies to unit preloading). If it's an SFX, create the SFX and destroy it

2)

JASS:

IsUnitDeadBJ(GetFilterUnit()) == true

Ewww, a stupid BJ

JASS:

//can become
GetWidgetLife (GetFilterUnit ()) &lt; .405
//OR, if you want to be safe (since you can use SetUnitState (whichUnit, UNIT_STATE_LIFE, whichVal) on dead units which screws up the above condition, but you shouldn&#039;t be dumb enough to increase a dead unit&#039;s life anyway <img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite8" alt=":D" title="Big Grin    :D" loading="lazy" data-shortname=":D" />
IsUnitType (GetFilterUnit (), UNIT_TYPE_DEAD)
//which is safer (if you have a tendency to add life to dead units (which has no effect whatsoever)

3) Your indentation method is... strange. IMO, you should have it something like

JASS:

//Just a random example
function MyFunc takes nothing returns nothing
  local integer i = 0
  loop
  exitwhen i &gt; 5
     call DoSomething(i)
     if i == 3 then
         call AnotherThing (i)
     endif
     set i = i + 1
  endloop
endfunction

That way, you know exactly what is contained in what block (since all code contained within the open/close statements is indented a bit further than the open/close statements)

4) Configuration header would be nice - if you're using vJASS, you can do

JASS:

globals
//private syntax requires a scope or library
   private constant integer SPELLID = &#039;A000&#039;
   private constant real RESTOREVAL = 20.
   private constant real MULTIPLIER = 5.
endglobals

If you're not using vJASS

JASS:

constant function SpellId takes nothing returns nothing
   return &#039;A000&#039;
endfunction

constant function GetRestoreVal takes integer level returns integer //not sure if I2R works within a constant function :\
  return 20 + level * 5
endfunction

5) You never nulled owner

Exide · Aug 5, 2008

Nice, cool idea. :thup:

I saw one or two things in your code that could be changed, though..

JASS:


function Trig_dead takes nothing returns boolean
    return ( IsUnitDeadBJ(GetFilterUnit()) == true )
endfunction

could probably be:

JASS:


function Trig_dead takes nothing returns boolean
    return ( GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) &lt;= 0 )
endfunction

Also:

JASS:


    set gg_trg_DeadlyS = CreateTrigger(  )

Could be:

JASS:


    local trigger gg_trg_DeadlyS = CreateTrigger(  )

EDIT: Beaten, and pwned.

manofsteel · Aug 5, 2008

Thanks for the feedback, I will change these things tomorrow since I need to sleep.

Larcenist · Aug 5, 2008

Cool spell (though I never tried it out) but kinda simple.

@Exide:

What good would it do localizing the trigger if you're using the gg_trg_blablabla?

JASS:


local trigger T = CreateTrigger()

or

local trigger trig = CreateTrigger()

Using easy trigger names saves you some time when adding conditions and events (unless you lame and convert a GUi trigger with event and condition).

Uberplayer · Aug 5, 2008

Nice to see a new Jasser here.

I usually don't mention anything about the spell's description, but this time I do. It is kinda weird that it's saying "Absorbs deadly energy from nearby corpses", when it's still giving hit points/mana to the caster. The "deadly" word could be removed completely from it

.

Then about intendation. When code is intended correctly, it's much easier to read. So.

1. Everything that's inside functions, should never be on the same 'level' as "function" and "endfunction" keywords. (I don't personally mind if comments or text macros are.):

JASS:

function Trig_DeadlyS_Actions takes nothing returns nothing
    local unit c = GetTriggerUnit() //the triggering unit
    local location p
    local unit pu
    local group g
    local real r
    local player owner

    //owner of triggering unit
    set owner = GetOwningPlayer(c)
    //amount of healing and mana gained
    set r = 20 + (R2I(GetUnitAbilityLevel(c, &#039;A000&#039; *(5))))
    //position of triggering unit
    set p = GetUnitLoc(c)
    set g = GetUnitsInRangeOfLocMatching(800, p, Condition( function Trig_dead ) )
    loop
    set pu = FirstOfGroup(g)
    exitwhen pu==null
    call GroupRemoveUnit(g,pu)
    if IsUnitEnemy(pu, owner)==true then
    call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
    call RemoveUnit( pu )
    else
    call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
    call RemoveUnit( pu )
    endif
    endloop
// the functions bellow removes and nulls the variables, prevents memory leaks
    call DestroyGroup(g)
    call RemoveLocation(p)
    set pu = null
    set g = null
    set c = null
    set p = null
endfunction

2. It's same with loops and if-then-else statements (some people like to leave the exitwhen part to the same level with loop and endloop keywords):

JASS:

function Trig_DeadlyS_Actions takes nothing returns nothing
    local unit c = GetTriggerUnit() //the triggering unit
    local location p
    local unit pu
    local group g
    local real r
    local player owner

    //owner of triggering unit
    set owner = GetOwningPlayer(c)
    //amount of healing and mana gained
    set r = 20 + (R2I(GetUnitAbilityLevel(c, &#039;A000&#039; *(5))))
    //position of triggering unit
    set p = GetUnitLoc(c)
    set g = GetUnitsInRangeOfLocMatching(800, p, Condition( function Trig_dead ) )
    loop
        set pu = FirstOfGroup(g)
        exitwhen pu==null
        call GroupRemoveUnit(g,pu)
        if IsUnitEnemy(pu, owner)==true then
            call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
            call RemoveUnit( pu )
        else
            call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
            call RemoveUnit( pu )
        endif
    endloop
    // the functions bellow removes and nulls the variables, prevents memory leaks
    call DestroyGroup(g)
    call RemoveLocation(p)
    set pu = null
    set g = null
    set c = null
    set p = null
endfunction

Then about optimization. You're able to set the variables' values when you're declaring them (like the you've done for the unit c):

JASS:

function Trig_DeadlyS_Actions takes nothing returns nothing
    local unit c = GetTriggerUnit() //the triggering unit
    local location p = GetUnitLoc(c)
    local unit pu
    local group g
    local real r = 20 + (R2I(GetUnitAbilityLevel(c, &#039;A000&#039; *(5))))
    local player owner = GetOwningPlayer(c)
    
    set g = GetUnitsInRangeOfLocMatching(800, p, Condition( function Trig_dead ) )
    loop
        set pu = FirstOfGroup(g)
        exitwhen pu==null
        call GroupRemoveUnit(g,pu)
        if IsUnitEnemy(pu, owner)==true then
            call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
            call RemoveUnit( pu )
        else
            call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
            call RemoveUnit( pu )
        endif
    endloop
    // the functions bellow removes and nulls the variables, prevents memory leaks
    call DestroyGroup(g)
    call RemoveLocation(p)
    set pu = null
    set g = null
    set c = null
    set p = null
endfunction

You don't actually need the owner variable, because you're only using it once. Locations can be replaced by coordinates (reals). The group picking function has to be changed to GroupEnumUnitsInRange(..) function, and it works a bit differently. You now have to create the group yourself, because this new function doesn't do it. And you don't have to null the pu variable, because it gets automatically nulled by the loop:

JASS:

function Trig_DeadlyS_Actions takes nothing returns nothing
    local unit c = GetTriggerUnit() //the triggering unit
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    local unit pu
    local group g = CreateGroup()
    local real r = 20 + (R2I(GetUnitAbilityLevel(c, &#039;A000&#039; *(5))))
    
    //see this new function
    call GroupEnumUnitsInRange(g, x, y, 800.00, Condition(function Trig_dead))
    loop
        set pu = FirstOfGroup(g)
        exitwhen pu == null
        call GroupRemoveUnit(g, pu)
        if IsUnitEnemy(pu, GetOwningPlayer(c)) == true then
            call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
            call RemoveUnit( pu )
        else
            call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
            call RemoveUnit( pu )
        endif
    endloop
    // the functions bellow removes and nulls the variables, prevents memory leaks
    call DestroyGroup(g)
    set g = null
    set c = null
endfunction

Exide · Aug 5, 2008

Larcenist said:
Cool spell (though I never tried it out) but kinda simple.

@Exide:

What good would it do localizing the trigger if you're using the gg_trg_blablabla?

JASS:
local trigger T = CreateTrigger() or local trigger trig = CreateTrigger()

Using easy trigger names saves you some time when adding conditions and events (unless you lame and convert a GUi trigger with event and condition).

I didn't know the name mattered?

quraji · Aug 6, 2008

Exide said:
I didn't know the name mattered?

It matters if you like typing quickly and easily. "t" is a lot faster and easier to type than "gg_trg_InitTrig_MyTrig". Plus it's less confusing, and less cluttering. :thup:

manofsteel · Aug 6, 2008

Updated the spell, changed effects and the code.

manofsteel · Aug 6, 2008

Could someone take a screenshot for me?

Flare · Aug 7, 2008

Code:

GetUnitAbilityLevel(c, 'A000' *(5))

Why is the *5 part of the abilId argument for that?

manofsteel · Aug 7, 2008

Flare said:
Code:

GetUnitAbilityLevel(c, 'A000' *(5))

Why is the *5 part of the abilId argument for that?

Ohh, didn't see that will change it.

Any more comments/feedback ?

cr4xzZz · Aug 7, 2008

Very good for 1st JASS spell. +rep from me.

manofsteel · Aug 7, 2008

cr4xzZz said:
Very good for 1st JASS spell. +rep from me.

Thanks.

More comments please! :thup:

Btw, is this going to be approved?

Andrewgosu · Aug 8, 2008

I suggest making a few constant functions for the configuration options, one for the rawcode, other for the "healing" amount.

That way the user has to change the rawcode only once and in addition, makes the spell easier to use.

Also, you should make the spell more appealing, add some special effects.

It's currently too dull and basic.

Next, screenshots are always welcomed, though, you need some special effects (check the suggestion above).

if IsUnitEnemy(pu, GetOwningPlayer(c)) == true then
- call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
- call RemoveUnit(pu)
else
- call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
- call RemoveUnit(pu)
endif

Can be shortened to because you are going to remove the dead unit anyway.

if IsUnitEnemy(pu, GetOwningPlayer(c)) == true then
- call SetUnitState (c, UNIT_STATE_LIFE, GetWidgetLife(c) + r )
else
- call SetUnitState (c, UNIT_STATE_MANA, GetUnitState(c, UNIT_STATE_MANA) + r )
endif
call RemoveUnit(pu)

Lastly, just a tip, try giving your variables descriptive names instead of "c", "b".

Currently it's easy to understand what each variable does because of the laconic code, but later, when you advance
and start scripting more elaborate spells, systems or snippets, you'll soon realise naming variables
"the long way" will avoid much trouble and time.

Spell Deadly Survival

New Member

Attachments

Why GUI when you can Jass?

Stops copies me!

I am amazingly focused right now!

New Member

REP: Respect, Envy, Prosperity?

-

I am amazingly focused right now!

zap

New Member

New Member

Stops copies me!

New Member

Also known as azwraith_ftL.

New Member

The Silent Pandaren Helper

Log in

The Helper Discord

Share this page

Affiliates

Network Sponsors