My first struct, please help

12-15-2008, 01:08 AM

-Many thanks to Moyack, i red his tutorial last night, and it gives me both needed information and inspiration to convert an old spell to the effective way. Yah, now i have known a little about struct and the use of periodic timer, below is the code of my spell. Im still new to this and not sure it's safe or not, so i hope that some experienced people could help me out.

-The idea is every 0.20 seconds, set DIST from the caster location to DIST+50, and create a special effect at that point, if DIST == 445 then end the trigger. Any enemy unit comes within 100 AOE will take damage, if it dies, it gives 1 skill point to the skill, and if the skill reach required point (x, y, z,..) it will level up.

(in truth, my first idea was to end the trigger after 1 seconds, but don't know how to do that).

-Well, here is my code and also the map:

JASS:

scope StompOfBeast

//***************************************************
globals
  private constant integer AbilityID          = 'A000'
  private constant attacktype AttackType      = ATTACK_TYPE_CHAOS
  private constant damagetype DamageType      = DAMAGE_TYPE_UNIVERSAL
  private constant real VSRatio               = 0.10
  private constant real BonusRatio            = 0.10
  private constant real dt                    = 0.10
  private constant string ET                  = "|cffffcc00Stomp of the Beast acquired total "
  private constant string CT                  = "|cffffcc00Stomp of the Beast's level has increased"
  private constant string SFX                 = "Abilities\\Spells\\Undead\\Impale\\ImpaleHitTarget.mdl"
  private string color                        = "|cffff00ff"
  private integer EXP                         = 0
  private unit caster
  private unit target
  private player p
  private real dam
  private real VSdam
  private real Bonusdam
  private unit u
endglobals
//****************************************************

private function condition takes nothing returns boolean
  return GetSpellAbilityId() == AbilityID
endfunction

private function Check takes nothing returns boolean
  return not IsUnitAlly(GetFilterUnit(), GetOwningPlayer(u)) and GetWidgetLife(GetFilterUnit())>0.405  and GetUnitTypeId(GetFilterUnit()) != 'H007'
endfunction


private struct Data
  unit caster
  integer DIST
  real ANG
  location loc
  static method create takes unit c, integer i, real r, location l returns Data
    local Data D = Data.allocate() 
    set D.caster = c
    set D.DIST   = i
    set D.ANG    = r
    set D.loc    = l
    return D
  endmethod
endstruct


private function callback takes nothing returns nothing
  local timer t = GetExpiredTimer()
  local Data D = GetCSData(t)
  local real X
  local real Y
  local group g = CreateGroup()
  local unit enum
  set u = D.caster
  
  set bj_forLoopAIndex = 1
  set bj_forLoopAIndexEnd = 4
    loop
        exitwhen bj_forLoopAIndex > bj_forLoopAIndexEnd
        set X = GetLocationX(D.loc) + D.DIST * Cos(D.ANG + ( 90.00 * I2R(GetForLoopIndexA())) * bj_DEGTORAD)
        set Y = GetLocationY(D.loc) + D.DIST * Sin(D.ANG + ( 90.00 * I2R(GetForLoopIndexA())) * bj_DEGTORAD)
        call DestroyEffect(AddSpecialEffect(SFX,X,Y))
        set bj_forLoopAIndex = bj_forLoopAIndex + 1
        call GroupEnumUnitsInRange(g, X, Y, 100, Condition(function Check))
        loop
          set enum = FirstOfGroup(g)
          exitwhen enum == null
          call GroupRemoveUnit(g,enum)
          if IsUnitInGroup(enum, udg_Group_Water) then
            call UnitDamageTarget(D.caster, enum, (dam+Bonusdam+VSdam), false, false, AttackType, DamageType, null)
            if GetWidgetLife(enum) <= 0.405 and GetUnitAbilityLevel(D.caster, AbilityID) < 10 then
              set EXP=EXP+1
              if udg_Bo_TextOff ==false then
                call DisplayTimedTextToForce( GetForceOfPlayer(GetOwningPlayer(D.caster)), 4.00, ET+I2S(EXP)+" SP")
              endif
              if EXP ==4 or EXP ==8 or EXP ==12 or EXP ==50 or EXP ==66 or EXP ==84 or EXP ==104 or EXP ==126 or EXP ==150 or EXP ==200 then
                call DisplayTimedTextToForce( GetForceOfPlayer(GetOwningPlayer(D.caster)), 4.00, CT)
                call IncUnitAbilityLevel(D.caster, AbilityID)
                call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\Human\\Resurrect\\ResurrectTarget.mdl", D.caster, "origin"))
              endif
            endif
          else
          call UnitDamageTarget(D.caster, enum, (dam+Bonusdam), false, false, AttackType, DamageType, null)
          call DisplayTimedTextToForce( GetForceOfPlayer(GetOwningPlayer(D.caster)), 4.00, ET+I2S(R2I(dam))+" damage")
            if GetWidgetLife(enum) <= 0.405 and GetUnitAbilityLevel(D.caster, AbilityID) < 10 then
              set EXP=EXP+1
              if udg_Bo_TextOff ==false then
                call DisplayTimedTextToForce( GetForceOfPlayer(GetOwningPlayer(D.caster)), 4.00, ET+I2S(EXP)+" SP")
              endif
              if EXP ==4 or EXP ==8 or EXP ==12 or EXP ==50 or EXP ==66 or EXP ==84 or EXP ==104 or EXP ==126 or EXP ==150 or EXP ==200 then
                call DisplayTimedTextToForce( GetForceOfPlayer(GetOwningPlayer(D.caster)), 4.00, CT)
                call IncUnitAbilityLevel(D.caster, AbilityID)
                call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\Human\\Resurrect\\ResurrectTarget.mdl", D.caster, "origin"))
              endif
            endif
          endif
       endloop 
    endloop
    set D.DIST = (D.DIST + 50)
    if D.DIST == 425 then 
      call DestroyGroup(g)
      call RemoveLocation(D.loc)
      set D.loc = null
      call D.destroy()
      call ReleaseTimer(t)
    endif
    set t = null
    set g=null
    set enum=null
endfunction


private function action takes nothing returns nothing
  //Variables
  local Data D = Data.create(GetTriggerUnit(), 25, GetUnitFacing(GetTriggerUnit()), GetUnitLoc(GetTriggerUnit()))
  local timer t = NewTimer()
  local integer lvl = GetUnitAbilityLevel(D.caster, AbilityID)
  // Damage
  set dam = ((60.00*I2R(lvl))+(2.00*I2R(GetHeroInt(D.caster, true))))/3
  set VSdam = dam*VSRatio
  if IsUnitInGroup(D.caster, udg_Group_Water) == true then
        set Bonusdam = BonusRatio*dam
        call TextTag_Unit(D.caster, "+"+I2S(R2I(Bonusdam)), color)
  else
        set Bonusdam = 0.00
  endif
  call SetCSData(t, D)
  call TimerStart(t,0.10,true,function callback)
  set t = null
endfunction

public function InitTrig takes nothing returns nothing
  local trigger Trig = CreateTrigger()
  call TriggerRegisterAnyUnitEventBJ(Trig, EVENT_PLAYER_UNIT_SPELL_EFFECT)
  call TriggerAddCondition(Trig, Condition(function condition))
  call TriggerAddAction(Trig, function action)
  call Preload("Abilities\\Spells\\Undead\\Impale\\ImpaleHitTarget.mdl")
  set Trig = null
endfunction
endscope

Attached Files

Spell Template.w3x (129.5 KB)

12-15-2008, 01:42 AM	#2
PandaMine	So what do you need exactly?

12-15-2008, 01:44 AM	#3
Leopard	I need you to check my code if it's alright or not... -I also mentioned my ideas so that you can see what i am trying to do...

12-15-2008, 01:50 AM

PandaMine

Well ok for starters

JASS:

  set bj_forLoopAIndex = 1
  set bj_forLoopAIndexEnd = 4
    loop
        exitwhen bj_forLoopAIndex > bj_forLoopAIndexEnd

and

JASS:

        set X = GetLocationX(D.loc) + D.DIST * Cos(D.ANG + ( 90.00 * I2R(GetForLoopIndexA())) * bj_DEGTORAD)
        set Y = GetLocationY(D.loc) + D.DIST * Sin(D.ANG + ( 90.00 * I2R(GetForLoopIndexA())) * bj_DEGTORAD)

No idea why you are using blizzards globals, you can just use local variables for the loop counters?

Apart from that it looks ok, do you get any syntax errors?

12-15-2008, 02:19 AM	#5
Leopard	Thanks Panda, since you have pointed out some problem, can you please include solutions. Also, do you feel that the experience system will work well? in this case if the target unit die, i count EXP = EXP+1 then check. For so many if/then/else i wonder it will work without any bug ? -EDIT: i do not encounter any error -And what should i do if i want the loop to be ended after 1 seconds ?

12-15-2008, 02:20 AM

Zerzax

It seems to be very heavy on the looping + conditions + a loop + more conditions, which can be pretty expensive. It's obviously not a big deal if you are experimenting, though. Try exploring methods and static declarations, they help the code out. Heck I don't even declare functions now, just methods.

EDIT: Testing yourself is the best way Leopard, if you find a specific problem we can pinpoint then that is a start.

12-15-2008, 02:26 AM	#7
Leopard	Thanks Zerzax, thats my problem, the spell may work well but i feel my code is not neat enough, i think that there is always a better way to do it....but i still have not known it,... :)

