Twister and Storm Spiral v1.5

[Moderator]

09:32, 27th May 2010
TriggerHappy:

Overall, it's good enough to get approved.

• Don't inline the spell event.
• Instead of group recycling you could be using a global group.
• You should find an alternative to FOG loops.

 

[watermelon_1234]

I have not tested these spells yet; I've only checked them online. I also apologize if my code looks strange since I'm on a different computer.

• if GetUnitFlyHeight(e) < 500 then

  Shouldn't 500 be configurable? You should also make any other numbers like that easier to change by using constant globals or functions. (Like damage for instance)

• call PauseUnit( e, true )

  Pausing a unit isn't really good. Besides that, it's a waste to declare e when you use it once. Just use GetEnumUnit() directly.
• Instead of

  local boolean ok = falsei
  if IsUnitEnemy(u, GetOwningPlayer(d.c)) /*
        */and     IsUnitVisible  (u, GetOwningPlayer(d.c)) /*
        */and not IsUnitType     (u, UNIT_TYPE_MAGIC_IMMUNE) /*
        */and not IsUnitType     (u, UNIT_TYPE_STRUCTURE) /*
        */and     GetWidgetLife  (u) > 0.405 /*
        */and not IsUnitInGroup  (u, d.g) then
              set ok = true
  endif

  you could just do

  local boolean ok =   IsUnitEnemy    (u, GetOwningPlayer(d.c)) and     IsUnitVisible  (u, GetOwningPlayer(d.c)) and not IsUnitType     (u, UNIT_TYPE_MAGIC_IMMUNE) and not IsUnitType     (u, UNIT_TYPE_STRUCTURE) and     GetWidgetLife  (u) > 0.405 and not IsUnitInGroup  (u, d.g)

  I also hope you know that GetWidgetLife(u) can bug and it would be better to use the native UnitAlive or check if a unit is alive with not IsUnitType(u,UNIT_TYPE_DEAD)
• Using a loop is much more efficient for creating your dummy units.
• It's better to set the variable as you declare them. Ex:

  local timer ti = NewTimer()

• You're using GroupUtils; it's better to use GroupEnumUnitsInArea instead of GroupEnumUnitsInRange. Make sure the library is updated to be able to use that function.
• You could preload your dummy unit.
• You don't need to inline TriggerRegisterAnyUnitEventBJ.
• This is kind of a standard: capitalizing constant global variables. It's not that important for you to do this but I thought I might as well as mention that.

I'll edit this post if I test these spells.

 

[game_war48]

1. Ok, I will use the globals for it.
2. Will remove pause. But I belive had someone tell me "Use GetEnumUnit() directly will leak and isn't faster than use variable instead". I will edit it when all things considered.
3. Oh, I do it deliberately. Because changing it is easier than do

local boolean ok =   IsUnitEnemy    (u, GetOwningPlayer(d.c)) and     IsUnitVisible  (u, GetOwningPlayer(d.c)) and not IsUnitType     (u, UNIT_TYPE_MAGIC_IMMUNE) and not IsUnitType     (u, UNIT_TYPE_STRUCTURE) and     GetWidgetLife  (u) > 0.405 and not IsUnitInGroup  (u, d.g)

And I will use not IsUnitType(u,UNIT_TYPE_DEAD) instead of GetWidgetLife.
4. Where ? Twister or Storm Spiral ? If it's Twister, I can't use loop for creating dummys because it doesn't according to any laws, it's just random.
5. Nothing to comment (of course, agreed with you).
6. Oh, I totally forgot it. Thanks you.
7. I will.
8. I really hate BJ, so I don't use that (sorry, just my taste ).
9. I will do it soon.

Over at all, thanks you so much .

EDIT: UPDATE v1.4.
EDIT2: UPDATE v1.5.

 

[PoWerJin]

im sry sir, but this map does not appear in the gamelist when i want to test it ,,,


FIX IT ,, -_-

 

[Lambdadelta]

You FIX it, it's a simple matter of moving it outside a diff folder or renaming it shorter. Also, why not just test from the WorldEditor?

 

[game_war48]

^^: I can test it normally, don't have any problem. Also, just do the things xBlackRose told.

Is your time at 8.15 AM (morning) ? I'm from Vietnamese and the time is 3:32 PM (afternoon). Well, what a long difference ! (Just a joke, nevermind it).

 

[Axarion]

Except for the spamming of really much sfx its really nice if you fix the above said things

 

[watermelon_1234]

Okay I tested the spells and the effects are generally nice though it does lag if you use them too much. (Most likely because of the mass special effects that was also mentioned by Axarion.)

• You need to compile the map since I had to save it myself before I could test it. (In other words, save it again with JNGP)
• I don't think you should have OID since it only provides information. Just take the order id you need for the spell and put it as a global in your spell. (You could still give credits to Cohadar though for providing them in the first place.)
• You didn't mention that your spells need MiscScript. If you made that library by yourself, I think you should also provide the code here as well.
• In addition to that, you may want to fix your spell descriptions since they don't state all the requirements they need. Example: Twister doesn't say it needs GroupUtils though it credits it.
• I want to see more things configurable in your spells. =P Such as the area of the spell and attacktype/damagetype/weapontype. Also, the number for the dummy unit creation should be configurable as well.
• You can still use a loop for certain aspects of your dummy unit creation. A formula could be derived by using modulo.
  

  local integer i = 0
  local integer mod
  local integer multi
  loop
      //Every fifth one, 90 should be negative, yes? However, i goes by 0 so we'll look for multiples of 4
      set mod = i - (i/4)*4
      if mod == 0 then //mod should be 0 if i is divisible by 4.
          set multi = -1    
      else
          set multi = 1    
      endif
      set d.d[i] = CreateUnit(GetOwningPlayer(d.c),DUMMYID,GetPPX(d.dx,d.h*d.hs[i],d.df+multi*90.),GetPPY(d.dy,d.h*d.hs[i],d.df+multi*90.),d.df+multi*90)
      call SetUnitZ(d.d[i],d.hb)
      call SetUnitVertexColor(d.d[i], 255, 255, 255, 0)
      //If i is divisble by 2 then we should set d.inc[i] to true. Else, set it to false
      set mod = i - (i/2)*2
      if mod == 0 then
          set d.inc[i] = true
      else
          set d.inc[i] = false
      endif
      exitwhen i == 5 //5 should be configurable
      set i = i + 1
  endloop

  I didn't do this for d.hs though since it seemed random as you said. It would be good if you could though since it makes it much easier to modify that spell.

 

[game_war48]

^I will fix all things you said. But the problem about loop, if I create dummys Twister with loop, the effects will not good like don't. Maybe I will do 2 version for Twister (configurable and not configurable) .

 

[Dj0z]

I can't write description for this spellpack because i can't think anything to write and I'm very bad in English

Honestly, i'm sure you can do it :/ i have no idea what the second spell does, for example, apart from damaging.

 

[laserdemon]

are you sure this spell works?