Cross of Heaven 1.2

[Moderator]

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

15:17, 14th Jun 2014
BPower:
review:
http://www.hiveworkshop.com/forums/spells-569/cross-heaven-1-2-a-251720/#post2541444
note: cool concept

 

[deathismyfriend]

Never use location in jass / vJass unless you are trying to get the Z height at the location.
Don't use SetUnitLocation. Use SetUnitX/Y

Remove the BJs. ( the red function calls above.)

All local handles need to be nulled at end of function.

This does not check if unit is alive accurately.
IsUnitAliveBJ
You need to do this.
GetWidgetLife( u) > .405 and not IsUnitType( u, UNIT_TYPE_DEAD) and UnitTypeId( u) != 0

Never use TriggerAddAction. Use TriggerAddCondition and run everything from there.

Take a look at my tutorial Converting GUI to efficient JASS
It will show you how to make efficient Jass code.

 

[EndOfWorld]

Nice to know those things, actually
Well, I will fix it tomorrow, I guess.
So I will use this function for checking if a unit is alive:

function IsUnitAlive takes unit u returns boolean
    return (GetWidgetLife(u) > .405 and not IsUnitType( u, UNIT_TYPE_DEAD) and GetUnitTypeId(u) != 0)
endfunction

Is it correct?

 

[deathismyfriend]

Yes that is correct.

 

[EndOfWorld]

Updated.
I guess I don't have to change

GetUnitLifePercent() 
and 
SetUnitLifeBJ()

even though they are BJ's, right?
I don't see a more efficient way without getting complicated too much.

 

[deathismyfriend]

Nope change them also. They are inefficient. Also calculating a units life percent is easy.
local real lifePercent = GetWidgetLife( unit) / GetUnitState( unit, UNIT_STATE_MAX_LIFE) * 100
and for setting units life.
call SetWidgetLife( unit, life)

 

[EndOfWorld]

Well then, updated that.

 

[deathismyfriend]

This

    if not HealsCasterToo then
        if (GetFilterUnit() == bj_lastCreatedUnit) then
            return false
        endif
    endif

Should be this.

    if not HealsCasterToo and GetFilterUnit() == bj_lastCreatedUnit then
        return false
    endif

Get rid of this BJ TriggerRegisterTimerEventPeriodic
In you actions function you use this multiple times. GetTriggerUnit()
You already store it in a variable. The variable should be used. Along with any other instances that you did this.

You use an old form of indexing / de-indexing that needs to be changed as this is inefficient and will hit op-limit. The new method of indexed arrays is in my tutorial Thins you should know when using triggers / GUI. The chapter on how to index.

 

[Wrda]

all TriggerRegisterTimerEventPeriodic( t2, 0.03)
call TriggerAddCondition( t2, Condition(function Loop))

You should be using call TimerStart(timer, 0.0312500, true, functionperiodic) on actions.

 

[zv27]

This is poor vJass...brrb...more like a bad(the way it is done) vanilla Jass

 

[BPower]

Most times spells should be a scope and not a library. This is such a case.

HolyCrossFunctions is not listed as requirement in the holycross library.

In actions this:

        set TempEffect = AddSpecialEffectTarget(EffectModel, Caster[Index], "origin")
//......
        call DestroyEffect(TempEffect)

should be call DestroyEffect(AddSpecialEffectTarget(EffectModel, Caster[Index], "origin")) In function actions: Store the ability level and the TriggerPlayer(Owner) into a local integer variable. In function loop: Either use one global location to determine the z value or you have to null the two local locations used. You have needless GetUnitX/Y function calls since the value is already stored into TempLoc2X/Y Handles of all types should be nulled once they are not used anymore. This also includes lightnings. [code=jass]function IsUnitAlive takes unit u returns boolean return (GetWidgetLife(u) > .405 and not IsUnitType( u, UNIT_TYPE_DEAD) and GetUnitTypeId(u) != 0) endfunction[/code] ---> [code=jass]function IsUnitAlive takes unit u returns boolean return not IsUnitType( u, UNIT_TYPE_DEAD) and GetUnitTypeId(u) != 0) endfunction[/code] or declare [icode=jass]native UnitAlive takes unit id returns boolean

The loop is running even though there is no active spell instance. Please ensure the timer is stopped once there is no active instance running.

Sometimes the lightinings are not created properly.