12-16-2008, 12:22 AM

Zerzax

Okay, I'll give some tips. It looks like you are mixing user-defined globals from the variable editor with those in your globals block. If I were you, unless it would wreck any GUI you are using in your map, consolidate all of the globals that pertain to your spell to that globals block or one that all of your spells / scripts share. Also, I noticed you are nulling your data location, that's not absolutely necessary with struct handles because those handles will get recycled the next time someone casts the spell (with that exact struct index, that is). I've seen examples of attachment systems in which you do need to null those handles, but that doesn't seem to be the case here. There seems to be some variation in how you indent your code (sometimes no indent, sometimes a few spaces, sometimes a full indent). Try to make all of your code indented the same way to give it regularity and make it look less messy, I prefer a full tab for each indent, but that's just me.

Also, there is a rare bug that occurs when you null timers, so I think you might want to try using your timer as a member of data. That way, you need not declare it as a variable, so when it comes time to release your timer you can just call ReleaseTimer(D.timer).

You should probably make your resurrection effects a constant in your globals block too.

EDIT: You don't need to null trig in your init trig function.

12-16-2008, 12:55 AM

Leopard

Nice Zerzax...And, i have just found out some problems, as Panada said, i should really not use that way of looping, instead, i can use:

JASS:

local integer i
set i = 1

loop
exit when i>4
set i = i+1

Also, i think storing location is not good, in this case, that should be corodinate...

JASS:

static method create takes unit c, integer i, real r, real x real yl returns Data

It's said that noone tried to help me that way, maybe except Zet and Panda (they did take a look at my code)

12-16-2008, 01:09 AM	#10
Zerzax	To be honest Leopard, most people will not just fix your code for you nor read through it, but the nice ones will go out of their way for it. Also, if you help out other people, they will help you as you probably know. (Hint: give Panda some rep). And yes, coordinates are better.