Aspard said:

Why the units in a target are setted on fire? Isn't it water spell? The dummy units attack enemies (including caster since they are spawned for neutrals, huh?) so we can hear attack sounds! They even have damage, wtf? The spells damages friendly units and even the caster himself. There is no single helpful command to test the spell, even the hero doesn't revive.

Why does the water spell kills destructibles?

This spell bugs at point of OBJECT EDITOR more than at triggering itself. You've chosen dull dummy ability and don't know how to work with dummy units. You even has three of them instead of one!

If I could, I'll rate it 1/5. But now I can just vote for rejection. There is a "Triggers&Scripts" section on forum for WIPs.

Click to expand...

Demongrip said:

Don`t delete it just fix the things mentioned and update it.

Click to expand...

• Set (Spell_DamageGroup) equal to Unit Group - Pick All Units in (Region / Range / etc) matching conditions (picked unit*) is an enemy of (Caster_Unit) equal to true and (other conditions)

FlowerofSpeech said:

That's rather easy..

• Set (Spell_DamageGroup) equal to Unit Group - Pick All Units in (Region / Range / etc) matching conditions (picked unit*) is an enemy of (Caster_Unit) equal to true and (other conditions)

*May be matching unit, it should say somewhere which one you need in grey text

You need to work the above trigger in somewhere in your spell.

Click to expand...

call ForGroupBJ( GetUnitsInRangeOfLocMatching(400.00, Pos, Condition(function Trig_damage_conditions)), function Trig_damage )// this is in the main function somewhere

function Trig_damage_conditions takes nothing returns boolean
    return ( GetOwningPlayer(GetEnumUnit()) != GetOwningPlayer(udg_Caster) )
endfunction

Roflcoptor said:

omfg

just seeing the script makes me.....

it leaks locations, what makes me want to ask, why the hell you even use those..


99% is just GUI-converted...


triggering is awful.

Read tutorials adn practice pls

Click to expand...

Roflcoptor said:

I dont care if it is your first or hundreth spell. my first spell didnt contain any leaks.

Spell section is made for finished full working spells.

PolarProjectionBJ() leaks

and you don't clear point leak

JASS:

local location point = Get.....
.....
call RemoveLocation(point)
set point = null

is correct.

Click to expand...

Description said:

Still trying to fix so the spell only damages enemies of caster.

Click to expand...

function Trig_damage_conditions takes nothing returns boolean
    return ( GetOwningPlayer(GetEnumUnit()) != GetOwningPlayer(udg_Caster) )
endfunction

function Trig_damage_conditions takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(udg_Caster) )
endfunction

function Trig_Spell_Init_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A002'
endfunction

• You should seriously think about getting JNGP and making use of vJass.
• Fix all of the BJ's except for TriggerRegisterAnyUnitEventBJ. I'm sure there is a tutorial on inlining somewhere.
• Your local location Pos leaks, you need to RemoveLocation it.
• Replace all of the global bj_forLoopAIndex with a local variable.
• Use timers instead of waits, this could complicate your code a little but it's definatly worth it.
• I think you should make some more practice spells first, and asking friends for help or posting a thread in the triggers and scripts forum.

Thanathos said:

I know why it damages alley units aswell.

Jass

JASS:

function Trig_damage_conditions takes nothing returns boolean
    return ( GetOwningPlayer(GetEnumUnit()) != GetOwningPlayer(udg_Caster) )
endfunction

should be changed to:

Jass

JASS:

function Trig_damage_conditions takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(udg_Caster) )
endfunction

Why don´t you merge the filter function with the action (damage) function ?
This would make it more effecient.

And by the way, your spell condition function is converted from GUI. It should look something like this.

Jass

JASS:

function Trig_Spell_Init_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A002'
endfunction

Oh and another advice. You should make some constant functions for the rawcodes. It´s just pain to go through the script and change them by hand.

Click to expand...

TriggerHappy187 said:

• You should seriously think about getting JNGP and making use of vJass.
• Fix all of the BJ's except for TriggerRegisterAnyUnitEventBJ. I'm sure there is a tutorial on inlining somewhere.
• Your local location Pos leaks, you need to RemoveLocation it.
• Replace all of the global bj_forLoopAIndex with a local variable.
• Use timers instead of waits, this could complicate your code a little but it's definatly worth it.
• I think you should make some more practice spells first, and asking friends for help or posting a thread in the triggers and scripts forum.

Click to expand...

• You still have a bunch of BJ's.
• Your leaking a bunch of locations (every PolarProjectionBJ)
• This is what your script should look like without the BJ's
  
  JASS:

      loop
          exitwhen i[2] > i[3]
          set x = GetSpellTargetX() + 100 * Cos(60.00 * I2R(i[2]) * bj_DEGTORAD)
          set y = GetSpellTargetY() + 100 * Sin(60.00 * I2R(i[2]) * bj_DEGTORAD)
          call CreateUnit(GetOwningPlayer(Caster), 'h002', x, y, 270)
          set i[2] = i[2] + 1
      endloop

• You should make the 'h002' configurable.
• You don't need an integer array for the loops, you can just use the same integer over and over, just reset it to 0 each time like this;
  
  JASS:

  local integer i = 0
  loop
      exitwhen i == 5
      set i = i + 1
  endloop
  set i = 0
  loop
      exitwhen i == 3
      set i = i + 1
  endloop

TriggerHappy187 said:

• You still have a bunch of BJ's.
• Your leaking a bunch of locations (every PolarProjectionBJ)
• This is what your script should look like without the BJ's
  
  JASS:

      loop
          exitwhen i[2] > i[3]
          set x = GetSpellTargetX() + 100 * Cos(60.00 * I2R(i[2]) * bj_DEGTORAD)
          set y = GetSpellTargetY() + 100 * Sin(60.00 * I2R(i[2]) * bj_DEGTORAD)
          call CreateUnit(GetOwningPlayer(Caster), 'h002', x, y, 270)
          set i[2] = i[2] + 1
      endloop

• You should make the 'h002' configurable.
• You don't need an integer array for the loops, you can just use the same integer over and over, just reset it to 0 each time like this;
  
  JASS:

  local integer i = 0
  loop
      exitwhen i == 5
      set i = i + 1
  endloop
  set i = 0
  loop
      exitwhen i == 3
      set i = i + 1
  endloop

Click to expand...

• You can give locals inital values ( local unit caster = GetTriggerUnit()).
• Why do you have two globals of type real and integer? Just use locals so there will be no global dependancy on your spell. People will just be able to copy and past the code without worrying about variables.
• This is how group enums should work in JASS without using the BJ's
  
  JASS:

  function spell_filter takes nothing returns boolean
      return GetUnitTypeId(GetFilterUnit()) == 'h002'
  endfunction
  
  local group g = CreateGroup()
  call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Filter(function spell_filter))
  call ForGroup(g)

  You can see what the BJ does by holding control and clicking the red letters in Jass New Gen.

Deaod said:

Welcome to the city of JASS. Please enjoy your stay. To avoid serious injuries (such as brain damage) during your time in the world of WarCraft 3 modding, please proceed to vJass city. It's located straight ahead.

A few things to remember:
- destroying groups leaks (thank Blizzard for that)
- you must not use constants directly within your code, for this makes maintaining code a nightmare
- TriggerSleepAction() is not a good way to offset actions (because it does not pause when the "Waiting for player" dialog popps up; And because its inaccurate).
- I2R is (almost) never needed. (v)Jass supports implicit downcasting (every integer is also a real, but not every real is an integer).

Click to expand...