Spell Blade Barrier

[Crazy_Dead]

Nice...

 

[Tyman2007]

meh... I've seen this before actually... More efficient method, but I've seen this done a couple times already. Maybe we're just tired of duplicates.

 

[BlackRose]

meh... I've seen this before actually... More efficient method, but I've seen this done a couple times already. Maybe we're just tired of duplicates.

Really? I want to see them

 

[BlackRose]

Update: v1.03
v1.02 is bugged! The effect is MUI, but the damage wasn't. I did some strange things ...

Why didn't that get spotted at TheHiveworkshop.... lucky I spotted while working on it in Pudge Wars

 

[Romek]

Why has the spell got such an awkward interface to use? 'Blade Barrier - Target' should be made possible by changing a value in the configurable globals, rather than having to create a struct in a different trigger. Besides that, the spell should be within a scope, not a library. And all the structs, functions and globals should be private. If the structs are private, then the individual members and methods no longer need a private prefix. I'd be interested to know why a seemingly random choice of members are actually private though.
Also, not every bracket, name, type and equals sign needs to be aligned. Honest.
____________

No configurable number of blades per level? No blood effects when a unit is sliced up by the blades?
____________

> [ljass]// Note: Some sounds cannot be played rapidly. See thread: http://www.thehelper.net/forums/forumdisplay.php?f=42[/ljass]
What thread?
____________

Sounds could be done every X seconds, not every X 'ticks'. Just add a note that it should be a multiple of the timeout, or it'll get rounded up. Makes it easier to configure.
____________

> [ljass]// Blade will be created at BLADE_AOE + GetRandomReal( -20, 20 )[/ljass]
BLADE_AOE? Where?
____________

Static if's don't work outside of methods or functions. So using them around a global block will never remove the global block, even if it's false.
____________

Struct members don't need to be nulled, they're globals, and are very likely to be overwritten. Those three or so instance that don't get overwritten won't impact anything in any way whatsoever. Thus, things like this as just unnecessary:

.
            if this.fxpath != null then
                call DestroyEffect( this.fxpath )
                set this.fxpath = null
            endif

As is this:

.
        method onDestroy takes nothing returns nothing
            set this.target = null
        endmethod

____________

.
            if  this.originData.sec >= this.originData.dur                  or /*
            */  ( tempX*tempX+tempY*tempY ) > this.radius_45                or /*
            */  IsUnitType( this.originData.caster, UNIT_TYPE_DEAD )        or 

            <Other stuff>

                return true
            else
                if ( tempX*tempX+tempY*tempY ) > this.radius_80 then
                
                    //call Damage_Pure( this.originData.source, this.target, this.dps )
                    call UnitDamageTargetEx( this.originData.source, this.target, this.dps, true, true, /*
                    */ ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNKNOWN, null )
                    static if DO_DMG_SFX then 
                        call DestroyEffect( AddSpecialEffectTarget( DAMAGE_EFFECT, this.target, "chest" ) )
                    endif
                endif
            endif
            
            return false

->

.
            if  this.originData.sec >= this.originData.dur                  or /*
            */  ( tempX*tempX+tempY*tempY ) > this.radius_45                or /*
            */  IsUnitType( this.originData.caster, UNIT_TYPE_DEAD )        or 

            <Other stuff>

                return true
            endif
            if ( tempX*tempX+tempY*tempY ) > this.radius_80 then
                
                //call Damage_Pure( this.originData.source, this.target, this.dps )
                call UnitDamageTargetEx( this.originData.source, this.target, this.dps, true, true, /*
                */ ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNKNOWN, null )
                static if DO_DMG_SFX then 
                    call DestroyEffect( AddSpecialEffectTarget( DAMAGE_EFFECT, this.target, "chest" ) )
                endif
            endif

            return false

You do this more than once. As a general guideline:

if <conditions> then
    <actions>
    return <value>
else
    <other actions>
endif

->

if <conditions> then
    <actions>
    return <value>
endif
<other actions>

____________

As before, I'm sure I've missed some things out, though I'll get to those later. The code also seems to be awfully long for something as simple as this, though that just may be because of the way you space things out. Good job though.

 

