Agro Roar v 1.2.0

[Moderator]

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

Needs Fix

Required changes:

• You are creating lot of triggers and never destroying them
• RoarGroupIssue uses bj_lastStartedTimer, not good

Suggested changes:

• You can delete set bj_lastStartedTimer = t. That is GUI stuff and not needed here
• I recommed you reduce the level count of the ability to the maximum armor bonus you can have. It decreases loading time
• Instead of eg group, you could use bj_lastCreatedGroup
• Set the struct member g to null after destroying it
• Filter out structures

Old review

Needs Fix

• Using globals block makes it vjass
• You clear the global group when any instance of the spell ends, thus the sepll will not work several spellcasters
• There is no importing instructions in the map file
• The spell is not easily configurable
• You don't need three separate abilities for the armor

 

[zv27]

What if I wanted to change something here?
It would be very annoying.

Here are some tips:

//This is unnecessary
//So it's not wrong, but it is unnecessary in this case
unit cast
//Use Hashtable - SaveAgentHandle or SaveUnitHandle = LoadUnitHandle

GetUnitAbilityLevel(caster,'A000')
->
constant function ExempleSomeName takes nothing returns integer
    return 'A000'/'A001'/'A002' and so on...
endfunction

or global

integer AbilCode = 'A000'/'A000'/'A001'/'A002' and so on...

GetUnitAbilityLevel( caster, ExempleSomeName()/AbilCode )

local group g = CreateGroup()
call DestroyGroup(g)
set g = null
->
bj_lastCreatedGroup

Example

call TimerStart( t, [5.+lvl "should be configurable"], false,function RemoveAbility )
->
function ExampleSomeName takes integer level returns real
    return 5. + level
endfunction
or
function ExampleSomeName takes real level returns real
    return 5. + level
endfunction

call TimerStart( t,  ExampleSomeName(), false,function RemoveAbility )

or again GLOBALS bla bla

Suggested changes:

function Trig_AgroRoar_Actions takes nothing returns nothing
endfunction
------->
function Trig_AgroRoar_Actions takes nothing returns boolean
    if ( GetSpellAbilityId() ) == ExampleSomeAbility() then
        bla,bla
    endif
    return false
endfunction

function InitTrig_AgroRoar takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_AgroRoar_Actions ) )
    set t = null
endfunction

All of this can and should be optimized

function Issued takes nothing returns nothing
local unit attacker = GetTriggerUnit()
if IsUnitInGroup(attacker,gr) == true then
    call IssueTargetOrderById(attacker,851983,cast)
endif
set attacker = null
endfunction

function AddHeroArmor takes unit u,integer i returns nothing
if i == 1 then
    call UnitAddAbility(u,'A001')
elseif i == 2 then
    call UnitAddAbility(u,'A002')
elseif i == 3 then
    call UnitAddAbility(u,'A003')
endif
endfunction

function SetHeroArmor takes unit u,integer i returns nothing
if i == 1 then
    if GetUnitAbilityLevel(u,'A001') <= 13 then
        call SetUnitAbilityLevel(u,'A001',GetUnitAbilityLevel(u,'A001')+1)
    endif
endif
if i == 2 then
    if GetUnitAbilityLevel(u,'A002') <= 25 then
        call SetUnitAbilityLevel(u,'A002',GetUnitAbilityLevel(u,'A001')+1)
    endif
endif
if i == 3 then
    if GetUnitAbilityLevel(u,'A003') <= 37 then
        call SetUnitAbilityLevel(u,'A003',GetUnitAbilityLevel(u,'A003')+1)
    endif
endif
endfunction

 

[tRu.Style]

thank you but I don't like to use global variables
fxd all, except Issue
As you are now? You can change all *_*

 

[Almia]

Fix indention please

also,linked lists for structs is better

 

[tRu.Style]

in next version,okey?

 

[Maker]

I'm waiting until you update this to v1.0 or close to that before reviewing it again. If it is v0.3, then you are obviously going to update it a lot still.

http://www.hiveworkshop.com/forums/faq/resourcerules/=/

Resources must be completed. For works in progress, post in the relevant forum, or the Pastebin.

 

[tRu.Style]

Now approved?

 

[BunnyAng]

Hmm. I played Tidehunter before but can you please add a description? It has only a level of ability, whereas DotA had 3. Tested the spell and found a few problems.

1st: The spell is a no target spell, why make it into a target spell?
2nd: It teleports you to where you cast it, which is not what i experienced in DotA while playing as Tidehunter.
3rd: Once i casted the spell, the spell doesn't work until the units stunned by the spell wakes up again.

Not sure if anyone found the same, but this is what i found. Please update this spell asap.

 

[tRu.Style]

Hehe, you are not there to unsubscribe