wc3campaigns
WC3C Homepage - www.wc3c.netUser Control Panel (Requires Log-In)Engage in discussions with other users and join contests in the WC3C forums!Read one of our many tutorials, ranging in difficulty from beginner to advanced!Show off your artistic talents in the WC3C Gallery!Download quality models, textures, spells (vJASS/JASS), systems, and scripts!Download maps that have passed through our rigorous approval process!

Go Back   Wc3C.net > Warcraft III Modding > Developer's Corner > Triggers & Scripts
User Name
Password
Register Rules Get Hosted! Chat Pastebin FAQ and Rules Members List Calendar



Reply
 
Thread Tools Search this Thread
Old 07-13-2009, 10:06 AM   #1
Michael Peppers
Lepus?
 
Michael Peppers's Avatar
 
Join Date: Jan 2009
Posts: 1,308

Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)

Default Criticism needed

So... I'm coding a bit in vJass in this period and I'm quite happy with the results of my spells, but I'm pretty sure that I'm always missing something in order to improve the code. (fix leaks etc.)

Tell me how can I improve this spell:

EDIT: MUIed

Collapse Aura of Decay:
scope AuraofDecay initializer Init

globals
    private constant integer HeroId = 'H001'
    private constant integer Aura = 'A00C'
    private constant integer AuraStatIcon = 'A00A'
    private constant integer PosBuff = 'A00B'
    private constant integer NegBuff = 'A005'

    private unit array Hero
    private integer Heroes = 0
    private unit H = null
    private player UP = null
    private group ENUM
endglobals

private function EnemyDeathKnights takes nothing returns boolean
return (IsUnitInGroup(GetFilterUnit(), GetUnitsOfTypeIdAll(HeroId))) and (IsUnitEnemy(GetFilterUnit(), UP)) and (GetUnitAbilityLevel(GetFilterUnit(), Aura) != 0)
endfunction

private function AlliedDeathKnights takes nothing returns boolean
return (IsUnitInGroup(GetFilterUnit(), GetUnitsOfTypeIdAll(HeroId))) and (IsUnitAlly(GetFilterUnit(), UP)) and (GetUnitAbilityLevel(GetFilterUnit(), Aura) != 0)
endfunction

private function BuffRemove takes nothing returns boolean
set UP = GetOwningPlayer(GetFilterUnit())
if(GetUnitAbilityLevel(GetFilterUnit(), NegBuff) != 0) then
    call GroupEnumUnitsInRangeCounted(ENUM, GetUnitX(GetFilterUnit()), GetUnitY(GetFilterUnit()), 900.00, Condition(function EnemyDeathKnights), 1)
    if (CountUnitsInGroup(ENUM) == 0) then
        call UnitRemoveAbility(GetFilterUnit(), NegBuff)
    endif
    set ENUM = null
endif
if (GetUnitAbilityLevel(GetFilterUnit(), PosBuff) != 0) then
    call GroupEnumUnitsInRangeCounted(ENUM, GetUnitX(GetFilterUnit()), GetUnitY(GetFilterUnit()), 900.00, Condition(function AlliedDeathKnights), 1)
    if (CountUnitsInGroup(ENUM) == 0) then
        call UnitRemoveAbility(GetFilterUnit(), PosBuff)
    endif
    set ENUM = null
endif
return false
endfunction

private function Group takes nothing returns boolean
local unit u = GetFilterUnit()
if (IsUnitEnemy(u, GetOwningPlayer(H))) and (GetWidgetLife(u) > 0.405) and (IsUnitType(u, UNIT_TYPE_MAGIC_IMMUNE) == false) and (IsUnitType(u, UNIT_TYPE_MECHANICAL) == false) and (IsUnitType(u, UNIT_TYPE_ANCIENT) == false) then
    if (GetUnitAbilityLevel(u, NegBuff) < (GetUnitAbilityLevel(H, Aura))) then
        call UnitAddAbility(u, NegBuff)
        call SetUnitAbilityLevel(u, NegBuff, (GetUnitAbilityLevel(H, Aura)))
    endif
    call UnitDamageTarget(H, u, (GetUnitAbilityLevel(H, Aura)), true, true, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_DEATH, WEAPON_TYPE_WHOKNOWS)
    call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\Undead\\DeathandDecay\\DeathandDecayDamage.mdl", u, "chest"))
