[vJASS] Holy Light (RPG style) V1.04.c4

[Moderator]

14:22, 19th Sep 2009
PurplePoot: What sets this enough apart from the melee game spell to make it worth a submission? It seems to me like it trades off bad mechanics (targeting mostly) for texttags and that's it.

 

[Vercas]

Ideas and suggestions are welcome to improve the spell / code.

Please be tough when commenting.

I hope a moderator will take an overview of this spell and probably approve it / tell me how to improve it.

 

[Deuterium]

Ok very simple vJass, I'm gonna test and get back to you...

 

[Vercas]

Nice. I really hoped you will give me a review.

 

[xXGuillermoXx]

works fine, and was what i need ty.. but cant copy in my map it says it has 43 errors help me plzz

 

[Deuterium]

Holy Light Edited

scope HolyLight initializer HLInit
//We have a scope now.

//Use globals, their easier to adjust, use them for everything,
//Ability Id, Damage, TagColoring, etc...
    globals
        
        //The raw code (ID) of the abillity.
        private constant integer AbilityIdIs = 'A001'
        
    endglobals

private function HLConditions takes nothing returns boolean
    return GetSpellAbilityId() == AbilityIdIs //See where I use the global?
endfunction

private function HLActions takes nothing returns nothing
    local real r = ( I2R(GetHeroInt(GetSpellAbilityUnit(), true)) * 2.00 )
    set bj_lastCreatedTextTag = CreateTextTag() //Creates a null(empty) text tag.
    //Why bj_lastCreatedTextTag? nothing important, just faster and zero leak ;) (although some people might disagree on this...)
    //Do your math instead of letting the PC do it ;)
    call SetTextTagVelocity(bj_lastCreatedTextTag, 0, .028) // '50.00' is the velocity. The text will only go up. I used ' * 0.071 / 128' to avoid using a BJ.
    call SetTextTagPermanent(bj_lastCreatedTextTag, false )
    call SetTextTagFadepoint(bj_lastCreatedTextTag, 3.00 )
    call SetTextTagLifespan(bj_lastCreatedTextTag, 5.50 )
    call SetTextTagVisibility(bj_lastCreatedTextTag, true )
    call SetTextTagPosUnit(bj_lastCreatedTextTag, GetSpellTargetUnit(), 30.00)
    if ( IsPlayerAlly(GetOwningPlayer(GetSpellTargetUnit()), GetOwningPlayer(GetSpellAbilityUnit())) == true ) then
        call SetWidgetLife( GetSpellTargetUnit(), ( GetUnitState( GetSpellTargetUnit(), UNIT_STATE_LIFE ) + r ) )
        call SetTextTagText(bj_lastCreatedTextTag, (I2S(R2I(r)) + "!" ), .018) //Avoided using a BJ.
        //Crowded Text tags are annoying, oh and you need to increase the size a bit :P
        call SetTextTagColor(bj_lastCreatedTextTag, 230, 230, 140, 255) //Used BJ's.
        //OK to turn percent to 255, simple "cross multiply" method... you know that eh?
    else
        call UnitDamageTarget( GetSpellAbilityUnit(), GetSpellTargetUnit(), r, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_DIVINE, WEAPON_TYPE_WHOKNOWS )
        call SetTextTagText(bj_lastCreatedTextTag, (I2S(R2I(r)) + "!" ), .018) //Avoided using a BJ.
        call SetTextTagColor(bj_lastCreatedTextTag, 230, 102, 140, 255) //Used BJ's.
    endif
endfunction //That's all!


//===========================================================================
// Now the basic trigger initialization.

private constant function AntiLeak takes nothing returns boolean
    return true
endfunction
//OK, TriggerRegisterPlayerUnitEvent leaks 16 times at map intialization, this avoids that

private function HLInit takes nothing returns nothing
    local trigger t = CreateTrigger()
    local filterfunc f = Filter(function AntiLeak)
    local integer i = 0
    loop
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_EFFECT, f)
        set i = i + 1
        exitwhen i >= 16
    endloop
    call TriggerAddCondition(t, Condition( function HLConditions ) )
    call TriggerAddAction(t, function HLActions )
    call DestroyFilter(f)
    set f = null
    set t = null
endfunction
endscope

Ok there were no problems with the script and it was well-done, yet, it could be improved, check out this... it says it all.

And oh, isn't this a bit too unoriginal!?

 

[Vercas]

Yeah... Maybe the idea could be 1/10 or less...

About the math stuff, I explained in the comments wich is the actual velocity / size of the text.

I will use your code and update the spell (and of course, credit you).

Why is "bj_lastCreatedTextTag" faster?

 

[Deuterium]

Well, but don't forget using the globals, use them for everything that you thing a user might want to edit

 

[Vercas]

Thanks for helping me.
I am going to update it in 10 minutes.

 

[xxdingo93xx]

Ok, since I always made the mistake by myself and noticed that it is wrong I will tell you about it ... xD
Don't nullify the trigger variable in the Init function. It is not a handle which is needed to be nullified, you can compare trigger variable leaks to integer leaks so basically nullifying makes it even slower.
I suggest to change it as fast as possible ^^

 

[Vercas]

Ok, I will no longer set the trigger variable to null.

 

[Deuterium]

