Double Spinning Blade v1.1.5

[Moderator]

Double Spinning Blade v1.1.4 | Reviewed by Maker | 27th Sep 2013

APPROVED

You put an inventive spin on the ability by having the shadow throw the projectile back. You made a solid ability Don't create the harvester if "destroy trees" is false Add a bit of follow through time so the caster finishes the cast animation Try to get rid of duplicate action blocks, like the deindexing block Set the death type of the Spinning blade to Can't raise, does not decay The projectile could make some sound or use a casting sound

[tr]

Old reviews Double Spinning Blade v1.1.2 | Reviewed by Maker | 15th Sep 2013 NEEDS FIX Create the peasant for neutral passive, hidden units reserve food Get rid of the A unit dies trigger. You can set the Art - death time to 0. Make the dummies have Can't raise, does not decay death type DSB_Damage_Group does not need to be an array The dummies give a lot of vision [tr]

 

[Rheiko]

~Reserved
like i said, "Feedbacks and Suggestions is appreciated"

 

[FUJI]

In the Configuration Trigger in the tree destroyer setup... Why you set the unit variable after the hide unit action instead use of unit variable?

 

[Rheiko]

Fuu*k.. I forgot about that
i'll fix them later
Thanks

 

[Malhorne]

I'll test it after (if i'm not too lazy~) and tell you what i think of it

EDIT : I think you can just use the alpha canal for the dummy.
Just wondering why use a peasant to destroy the trees ?

 

[Daffa]

Dummy Removal is useless, set dummy's death to can't raise and doesn't decay

 

[FUJI]

Dummy Removal is useless, set dummy's death to can't raise and doesn't decay

He don't want to see the death animation of the dummy model.. That's why.

 

[Malhorne]

So just remove it since the beggining.

 

[Rheiko]

I'll test it after (if i'm not too lazy~) and tell you what i think of it

EDIT : I think you can just use the alpha canal for the dummy.
Just wondering why use a peasant to destroy the trees ?

if i didn't use peasant, it will also destroy the other destructibles

So just remove it since the beggining.

what do you mean by that and how?

Thanks for the feedbacks everyone :3

 

[FUJI]

So just remove it since the beggining.

He used a expiration timer to dummy not a duration or a counter. That's why it can't remove it in the beginning. If he removes it in the beginning it will not work and can cause a bugs.

 

[Rheiko]

ah that's what he meant
Yeah, thanks for pointing it out jakezinc :3

 

[Malhorne]

Why create a dummy peasant at the beggining ?

 

[Rheiko]

grrr,

Spoiler

you are so fu*cking a*noy*ng

okay.. i will explain it from the beginning
first of all, if you didn't use something like this it will also destroy the other destructible
to avoid this problem, first i create a dummy peasant at the beginning.. when i want to destroy the tree, i will order that dummy to check wether that destructible is a tree or not using "harvest" if it's tree, destroy the tree. if it's not ---
can you understand? this is a trick, maybe i will need mag to explain it to you if you are still not understand it

 

[FUJI]

Why create a dummy peasant at the beggining ?

Peasant is used to cut or harvest ONLY TREES not other destructible like BRIDGES.. Now to used a peasant you must pick any destructible and order a peasant to harvest that destructible if peasant CAN'T harvest bridges so it will not be picked as a desired tree. So harvest picked trees right??? so the only destructible selected is trees. This can be done in ITE if the current order of the peasant in harvest that tree then kill that tree. Then after that ITE cond, stop the order. Anyway you can add this line for safety in the tree destroyer setup for adding harvester ability to the peasant

• Custom script: call UnitAddAbility(udg_your_unit,'Aloc')

 

[Rheiko]

finally, someone stand up to give me a hand
thanks jakezinc :3

 

[Malhorne]

Ah another idea to avoid the isdestructible tree ^^

Okay.

 

[Rheiko]

haha, finally you could understand it.. (borrowing some emoticon from red baron xD)

 

[Ofel]

Review :

• You forgot to remove location stored in TempPoint and TempPoint2.
• Store (Picked unit) and use it to check valid units and more.
• Same goes to (Picked destructable).
• You don't need to check if the current stage is 1 or 2 again.
• When the spell is going to change to stage 2, remove the location in TempPoint and TempPoint2 first.