elseif (IsUnitAlly(u, GetOwningPlayer(H))) and (IsUnitType(u, UNIT_TYPE_SUMMONED)) then
    if (GetUnitAbilityLevel(u, PosBuff) < (GetUnitAbilityLevel(H, Aura))) then
        call UnitAddAbility(u, PosBuff)
        call SetUnitAbilityLevel(u, PosBuff, (GetUnitAbilityLevel(H, Aura)))
    endif
    call SetUnitState(u, UNIT_STATE_LIFE, (GetUnitState(u, UNIT_STATE_LIFE) + 1))
endif
set u = null
return false
endfunction

private function DecayCheck takes nothing returns boolean
local integer n = 0
set ENUM = CreateGroup()
loop
    exitwhen (n == Heroes)
    if (Hero[n] != null) then
        set H = Hero[n]
        call GroupEnumUnitsInRange(ENUM, GetUnitX(Hero[n]), GetUnitY(Hero[n]), 900.00, Condition(function Group))
    endif
    set n = n + 1
endloop
call GroupEnumUnitsInRect(ENUM, bj_mapInitialPlayableArea, Condition(function BuffRemove))
call DestroyGroup(ENUM)
set H = null
return false
endfunction

private function AuraofDecayMain takes nothing returns boolean
    if (GetLearnedSkill() == Aura)  then
        if (GetUnitAbilityLevel(GetLearningUnit(), Aura) == 1) then
            set Hero[Heroes] = GetLearningUnit()
            call UnitAddAbility(GetLearningUnit(), AuraStatIcon)
        set Heroes = Heroes + 1
        else
            call SetUnitAbilityLevel(GetLearningUnit(), AuraStatIcon, (GetUnitAbilityLevel(GetLearningUnit(), Aura)))
        endif
    endif
return false
endfunction

private function Removal takes nothing returns boolean
    if (GetItemTypeId(GetManipulatedItem()) == 'tret') then
        call UnitRemoveAbility(GetManipulatingUnit(), Aura)
        call UnitRemoveAbility(GetManipulatingUnit(), AuraStatIcon)
    endif
return false
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger t1 = CreateTrigger()
    local trigger t2 = CreateTrigger()
    local trigger t3 = CreateTrigger()

    call TriggerRegisterAnyUnitEventBJ(t1, EVENT_PLAYER_HERO_SKILL)
    call TriggerAddCondition(t1, Condition(function AuraofDecayMain))

    call TriggerRegisterTimerEvent(t2, 0.50, true)
    call TriggerAddCondition(t2, Condition(function DecayCheck))

    call TriggerRegisterAnyUnitEventBJ(t3, EVENT_PLAYER_UNIT_USE_ITEM)
    call TriggerAddCondition(t3, Condition(function Removal))

endfunction

endscope
__________________
Projects:Tutorials: Competitive AI Step by Step with AI Editor (Pending)
Resources: [AI Script] Michael Peppers's Melee AI template (Pending)

Last edited by Michael Peppers : 07-13-2009 at 08:32 PM.
Michael Peppers is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 07-13-2009, 11:31 AM   #2
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

It's not MUI.
__________________
Anitarf is offline   Reply With Quote
Old 07-13-2009, 11:44 AM   #3
Michael Peppers
Lepus?
 
Michael Peppers's Avatar
 
Join Date: Jan 2009
Posts: 1,308

Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)

Default

Quote:
Originally Posted by Anitarf
It's not MUI.

It's MPI, though. As a unique hero spell, it can work this way too.

Anyway, thoughts on making it MUI?

EDIT: Nevermind. I just had an idea :P

EDIT 2: Done. MUIed.
__________________
Projects:Tutorials: Competitive AI Step by Step with AI Editor (Pending)
Resources: [AI Script] Michael Peppers's Melee AI template (Pending)

Last edited by Michael Peppers : 07-13-2009 at 11:58 AM.
Michael Peppers is offline   Reply With Quote
Old 07-13-2009, 03:34 PM   #4
Hans_Maulwurf
root bloody root
 
Hans_Maulwurf's Avatar
 
Join Date: Mar 2007
Posts: 117

Hans_Maulwurf will become famous soon enough (52)Hans_Maulwurf will become famous soon enough (52)

Send a message via ICQ to Hans_Maulwurf
Default

you leak GetUnitLoc(Hero[n]). why dont use the x,y function?
and you are enuming and destroying the local groups in BuffRemove without ever creating them. does this even work oO? also why are you using two different groups? you should use a global group there anyway
using a trigger just for DecayCheck seems a bit overkill to me. but i dont know if it would be any better to make a periodic timer run that function instead of TriggerRegisterTimerEvent so.. meh
__________________

