Dune Worm v1.05b

[Moderator]

Dune Worm v1.05 | Reviewed by Maker | 20th October 2013 [COLOR="gray" [COLOR="gray" [COLOR="gray" [COLOR="gray" [COLOR="gray" [COLOR="gray" Concept[/COLOR]] A giant work emerging from the ground and having a go at its target is both original and impressive Triggers[/COLOR]] The spell is MUI There is sufficient configuration options and the coding is generally fine Get rid of call SetUnitPosition Objects[/COLOR]] Almost everything seems to be set up correctly You should add follow through time so the caster completes the casting animation Effects[/COLOR]] The effects are alright The spikes that rise from the ground could make a sound The learn tooltip is missing the text that displays which level the ability is When the ability is cast, the worm can be briefly seen a bit before it is supposed to emerge The sound effect when the worm rises and goes back under ground could be something different. Try Impale, Burrow or Root/Uproot for example Rating[/COLOR]] CONCEPT TRIGGERS OBJECTS EFFECTS RATING STATUS APPROVED Links[/COLOR]] Spell Effect Event [COLOR="gray"

[/TD]

Old reviews Dune Worm v1.03 | Reviewed by Maker | 20th October 2013 [COLOR="gray" [COLOR="gray" [COLOR="gray" [COLOR="gray" [COLOR="gray" [COLOR="gray" Concept[/COLOR]] A giant work emerging from the ground and having a go at its target is both original and impressive Triggers[/COLOR]] The spell is MUI There is sufficient configuration options and the coding is generally fine Null the struct unit members before calling destroy You could use SetUnitX/Y instead of SetUnitPosition SUP checks pathing and that is a slow process By default the spell causes a big FPS drop since it creates so many special effects. Luckily it can be configured There is no importing instructions GroupEnumUnitsInRange clears the group before adding units to the group, so I don't think there is need for call GroupClear(GLOBAL_GROUP) Objects[/COLOR]] The tooltip description is...well...descriptive and compact The tooltip lists the stats per level The tooltip does not have learn hotkey and it is missing the text that displays the level You could change the command card position to [0,2] The cast animation of the spell is "spell channel" but the hero in the map does not have that animation Art - Caster effect is the default Channel one, but Art - Duration is 0 Is the ability really supposed to have that art? Set Disable other abilities to false so the command card isn't blacked out You should add follow through time so the caster completes the casting animation The worm unit gives huge vision and uses upgrades and food Effects[/COLOR]] The initial cast effects look fine The custom worm model is great The ground effect with the dust fit the spell very well The Art - caster should not be there When the ability is cast, the worm can be briefly seen a bit before it is supposed to emerge Rating[/COLOR]] CONCEPT TRIGGERS OBJECTS EFFECTS RATING STATUS NEEDS FIX Links[/COLOR]] Spell Effect Event Jass Proper Application Guide [COLOR="gray"

Old reviews

[/TD]

 

[IcemanBo]

Very good idea... nice spell. But in my test, sometimes worm didnt attack as often as he should, and then since a certain point no worm summoned anymore.

But looks really nice! 5/5

 

[TriggerHappy]

Very good idea... nice spell. But in my test, sometimes worm didnt attack as often as he should, and then since a certain point no worm summoned anymore.

But looks really nice!

Ah you're right.

I'm updating the fixed version. The problem wasbj_groupRandomCurrentPickwould sometimes return null. I fixed it by usingFirstOfGroupinstead but does anyone know why it's happening?

EDIT: Updated.

 

[axleion]

The problem wasbj_groupRandomCurrentPickwould sometimes return null.

this

your trigger

call ForGroup(GLOBAL_GROUP, function GroupPickRandomUnitEnum)

from JASSCraft

function GroupPickRandomUnitEnum takes nothing returns nothing
    set bj_groupRandomConsidered = bj_groupRandomConsidered + 1
    if (GetRandomInt(1,bj_groupRandomConsidered) == 1) then
        set bj_groupRandomCurrentPick = GetEnumUnit()
    endif
endfunction

function GroupPickRandomUnit takes group whichGroup returns unit
    // If the user wants the group destroyed, remember that fact and clear
    // the flag, in case it is used again in the callback.
    local boolean wantDestroy = bj_wantDestroyGroup
    set bj_wantDestroyGroup = false

    set bj_groupRandomConsidered = 0
    set bj_groupRandomCurrentPick = null
    call ForGroup(whichGroup, function GroupPickRandomUnitEnum)

    // If the user wants the group destroyed, do so now.
    if (wantDestroy) then
        call DestroyGroup(whichGroup)
    endif
    return bj_groupRandomCurrentPick
endfunction

as you can see, the GroupPickRandomUnitEnum uses bj_groupRandomConsidered to determine the random part,

the BJ alternative always set the bj_groupRandomConsidered to 0 before forgroup call, thus the random unit is at least will never be null from the first loop.

your bj_groupRandomConsidered is always increases and never reset to 0, thus to actually return not null unit, you need to get pass over this if-then-else if (GetRandomInt(1,bj_groupRandomConsidered) == 1) then

to solve this, you can either use the GroupPickRandomUnit() or set the bj_groupRandomConsidered to 0 before forgroup call.

I suggest you to add set bj_groupRandomCurrentPick = null in case of forgroup call on empty group returning last forgroup call result.

on the trigger itself,

            call UnitAddAbility(u, 'Avul')
            call UnitAddAbility(u, 'Aloc')

why not place those rawcode into constant? probably for anyone who somehow change the default ability.

on the create method, is it necessary for 360 sfx at once? it probably lags a bit on lower spec PC.

                    call DestroyEffect(AddSpecialEffectTarget(BLOOD_FX, this.target, "origin"))

allow the attachment point to be configurable.

in callback method, you should null the timer variable t
same case on variable tmr on create method.

 

[Geshishouhu]

This is a really really nice spell! If the movement or animation speed of the worm is configurable, it would be perfect.

BTW: TriggerHappy, are you TriggerHappy187?

 

[TriggerHappy]

compare

your trigger

call ForGroup(GLOBAL_GROUP, function GroupPickRandomUnitEnum)

from JASSCraft

function GroupPickRandomUnitEnum takes nothing returns nothing
    set bj_groupRandomConsidered = bj_groupRandomConsidered + 1
    if (GetRandomInt(1,bj_groupRandomConsidered) == 1) then
        set bj_groupRandomCurrentPick = GetEnumUnit()
    endif
endfunction

function GroupPickRandomUnit takes group whichGroup returns unit
    // If the user wants the group destroyed, remember that fact and clear
    // the flag, in case it is used again in the callback.
    local boolean wantDestroy = bj_wantDestroyGroup
    set bj_wantDestroyGroup = false

    set bj_groupRandomConsidered = 0
    set bj_groupRandomCurrentPick = null
    call ForGroup(whichGroup, function GroupPickRandomUnitEnum)

    // If the user wants the group destroyed, do so now.
    if (wantDestroy) then
        call DestroyGroup(whichGroup)
    endif
    return bj_groupRandomCurrentPick
endfunction

as you can see, the GroupPickRandomUnitEnum uses bj_groupRandomConsidered to determine the random part,

the BJ alternative always set the bj_groupRandomConsidered to 0 before forgroup call, thus the random unit is at least will never be null from the first loop.

your bj_groupRandomConsidered is always increases and never reset to 0, thus to actually return not null unit, you need to get pass over this if-then-else if (GetRandomInt(1,bj_groupRandomConsidered) == 1) then

to solve this, you can either use the GroupPickRandomUnit() or set the bj_groupRandomConsidered to 0 before forgroup call

Thanks

This is a really really nice spell! If the movement or animation speed of the worm is configurable, it would be perfect.

Agreed, I'll consider adding those.

BTW: TriggerHappy, are you TriggerHappy187?

Yeah.

UPDATED WITH VIDEO

 

[BPower]

null local trigger t

This might be usefull:
http://www.hiveworkshop.com/forums/jass-resources-412/snippet-spelleffectevent-187193/

I'm ~99% 100% sure that 'Avul' is needless, when adding 'Aloc' aswell.

call UnitAddAbility(u, 'Avul')
call UnitAddAbility(u, 'Aloc')

Beside that, this spell is refreshing cool.

 

[TriggerHappy]

>> null local trigger t

Not needed, the trigger is never destroyed.

>> This might be usefull:]

Overkill IMO, maybe I can implement as an optional library or something.

>> I'm ~99% sure that 'Avul' is needless, when adding 'Aloc' aswell.

Thanks for the tip I'll look into it.

>> why not place those rawcode into constant? probably for anyone who somehow change the default ability.

mm, alright.

>>in callback method, you should null the timer variable t

The timers are recycled, it doesn't need be nulled.

>> Beside that, this spell is refreshing cool.

Thanks
Funny cause I made it years ago.

 

[Geshishouhu]

>> null local trigger t

Not needed, the trigger is never destroyed.

>> This might be usefull:]

Overkill IMO, maybe I can implement as an optional library or something.

>> I'm ~99% sure that 'Avul' is needless, when adding 'Aloc' aswell.

Thanks for the tip I'll look into it.

>> Beside that, this spell is refreshing cool.

Thanks
Funny cause I made it years ago.

With "Aloc", the unit is invulnerable.

Edit:
Glad to see you back, TriggerHappy.

 

[nhocklanhox6]

When you use TimerUtils with structs, you don't need to save the timer as a member of the struct. Remove all this. front of the struct member.

GetWidgetX/Y faster than GetUnitX/Y, this key is here:

type unit extends widget
type widget extends agent
type agent extends handle

Because worm powerful and can move any terrain so i think don't need:
SetUnitPosition
=>
SetUnitX SetUnitY

Very nice spell but i love the colorful >.<, 5/5 with other people but with me is 4/5...^^~

 

[TriggerHappy]

When you use TimerUtils with structs, you don't need to save the timer as a member of the struct

Yeah, you're right.

EDIT: I don't do that?

Remove all this. front of the struct member.

That's a personal preference, why would I have to remove it?

GetWidgetX/Y faster than GetUnitX/Y, this key is here:

Forgot about those

Very nice spell but i love the colorful >.<, 5/5 with other people but with me is 4/5...^^~

What do you mean "but i love the colorful"?

--

I will be uploading an optimized version later (with more configuration).

 

[nhocklanhox6]

What do you mean "but i love the colorful"?

Your spell have a little color =.=!, i like the colorful like seven colors or somthing >.<

That's a personal preference, why would I have to remove it?

if it's that personal preference, so, don't care about that thing =.=!

Cool, i hope next version have more colorful ...

 

[msongyboi]

Pretty epic spell

The stun comes before the unit gets hit by the duneworm - dunno if thats intentional or not

 

[TriggerHappy]

The stun comes before the unit gets hit by the duneworm - dunno if thats intentional or not

It is (I agree it looks a little odd, though). I don't want units to move when the worm is coming up (at least I think so, will test later).

Thanks for the comments guys.

 

[Almia]

I don't think using the native IsUnitAlive is a good idea

and also 'Avul' is useless. Aloc already gives it

 

[TriggerHappy]

I don't think using the native IsUnitAlive is a good idea

And whys that?

and also 'Avul' is useless. Aloc already gives it

"DAMAGE_TYPE_UNIVERSAL can deals damage and kill locust unit" - nhocklanhox6

 

[Almia]

Hmm, fair enough.

 

[FUJI]

You may check if the unit type of the unit is UNIT_TYPE_DEAD.

 

[TriggerHappy]

You may check if the unit type of the unit is UNIT_TYPE_DEAD.

UnitAlivetakes care of that

 

[axleion]

>> null local trigger t
Not needed, the trigger is never destroyed.
>>in callback method, you should null the timer variable t
The timers are recycled, it doesn't need be nulled.

it's not about the timer is recycled or the trigger is never destroyed, it's about the pointer leak. whether pointer leak is valid or not, its a good practice eventually.

 

[TriggerHappy]

Practice shmactice, it's not "needed"

See this page.

 

[axleion]

yes, it's not needed hence why I referred it as suggestion for good practice, not as a correction.

 

[Maker]

hodenhelm: DAMAGE_TYPE_UNIVERSAL can deals damage and kill locust unit

I've tested that before and I tested it again, this is not true. If you can prove otherwise, please post a test configuration.

 

[nhocklanhox6]

After test again...oh my..., i'm sorry, perhaps due to my confusion , sorry sorry. I did not mean to say that. My bad + +!, perhaps because I was associated with the code so it can damage the invulnerable and locust unit (Is not UnitTargetDamage or something else about Damage)...

 

[pred1980]

It's a cool spell. I'm thinking to use it in my project "Forsaken Bastion's Fall" but i need a worm looks like undead. A worm with bones or something like that. I would place him at a graveyard and he would do your spell in a loop the whole game. I send the creater of the model a message and if he would do that, here's my question to you: Can i use your code?

Reg.

 

[TriggerHappy]

I've tested that before and I tested it again, this is not true. If you can prove otherwise, please post a test configuration.

Thanks for that, I'll keep it in mind for the next update

A worm with bones or something like that. I would place him at a graveyard and he would do your spell in a loop the whole game. I send the creater of the model a message and if he would do that, here's my question to you: Can i use your code?

Yeah, go ahead (that's why I submitted it).

EDIT: v1.03 uploaded.

 

[pred1980]

Yeah, go ahead (that's why I submitted it).

Super. Thank you. But first i need the model ^^

 

[Maker]

The tooltip lacks the current level, and you could remove the "description" word. You could set the learned icon position to [0,2]. There is no learn hotkey.

The cast animation is set to an animation that the hero does not have. There is no follow through time so the casting animation gets aborted a bit too quickly. Set "Disable other abilities" to false. Is the ability meant to use the Art - caster effect? Art - duration is 0.

The worm uses food and upgrades. Give the unit Locust in object editor. You can check if the unit has Locust in the create method if you want to be sure the unit has it.

The spell causes a pretty heavy FPS drop by default, but luckily that can be countered with the variable. You could use radians in the loop instead of degrees.

GroupEnumUnitsInRange clears the group before adding units to the group, so I don't think there is need for call GroupClear(GLOBAL_GROUP)

You could use SetUnitX/Y for the worn instead of SetUnitPosition as you don't need to use pathing.

Null the local timer variable. Null the unit struct members before .destroy if they are not null already.

 

[TriggerHappy]

Null the local timer variable.

People really need to stop suggesting this..

 

[Maker]

You are right, it does not cause a leak.

 

[Geshishouhu]

People really need to stop suggesting this..

You are right, it does not cause a leak.

So the conclusion is that nullifying a local trigger variable is unnecessary under any circumstances?

 

[Maker]

^Local triggers do not need to be nulled unless you destroy them. Local timer used for GetExpiredTimer for example does not need to be nulled.

 

[Lord_Marrowgrath]

EPIC! +rep Add more worms!

 

[Geshishouhu]

^Local triggers do not need to be nulled unless you destroy them. Local timer used for GetExpiredTimer for example does not need to be nulled.

Got it, thanks.
Is there a tutorial about this? A comprehensive and detailed one would be perfect.

 

[Bloody Wolf]

pretty sweet spell

 

[FUJI]

Dune Worm v1.03
This spell had been reviewed by jakeZinc.

Thecode looks good and clean.

The documentation is well good but in this line private constant integer LOCUST_ID = 'Aloc' this must have a documentation because some users may harm that if they didn't knew what was that for.

The configuration is fine, but you can use global constant as for the damage base, damage per level and use the constant function for enumerating the constant globals.

It is nice spell and unique idea.

Having 360 special effect at once may cause lag, maybe add a configuration on that.

There's no importing instruction in the spell

The special effects are well fine and the worm model really fits the spell. Maybe make a special effect in the position of the worm while moving, example: Impale Dust, to make it more realistic

Can be useful hunting down enemies one by one

Fix the button position of the spell, the test map must have at least 2 casters or make the cooldown of ability to 0. You can use set tmr = NewTimerEx(this) instead of call SetTimerData(tmr, this) but I think there's no difference at that . You can use SpellEffectEvent so that the 3 lines will be 1 line. Put the onInit function and Action function inside the struct and make them static method. Merge the onCast and constructor into 1 static method. You can use thistype but I think data is shorter .

I think the learn tooltip is nice and it is support every level of the spell.

Rejected: 1-10
Unacceptable : 11-30
Lacking : 31-49
Useful : 50-69
Recommended : 70-90
Highly Recommended : 91-100
Director Cut : 101-105

1) Triggering - 18/20
2) Documentation - 6/10
3) Importing Instruction - 0/10
4) Special Effects - 14/15
5) Usefulness - 15/20
6) Tooltip - 7/10
7) Configuration - 7/10

Total Score - 67/105
jakeZinc Rating - 3.5/5
Status - Useful and Voting for Approval

 

[pred1980]

"Dune Worm v1.04 by TriggerHappy"

 

[WerBackIII]

This.

 

[Gamegear]

wtf its possible

 

[pred1980]

@TriggerHappy

As i told you, i'll include your really cool spell in our mod "Forsaken Bastion's Fall" as a great special event on our graveyard.

Sry, but i'd to adapt the code a bit because no unit/hero cast this worm and i've a hint for your "TempPlayer".

in the struct data you can create a variable "static thisytpe temp" and in the create() you can just write "set .temp = this" and so you can easy use the "p" in this way ".temp.p" to get the player ;-)

I'll give credit So. Thx a lot.

 

[G.One]

PRob


When i imported the coding it got too many compiling errror please reply for solution

 

[pred1980]

You need JNPG with a jass compiler. I suggust you're using the standard World Editor, right?