Suggestions :

• Make attack-type and damage-type configurable.
• Store the (Order(harvest)) into a variable with type Order.
• To pick nearby units, use "Custom script: set bj_wantDestroyGroup = true" instead. If you still want to use the current way, you don't need to make the DSB_Damage_Group arrays.

Sorry if there's wrong grammar...

 

[Malhorne]

Ahah i'm trying to recover some exhaust i got yastarday xD
And now gotta have to work in vJASS x) (never liked GUI ^^)

 

[Rheiko]

Review :

• You forgot to remove location stored in TempPoint and TempPoint2.
• Store (Picked unit) and use it to check valid units and more.
• Same goes to (Picked destructable).
• You don't need to check if the current stage is 1 or 2 again.
• When the spell is going to change to stage 2, remove the location in TempPoint and TempPoint2 first.

Suggestions :

• Make attack-type and damage-type configurable.
• Store the (Order(harvest)) into a variable with type Order.
• To pick nearby units, use "Custom script: set bj_wantDestroyGroup = true" instead. If you still want to use the current way, you don't need to make the DSB_Damage_Group arrays.

Sorry if there's wrong grammar...

what about the picked unit? i store them already :\

Make attack-type and damage-type configurable.

that's not really important

Store the (Order(harvest)) into a variable with type Order.

to make it configurable? no i won't
if someone change the order, it will messed up

To pick nearby units, use "Custom script: set bj_wantDestroyGroup = true" instead. If you still want to use the current way, you don't need to make the DSB_Damage_Group arrays.

why is that?

 

[Malhorne]

Attack type and damage type isn't really needed ^^
As said above don't think making the order configurable is good x)

 

[Daffa]

Make attack-type and damage-type configurable.

that's not really important

It's a good addition you know

Store the (Order(harvest)) into a variable with type Order.

NO! That's just ridiculous, if you edit the order, everything are messed up.