Suit up! Tonight is gonna be legen... wait for it... DARY!
Hans_Maulwurf is offline   Reply With Quote
Old 07-13-2009, 03:49 PM   #5
Michael Peppers
Lepus?
 
Michael Peppers's Avatar
 
Join Date: Jan 2009
Posts: 1,308

Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)

Default

Quote:
Originally Posted by Hans_Maulwurf
you leak GetUnitLoc(Hero[n]). why dont use the x,y function?

Fixed. Thanks.

Quote:
Originally Posted by Hans_Maulwurf
and you are enuming and destroying the local groups in BuffRemove without ever creating them. does this even work oO?

Yep, I've created them :)

Quote:
Originally Posted by Hans_Maulwurf
also why are you using two different groups? you should use a global group there anyway

One is for the allied units, one for the enemy units.

Quote:
Originally Posted by Hans_Maulwurf
using a trigger just for DecayCheck seems a bit overkill to me. but i dont know if it would be any better to make a periodic timer run that function instead of TriggerRegisterTimerEvent so.. meh

Allright, thanks again for the feedback.
__________________
Projects:Tutorials: Competitive AI Step by Step with AI Editor (Pending)
Resources: [AI Script] Michael Peppers's Melee AI template (Pending)

Last edited by Michael Peppers : 07-13-2009 at 03:54 PM.
Michael Peppers is offline   Reply With Quote
Old 07-13-2009, 05:19 PM   #6
Viikuna-
User
 
Viikuna-'s Avatar
 
Join Date: Feb 2009
Posts: 203

Viikuna- will become famous soon enough (44)Viikuna- will become famous soon enough (44)

Default

Your group usage is pretty bad, you should not create&destroy them like that. Use a global group for calling GroupEnums and do your stuff in filterfunction.

Getting GroupUtils is not a bad idea, neither is reading that GroupUtils thread.
__________________
No Marlo, no game.
Viikuna- is offline   Reply With Quote
Old 07-13-2009, 05:36 PM   #7
Michael Peppers
Lepus?
 
Michael Peppers's Avatar
 
Join Date: Jan 2009
Posts: 1,308

Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)

Default

Quote:
Originally Posted by Viikuna-
Your group usage is pretty bad, you should not create&destroy them like that. Use a global group for calling GroupEnums

Is that a performance issue? Or only fancy code habits? :D

Quote:
Originally Posted by Viikuna-
and do your stuff in filterfunction.

Uhm... well... I do, when I can, but sometimes it's not possible.

Quote:
Originally Posted by Viikuna-
Getting GroupUtils is not a bad idea, neither is reading that GroupUtils thread.

Okie, I'll look into it.
__________________
Projects:Tutorials: Competitive AI Step by Step with AI Editor (Pending)
Resources: [AI Script] Michael Peppers's Melee AI template (Pending)

Last edited by Michael Peppers : 07-13-2009 at 05:38 PM.
Michael Peppers is offline   Reply With Quote
Old 07-13-2009, 06:07 PM   #8
Viikuna-
User
 
Viikuna-'s Avatar
 
Join Date: Feb 2009
Posts: 203

Viikuna- will become famous soon enough (44)Viikuna- will become famous soon enough (44)

Default

Creating and destroying handles is slow, so its better to recycle them when you can.

( Or just use one global group for all GroupEnum, one global dummy caster for adding all buffs, one global timer for all knockbacks... )

Also, if I remember right, some guy in that GroupUtils thread said that GroupEnum leaks with temporary groups.
__________________
No Marlo, no game.

Last edited by Viikuna- : 07-13-2009 at 06:08 PM.
Viikuna- is offline   Reply With Quote
Old 07-13-2009, 07:40 PM   #9
Michael Peppers
Lepus?
 
Michael Peppers's Avatar
 
Join Date: Jan 2009
Posts: 1,308

Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)Michael Peppers is a jewel in the rough (188)

Default

Thanks. Check your rep if you didn't yet
__________________
Projects:Tutorials: Competitive AI Step by Step with AI Editor (Pending)
Resources: [AI Script] Michael Peppers's Melee AI template (Pending)
Michael Peppers is offline   Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off


All times are GMT. The time now is 04:14 PM.


Affiliates
The Hubb The JASS Vault Clan WEnW Campaign Creations Clan CBS GamesModding Flixreel Videos

Powered by vBulletin (Copyright ©2000 - 2019, Jelsoft Enterprises Ltd).
Hosted by www.OICcam.com
IT Support and Services provided by Executive IT Services