[Crazy_Dead]

@Struct Members dont need to be nulled

No, but BlackRose thinks it's good training.

 

[kingkingyyk3]

@Struct Members dont need to be nulled

No, but BlackRose thinks it's good training.
If you are a super scarer of leaking, nulling it is okay, nothing wrong with it.

 

[BlackRose]

That's the most thorough check on this resource. Ever. Though it took some time Although Pudge War's Meathook Revisted took longer I'm guessing.

'Blade Barrier - Target' should be made possible by changing a value in the configurable globals, rather than having to create a struct in a different trigger.

What if someone wants to create a Blade Barrier when they destroy a certain rune or activate some command? What do you mean by changing a value? A constant boolean?

No configurable number of blades per level? No blood effects when a unit is sliced up by the blades?

I'll add that. Is a gigantic finale effect really needed when a unit dies from this spell? I'll add an option regardless.

Also, not every bracket, name, type and equals sign needs to be aligned. Honest.

Habit, dear sir. :thup: Dunno why I'm guessing this brought that up

            call    SetSoundVolume( s, thistype.INLINED_VOLUME )
            call  SetSoundPosition( s, this.cx, this.cy, GetUnitFlyHeight( this.caster ) )
            call        StartSound( s )
            call KillSoundWhenDone( s )

And all the structs, functions and globals should be private. If the structs are private, then the individual members and methods no longer need a private prefix.

All of structs are private, except for BladeBarrierData??

> [ljass]// Note: Some sounds cannot be played rapidly. See thread: http://www.thehelper.net/forums/forumdisplay.php?f=42[/ljass]
What thread?

Oops. Pasted wrong link.

> [ljass]// Blade will be created at BLADE_AOE + GetRandomReal( -20, 20 )[/ljass]
BLADE_AOE? Where?

Mean't RADIUS =)

Static if's don't work outside of methods or functions. So using them around a global block will never remove the global block, even if it's false.

Really? ... ... OK :thup:

if <conditions> then
    <actions>
    return <value>
endif
<other actions>

Eh. Does that really matter?

If you are a super scarer of leaking, nulling it is okay, nothing wrong with it.

(Setting to null takes the same speed as calling an empty function)

Is this true I want to know first

 

[Romek]

> If you are a super scarer of leaking, nulling it is okay, nothing wrong with it.
Making methods which do nothing except null struct members is pointless. There's as much 'wrong' with it, as there is wrong with placing random "DoNothing" calls in every other line.

> What if someone wants to create a Blade Barrier when they destroy a certain rune or activate some command?
If memory serves, you can make runes cast spells. So that's no problem. For 'commands', add the spell, cast it, remove it. Three short lines of code. Besides that, it's not really about length, just more about the spell following standard conventions.

> Is a gigantic finale effect really needed when a unit dies from this spell?
Not when they die, but when they're actually damaged by it. It'd be a nice (and easy) option to add, and it looks silly without blood when there are 20 blades slashing through someone.

> All of structs are private, except for BladeBarrierData??
All of them. Not all of them with exceptions. Spells shouldn't make anything public, really.

> Eh. Does that really matter?
There's no point in using an else if it can be done without one. It's nothing major though, no.

 

[BlackRose]

If memory serves, you can make runes cast spells. So that's no problem. For 'commands', add the spell, cast it, remove it. Three short lines of code. Besides that, it's not really about length, just more about the spell following standard conventions.

Eh. Made struct private and replaced with a public function:

[ljass]public function Create takes unit source, unit target, integer level returns boolean[/ljass] - Does that satisfy?

Not when they die, but when they're actually damaged by it. It'd be a nice (and easy) option to add, and it looks silly without blood when there are 20 blades slashing through someone.

? It already had it? Even before the update...?

// Where damage effects are made.
        private constant boolean DO_DMG_SFX = true
        private constant string  DMG_SFX    = "Abilities\\Spells\\Other\\Stampede\\StampedeMissileDeath.mdl"
        private constant string  DMG_LOC    = "chest"

 

[Romek]

> Does that satisfy?
Not really, but it'll do.

> ? It already had it? Even before the update...?
Ahh.. I hadn't noticed.

Approved.