SUGGESTIONS
OPTIONAL
I recommend an IsDestTree (preferably Almia's Vjass Version(Unless you intend this for normal WE spell)) as it will use less triggers.

 

[Rheiko]

ah, i forgot you're making the GUI version..
i will check it out

 

[Daffa]

also, what Ofel said needs to be done :

1. You forgot to remove location stored in TempPoint and TempPoint2.
2. Store (Picked unit) and use it to check valid units and more.
4. You don't need to check if the current stage is 1 or 2 again.
5. When the spell is going to change to stage 2, remove the location in TempPoint and TempPoint2 first.

remember, A Location will Leaks Once you change their reference

 

[Rheiko]

i know, just forgot to remove it..
i will fix them all on the next update, so if there is something else let me know..

 

[FUJI]

Anyway, There;s a slightly not MUI in this spell because when it cast RAPIDLY it causes a dummy not returning to the caster and the dummyIllusion is not spawning correctly. Properly your spell is Not MUI (((

 

[Rheiko]

Hmm.. I will test it again tomorrow, and see what comes up.
Thanks for your report

 

[FUJI]

Anyway just remember to decrease the lenght of recycling..... Nulling GLOBAL variables like such this: set udg_unit = No Unit It is not working you can get rid of those *_*

 

[Ofel]

Store the (Order(harvest)) into a variable with type Order.

It's just a suggestion...and i don't meant to be configurable...
the function of it is :

String2OrderIdBJ

which contain :

function String2OrderIdBJ takes string orderIdString returns integer
    local integer orderId
    
    // Check to see if it's a generic order.
    set orderId = OrderId(orderIdString)
    if (orderId != 0) then
        return orderId
    endif

    // Check to see if it's a (train) unit order.
    set orderId = UnitId(orderIdString)
    if (orderId != 0) then
        return orderId
    endif

    // Unrecognized - return 0
    return 0
endfunction

You can make it with custom script to make it more unreadable...

• Custom script: set udg_DSB_HarvestOrder = String2OrderIdBJ("harvest")

Edit
I got another problem that may can cause nearby units damaged more than once...

This :

• Custom script: set udg_DSB_UnDamage_Group[udg_DSB_Max_Index]

Should be :

• Custom script: set udg_DSB_UnDamage_Group[udg_DSB_Current_Index]

 

[Rheiko]

could you tell me how to use it?
i have no idea how to use Jass or Script for the first time, i'm sucks ;P

 

[Daffa]

Ofel, AFAIR, there's no need to use Jass Code for the String to encrypt it, just set it inside the UNCONFIGURABLE block on Map Init.

Rheiko, you can directly use Ofel's Custom Script as I saw they are valid. except you need to know what to replace.

 

[Rheiko]

i don't know :\
and i sucks at Jass and Scripts

 

[deathismyfriend]

Store picked unit before the ITE and use the variable in the conditions.

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• If - Conditions
• ((Picked unit) belongs to an enemy of DSB_Owner[DSB_Current_Index]) Equal to DSB_Conditions[1]
• ((Picked unit) is alive) Equal to DSB_Conditions[2]
• ((Picked unit) is Magic Immune) Equal to DSB_Conditions[3]
• ((Picked unit) is A structure) Equal to DSB_Conditions[4]
• ((Picked unit) is in DSB_UnDamage_Group[DSB_Current_Index]) Equal to DSB_Conditions[5]
• Then - Actions
• Set TempUnit = (Picked unit)

TempPoint and TempPoint2 leak in your cast trigger.

Move your turn off ITE in the loop trigger into the de-indexing portion of the spell.

This seems pointless ? Set DSB_Stage[DSB_Max_Index] = 1

You never de-index the ability lvl. Everything you index has to be de-indexed.

 

[Rheiko]

thanks for the review

This seems pointless ? Set DSB_Stage[DSB_Max_Index] = 1

why?

 

[FUJI]

thanks for the review

why?

Because I think it is already 1 in the variable editor Initialized.

 

[Rheiko]

now, i understand what you meant
explaination:
at the loop trigger (now the stage = 1) check if the stage is 1 then do actions
when the stage 1 is finish, change the stage to 2 (now the stage = 2)
when the stage 2 is finish, i turn off the trigger
so if an unit want to cast the spell again i change it to 1 again, though if it is pointless i will remove it

 

[FUJI]

now, i understand what you meant
explaination:
at the loop trigger (now the stage = 1) check if the stage is 1 then do actions
when the stage 1 is finish, change the stage to 2 (now the stage = 2)
when the stage 2 is finish, i turn off the trigger
so if an unit want to cast the spell again i change it to 1 again, though if it is pointless i will remove it

I don't know what dimf said or pointing it out xD...

 

[Rheiko]

what he meant is Set DSB_Stage[DSB_Max_Index] = 1 seems pointless
but maybe he's right, it is pointless

 

[FUJI]

Ahhh..... yes it seems pointless because it is not needed to be index ( I think ) try it to get rid of the Stage Indexed Array and set it to normal variable.

 

[Rheiko]

if i set it to normal variable, the triggers will messed up :\

 

[FUJI]

^ No they're not messing up I tried it in my test map and it works properly..

EDIT:

Sorry My bad xD I think it will not working properly if your making a MUI.

 

[Rheiko]

you're using index?
what about two unit cast the spell?
if the first unit has reach into stage = 2
then the second unit cast the spell which is now set the stage = 1?
how about that?

 

[FUJI]

nah... I'm using hashtable.....

I think it is wrong but just PM or VM dimf xD that what he talking about pointless in stages.

 

[deathismyfriend]

It actually isn't pointless i just missed seeing it in the loop trigger.

You also need to de-index that value.

 

[Rheiko]

okay, i will

 

[FUJI]

It actually isn't pointless i just missed seeing it in the loop trigger.

You also need to de-index that value.

Yes recycling that stages will make your spell 100 % MUI because you didn't recycle tho.
Anyway I'm not an Indexer User ^^ but I know the basics of indexing because to the turorial of dimf.

 

[Ofel]

@dimf
Damn! i wanted to say that but i don't know how to arrange my english... xD

Spoiler

I never study in an english course...

 

[deathismyfriend]

@dimf
Damn! i wanted to say that but i don't know how to arrange my english... xD

Spoiler

I never study in an english course...

Lol well now you know for next time i screw up like that.

And yes that will make it MUI but he also needs to de-index everything else. Example: ability lvl.

 

[Rheiko]

Spoiler

I never study in an english course...

well, me too