Ok, since I always made the mistake by myself and noticed that it is wrong I will tell you about it ... xD
Don't nullify the trigger variable in the Init function. It is not a handle which is needed to be nullified, you can compare trigger variable leaks to integer leaks so basically nullifying makes it even slower.
I suggest to change it as fast as possible ^^

We're not nullifying the trigger/handle here, but we're nullifying the local variable which was holding it.
Nullifying isn't related to handles, that has DestroyBla()/RemoveBla().

What he does basically cleans up the local which would never be accessed after being used...

 

[Vercas]

I'll just not set it null...
I don't think it will hurt anyone.

I will update the spell again.

 

[Thanathos]

you can compare trigger variable leaks to integer leaks so basically nullifying makes it even slower.

Did i get that right ? Integers leak and if ints does what about the other primitives (handle is the exception) ?! Ehm and by nullifying handles you prevent leaks, which cause the game (if you are plenty of them) to slowdown.

 

[Cheezeman]

Hey deterium!
I've seen you teach that 'anti-leak event' for quite some time, and I thought this topic might interest you.
According to Antarif on wc3c (I think it was antarif), filters don't leak for events, only for groups. Maybe dig deeper into this statement?

Anyhow, I think I've seen this spell somewhere before Vercas? Was it you who over-used BJ functions?
Great improvements I must say. I'm gonna give you 3/5 for a good work and a nice intro (really explains alot).
As usually, I still have improvements
Here're some points (remember; this is constructive critisim)

1: If your functions are private, why do you add the HL preffix to everything?
Maybe you don't know what 'private' really does, so I'm gonna go ahead and explain it to you.
See, if a function has the preffix private and is inside a scope, JassHelper converts the function's name to "ScopeName___[RandomDigit]___FunctionName".
So, in for example HLAction's case, the 'real' function name is "HolyLight___296___HLActions", and that makes the HL preffix quite unneccessary, right? "HolyLight___296___Actions" seems fair enough ("private function Actions")
(That 296 can be any number between 000 and 999)

2: You've forgotten to clean up texttag, and you never assigned and nulled the GetTriggerUnit().
There's been a discussion about whether or not to null handles, and the conclusion was that Yes, it's neccessary, because otherwise the handle's index may never be recycled if you don't.

3: Maybe consider to add a Setup section? It'll enhance user-friendlyness to a very high standard.
If you don't know what a Setup section is, either check out my time stop on the front page, or search for JESP and read the introduction text (before the document itself).


===== Edit ======

We're not nullifying the trigger/handle here, but we're nullifying the local variable which was holding it.
Nullifying isn't related to handles, that has DestroyBla()/RemoveBla().

(This probobly helps Thanathos aswell)
What he does basically cleans up the local which would never be accessed after being used...

Unlike other handles, triggers are never destroyed (ever) and thus, we don't need to null it. Handle variables are just pointers to index memmory, they're not 'holders' as you state.
If I remember correctly, every other handle is destroyed in some way, but triggers can only be disabled/enabled, nothing more.
Also, trigger handle is just a long integer pointing to some memmory index. Integers, as we know, does not need to be nulled.

If you want more reasoning I have a topic on this on wc3c, where Vexorian himself explained this (I'm too lazy atm to link it)

 

[Deuterium]

Well maybe I was wrong about that "nulling trigger thing".

But the Anti-Leak thing is used by ALOT of good vJassers... so don't know whether that is right or wrong...

 

[Cheezeman]

But the Anti-Leak thing is used by ALOT of good vJassers... so don't know whether that is right or wrong...

I'm not saying you should stop, but that it might be worth asking around about.
As soon as my current project is done, I'm gonna ask how they make these 'leak-tests' and try it myself.

 

[Vercas]

JESP quote from Vexorian: "Its main purpose was to educate people in a darker age. Now its spirit has become common sense when coding vJass and there are enough people educated to value what this manifest wanted to encourage. It has served its purpose."

I made it more customizable, avaible for more levels...
Will be updated soon with lots of more changes.

Updated some hours ago...

 

[Vercas]

I've updated this spell.
I've been bored so I've improved the code a little.

 

[Roflcoptor]

Well I'd set the units to variables, but it's not neccesary, looks neater (i don't wanna say more, cause i might be wrong)

local unit c = GetTriggerUnit()
local unit t = GetSpellAbilityTargetUnit()

basically using GetSpellAbiltyUnit() is less efficient than GetTriggerUnit()

Otherwise, meh still very simple spell.

 

[Vercas]

I know...

 

[Roflcoptor]

Then why don't you change it? :S

 

[Deuterium]

Actually setting those into variables speed up the code (although VERY SLIGHTLY )

 

[Vercas]

There's always room for improvement...
I'll do it, then!

 

[TriggerHappy]

I come with comments!

• The any unit BJ event is perfectly fine.
• local filterfunc f = Filter(function AntiLeak) is not needed. null boolexpr's don't leak inside events.
• GetUnitState( GetSpellTargetUnit(), UNIT_STATE_LIFE ) + r ) should be changed to GetWidgetLife( GetSpellTargetUnit()) + r )
• ATTACK_TYPE_NORMAL, DAMAGE_TYPE_DIVINE, WEAPON_TYPE_WHOKNOWS - These should all be configurable.
• Storing GetSpellTargetUnit() would save many function calls.
• Make the texttag colors configurable.

 

[Vercas]

Ok.
I'll work on these too today or tomorrow.