Forestborn 2.1b

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.


11 Nov 2011
Bribe: You forgot to call "deallocate" from the "destroy" method in struct "Data". It leaks.

You still need to get rid of "EnumDestructablesInCircleBJ", if you look at JassHelper's API database you will see how to make it yourself.

Instead of setting HEAL_BASE and HEAL_LVL to 0, make a constant boolean that says "HEAL_ENABLED" which is compiled as a static-if, so if it is unwanted the code is not generated either.

In your "init" you actually need to add the ability, you can't just set the ability level.

 

[Magtheridon96]

call EnumDestructablesInCircleBJ(AOE, l, function TreeCheck) //Picks every destructable in range
You should get rid of that

edit
You should also change the scope to a library (to make the requirements explicit)
And while you're at it, remove that initializer, delete the "init" function and just C'n'P this at the end of your script

private module Init
    private static method onInit takes nothing returns nothing
        local unit u
        set ForestbornTrigger  = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(ForestbornTrigger, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition(ForestbornTrigger, Condition(function Actions))
        set u = CreateUnit(Player(15), 'Hpal', 0., 0., 0.)  //Preload ability
        call UnitAddAbility(u, BOOK_ID)
        call SetUnitAbilityLevel(u, MOVE_ID, MOVE_BASE)
        call SetUnitAbilityLevel(u, EVADE_ID, EVADE_BASE+1)
        call RemoveUnit(u)
        set u = null
    endmethod
endmodule
private struct Inits extends array
    implement Init
endstruct

Or, If you want, just implement that module directly into your Data struct

 

[Tommerbob]

call EnumDestructablesInCircleBJ(AOE, l, function TreeCheck) //Picks every destructable in range
You should get rid of that

The native that BJ calls runs through several checks anyway, so isn't calling the BJ function pretty much just as fast? Similar to comparing it to the "TriggerRegisterAnyUnitEventBJ"?

You should also change the scope to a library (to make the requirements explicit)

What do you mean by making the requirements explicit?

And while you're at it, remove that initializer, delete the "init" function and just C'n'P this at the end of your script

private module Init
    private static method onInit takes nothing returns nothing
        local unit u
        set ForestbornTrigger  = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(ForestbornTrigger, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition(ForestbornTrigger, Condition(function Actions))
        set u = CreateUnit(Player(15), 'Hpal', 0., 0., 0.)  //Preload ability
        call UnitAddAbility(u, BOOK_ID)
        call SetUnitAbilityLevel(u, MOVE_ID, MOVE_BASE)
        call SetUnitAbilityLevel(u, EVADE_ID, EVADE_BASE+1)
        call RemoveUnit(u)
        set u = null
    endmethod
endmodule
private struct Inits extends array
    implement Init
endstruct

Or, If you want, just implement that module directly into your Data struct

I'm still new to vJass, whats a module? Whats wrong with how I'm already doing it? How is your way faster or better?

 

[Bribe]

Magtheridon, requiring everyone to initialize spells from modules is a bit overkill. Systems, yeah, but little spells don't need such micromanagement.

 

[Adiktuz]

this is certainly useful... just fix those they said above... ^_^

 

[Black-kitty]

Very useful model, good in detail and the way it is preformed by different hero's. lol good job.

 

[Adaethus]

It's not a model -.-

 

[Tommerbob]

@ Bribe:

- Don't use == true or == false, use "boolean" or "not boolean" respectively.

Why? What's wrong with using "== true" or "== false" ? Could you give me an example of how to use "boolean" or "not boolean" in this spell, I'm kinda confused.

I just got very busy with a few things in real life, will update as soon as I can.

 

[Bribe]

These two lines:

    if IsDestructableTree(dest) == true then
        if IsDestructableDead(dest) != true then

Can be written as:

    if IsDestructableTree(dest) then
        if not IsDestructableDead(dest) then

 

[Adiktuz]

You see, booleans already return a boolean so why use == true or == false... its like checking if true == true, which is a waste of time...

 

[Tommerbob]

*Updated*

Made all the suggested changes.

I tried for about 2 hours to inline that EnumDestructibleBJ call, but I can't figure it out. Honestly, I don't think it will be that much faster than simply calling the BJ function. Anyway, for the time being I am leaving it as it is. Enjoy.

 

[watermelon_1234]

The spell is pretty nice. However, the effect disappears if the caster moves, regardless of CAN_MOVE.

• I think it would be easier if you just made the user rely on configuring the functions instead of using the globals (I'm talking about Duration, Heal, Evasion, and MoveBonus).
• The tree and hassfx members in Data can be initialized to false like globals so you don't need to set them in create.
• You really should work on inlining EnumDestructablesInCircleBJ since it's pretty wasteful.
  I'll get you started a bit:
  

  globals
      private rect TempRect = Rect(0, 0, 0, 0) // Just use on rect so you don't need to remove it all the time
  endglobals
  ...
  // When you call that function:
  local real x = GetUnitX(data.caster)
  local real y = GetUnitY(data.caster)
  call SetRect(TempRect, x - AOE, y - AOE, x + AOE, y + AOE)
  call EnumDestructablesInRect(TempRect, Filter(function EnumDestructablesInCircleBJFilter), function TreeCheck)

  Now all you need to do is inline EnumDestructablesInCircleBJFilter by making your own filter function.
• You can use a static if for CAN_MOVE.
• The way you set up TRANSPARENCY is rather confusing. Why don't you just let the user specify the exact alpha value they want instead of subtracting their value by 100?
• The duration check for the spell should be made at the beginning of NearTree rather than at the end.
• For Actions, you could have made the condition GetSpellAbilityId() == SPELL_ID so you could put everything else in there. It looks better in my opinion.
• SetPlayerAbilityAvailable should have been done in init for every player rather than doing it when the spell is cast.
• I don't understand why you set the levels for the movement speed ability and evasion ability for the dummy unit when you're preloading the spellbook ability.