Cutting Edge v2.1

[Moderator]

Bribe:

I'm not sure why you initialize these two groups as you just overwrite them later (memory leaks):

Custom script: set udg_CEDummyGroup[udg_CEIndex] = CreateGroup()
Custom script: set udg_CEDebuffGroup[udg_CEIndex] = CreateGroup()

The spell itself I found to be a bit underwhelming. This might have to do that a shadow-strike mixed with the crushing-wave effect you chose doesn't seem to fit. Stunning the units instead of critical striking them would have fit better.

Maker, v2.0, 14.07.2011
Approved, could be useful. A simple spell. You should add importing instructions.

 

[Laserdude]

I can't test this due to my WE problem...
However, the triggering looks fine and leakless.
You are using a maximum of 100 casts at the same time, it should not exceed that limit, I guess.
The way you are applying the debuff is delayed, but that should not be a too big problem.
4/5, due to the late applying of the debuff, and that I don't think that an armor decreasing debuff fits this spell... Since it is based off the Warden, why not use a Shadow Strike debuff?
Vote for approval.

 

[GolemWarrior]

I took a brief test on it and it seemed fine. Great job!

 

[Klingo]

there's something disturbing me, the special effect still appear even if the enemy is spell immune.

 

[Patio_111]

Well done, Marsal2000. It's a pretty simple spell, in my opinion... However, the triggering is done efficiently, and the spell is leakless and MUI. It fits the Warden hero - especially with the bleed effects. Good work on the triggering part. Overall, I'll give it a 4/5 and vote for approval

 

[Marsal]

Ah thanks

I can't test this due to my WE problem...
However, the triggering looks fine and leakless.
You are using a maximum of 100 casts at the same time, it should not exceed that limit, I guess.
The way you are applying the debuff is delayed, but that should not be a too big problem.
4/5, due to the late applying of the debuff, and that I don't think that an armor decreasing debuff fits this spell... Since it is based off the Warden, why not use a Shadow Strike debuff?
Vote for approval.

The maximum can be increased to the maximum array size you just need to modify the CEMax integer value at the init. Dunno but the shadow strike could be good ~.~
How could I apply it without delay?

there's something disturbing me, the special effect still appear even if the enemy is spell immune.

Hmm I'll check this.
edit: Ah just a little missclick, instead of magic immune i've chosen the etheral.

 

[Klingo]

just put a comparison where the special effects are created that will check if the unit is spell imune or not

 

[Marsal]

i just missclicked at the boolean and chosen the "Etheral" instead of "Magic Immune"
UPDATED

 

[Laserdude]

Ah thanks


The maximum can be increased to the maximum array size you just need to modify the CEMax integer value at the init. Dunno but the shadow strike could be good ~.~
How could I apply it without delay?


Hmm I'll check this.
edit: Ah just a little missclick, instead of magic immune i've chosen the etheral.

Well, as my Buff System so-called "spell" specifies, you can add items to the targeted units to instantly add buffs or debuffs.
This is not commercial for my spell, since I am highly considering making it a tutorial.

 

[iAyanami]

• Unit Group - Pick every unit in CETempGroup and do (Actions)
  • Loop - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • ((Picked unit) is alive) Equal to True
      • Then - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Picked unit) is Magic Immune) Equal to True
          • Then - Actions
          • Else - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • ((Picked unit) is in CEDebuffGroup[CEInteger]) Equal to True
              • Then - Actions
              • Else - Actions
                • Unit Group - Add (Picked unit) to CEDebuffGroup[CEInteger]
                • Special Effect - Create a special effect attached to the overhead of (Picked unit) using Abilities\Spells\Other\Stampede\StampedeMissileDeath.mdl
                • Special Effect - Destroy (Last created special effect)
                • Unit - Create 1 Dummy for (Owner of CECaster[CEInteger]) at CETempPoint facing Default building facing degrees
                • Unit - Add a 2.00 second Generic expiration timer to (Last created unit)
                • Unit - Add Debuff to (Last created unit)
                • Unit - Set level of Debuff for (Last created unit) to (Level of Cutting Edge for CECaster[CEInteger])
                • Unit - Order (Last created unit) to Night Elf Warden - Shadow Strike (Picked unit)
      • Else - Actions

This part. You can really just filter all the units when you set the unit group. Just to remove that massive If/Then/Else part. And it's better to filter the units when you set the group, rather looping through all the units in the group, then filtering them.

As for your periodic timer of 0.02 seconds, I think that's quite an overkill. I think using 0.03 or 0.04 seconds wouldn't make much of a difference, especially for human eyes.

And about this condition:

• (CETempGroup2 is empty) Equal to True

From what I know, this function is pretty inefficient. What it does is that it loops through the unit group to find the number of units in a unit group. Instead of doing this, I suggest you manually use an integer variable to count the number of units.

Lastly, this is just an opinion, but wouldn't hashtables be more efficient in terms of MUI? Just an opinion though. Your method works just fine.

Anyways, pretty nice spell

 

[Marsal]

This part. You can really just filter all the units when you set the unit group. Just to remove that massive If/Then/Else part. And it's better to filter the units when you set the group, rather looping through all the units in the group, then filtering them.

As for your periodic timer of 0.02 seconds, I think that's quite an overkill. I think using 0.03 or 0.04 seconds wouldn't make much of a difference, especially for human eyes.

And about this condition:

• (CETempGroup2 is empty) Equal to True

From what I know, this function is pretty inefficient. What it does is that it loops through the unit group to find the number of units in a unit group. Instead of doing this, I suggest you manually use an integer variable to count the number of units.

Lastly, this is just an opinion, but wouldn't hashtables be more efficient in terms of MUI? Just an opinion though. Your method works just fine.

Anyways, pretty nice spell

i've made this spell in a hurry so the trigger isn't the best but it's leakless and MUI so the main function is okay
at 0.05 it looks like you have looping mini lag

hmm... okay
I don't like hashtables, they are too complicated and slower than the indexing (as I know)
edit: oookay updated it

 

[iAyanami]

i've made this spell in a hurry so the trigger isn't the best but it's leakless and MUI so the main function is okay
at 0.05 it looks like you have looping mini lag

hmm... okay
I don't like hashtables, they are too complicated and slower than the indexing (as I know)

True, hashtables may be slower. However, you won't need to needlessly loop 100 times per 0.02 seconds. Plus, Hashtables are basically 2D-Arrays, which is very efficient for storing lots of values. Though, you could imitate 2D-Arrays with global variables too.

 

[Marsal]

nah, indexing is better for me

 

[MortAr]

True, hashtables may be slower. However, you won't need to needlessly loop 100 times per 0.02 seconds. Plus, Hashtables are basically 2D-Arrays, which is very efficient for storing lots of values. Though, you could imitate 2D-Arrays with global variables too.

You wouldn't need to loop 100 times if you were using normal indexing system,
which loops through all created instances, instead through each of them,
which will also give the oppurtunity to use 8192 slots, instead of 100. Also, indexing allows you to use 2D arrays as well, you just need to know how.

On spell:

- Periodic should always be 0.03 to minimize lag from created/used objects

• Set CECaster[CEIndex] = (Casting unit)

(Triggering Unit) is faster and more reliable

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• If - Conditions
• CECount Equal to 0
• Then - Actions
• Trigger - Turn off (This trigger)
• Else - Actions

Can be inside the loop to reduce length

I also don't agree with the fact that you have a limit over the possible instances, but thats all your decision to make. You might also want to change the "bleeding" spell to something else, because most maps uses shadow-strike already it will only cause it to overrite the buff.

3/5 Overall.

 

[Laserdude]

You wouldn't need to loop 100 times if you were using normal indexing system,
which loops through all created instances, instead through each of them,
which will also give the oppurtunity to use 8192 slots, instead of 100. Also, indexing allows you to use 2D arrays as well, you just need to know how.

On spell:

- Periodic should always be 0.03 to minimize lag from created/used objects

• Set CECaster[CEIndex] = (Casting unit)

(Triggering Unit) is faster and more reliable

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• If - Conditions
• CECount Equal to 0
• Then - Actions
• Trigger - Turn off (This trigger)
• Else - Actions

Can be inside the loop to reduce length

I also don't agree with the fact that you have a limit over the possible instances, but thats all your decision to make. You might also want to change the "bleeding" spell to something else, because most maps uses shadow-strike already it will only cause it to overrite the buff.

3/5 Overall.

You don't nesseccarily need to make it 0.03, just make sure it is 0.03 or Slower.
A limit is ok in my opinion, allthought I think that it should be around 300, this has a very small chance of being overridden and it is a fair ammount.

 

[Marsal]

You wouldn't need to loop 100 times if you were using normal indexing system,
which loops through all created instances, instead through each of them,
which will also give the oppurtunity to use 8192 slots, instead of 100. Also, indexing allows you to use 2D arrays as well, you just need to know how.

On spell:

- Periodic should always be 0.03 to minimize lag from created/used objects

• Set CECaster[CEIndex] = (Casting unit)

(Triggering Unit) is faster and more reliable

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• If - Conditions
• CECount Equal to 0
• Then - Actions
• Trigger - Turn off (This trigger)
• Else - Actions

Can be inside the loop to reduce length

I also don't agree with the fact that you have a limit over the possible instances, but thats all your decision to make. You might also want to change the "bleeding" spell to something else, because most maps uses shadow-strike already it will only cause it to overrite the buff.

3/5 Overall.

I'm not a kind of a pro triggerer but, hey it works and leakless. You can change the ability whatever you want easily example: a cripple. The period and the maximum instances can be modified too... Easy to edit these things...

 

[ShiroiSenji]

New version has more eyecandy :] good

 

[Marsal]

c'mon could someone rewiev it for approval? ~.~

 

[baassee]

c'mon could someone rewiev it for approval? ~.~

Give them a break. THW got way too few spell moderators.

 

[Marcos DAB]

Overall seems fine.

 

[Bribe]

Why do you loop through all indices in order to find one that has been recycled? You can simply do a singular-linked list to track instances valid for recycling:

• -------- Add new index to be recycled --------
• Set recycleList[CEInteger] = lastRecycled
• Set lastRecycled = CEInteger
• -------- Get a recycled index --------
• Set CEInteger = lastRecycled
• Set lastRecycled = recycleList[lastRecycled]

 

[Marsal]

Heh when you last told me
that you gave me a jass code and i'm lost in it and gave up Thanks for that ver with correct var names ~.~
edit2:found the problem ~.~
updated*

 

[Bribe]

The loop indexing can disappear if you tactically merge the loop with the new indexing technique I showed you.

 

[Marsal]

Now I have an idea for it ~.~ A few minutes and I update again.

 

[Bribe]

There are still some redundancies (recycle boolean not important), the recycle variables should probably be prefixed to avoid conflict with other libraries, and the groups don't actually need to be destroyed if you are recycling instances. Just use "Remove all units from group" instead of the destroygroup custom script.

 

[Marsal]

Changes made.

 

[Bribe]

There are some things going on with the code that just don't make sense.

CEHas[CELastRecycled] Equal to True

I recommend just checking "CELastRecycled Equal to 0". It's more simple and avoids creating a recycle boolean array.

At that point you can also remove the part where you initialize the groups in your map initialization trigger.

If you used just one single dummy unit (with the dummy.mdx model) it would avoid creating four different units for each different special effect. You would then just attach the appropriate effects to the dummy units and destroy the effects when the dummy units died. With this change you could add configurables in your setup trigger which would allow users to very easily define what the spell should look like (and also avoid having to c & p 4 different dummy units just to make your spell work).

It took me a while to figure out what it is I was really uneasy about due to the unique way you coded this, but these attribute to why I haven't approved it yet.

 

[Marsal]

There are some things going on with the code that just don't make sense.

CEHas[CELastRecycled] Equal to True

I recommend just checking "CELastRecycled Equal to 0". It's more simple and avoids creating a recycle boolean array.

At that point you can also remove the part where you initialize the groups in your map initialization trigger.

If you used just one single dummy unit (with the dummy.mdx model) it would avoid creating four different units for each different special effect. You would then just attach the appropriate effects to the dummy units and destroy the effects when the dummy units died. With this change you could add configurables in your setup trigger which would allow users to very easily define what the spell should look like (and also avoid having to c & p 4 different dummy units just to make your spell work).

It took me a while to figure out what it is I was really uneasy about due to the unique way you coded this, but these attribute to why I haven't approved it yet.

That was the first I tried but it made the spell non-MUI.

Wut?

The dummies have different sizes,colors and animation speed so I can't do that if I want to get the same effect.


It's really not that hard ~.~ If you want I can make a documentation that explains everything.

 

[Bribe]

The sizes, colors and animation speed can be adjusted with triggers, it will make the effects larger or smaller or different colors depending on the properties of the unit they are attached to.

Adding some comments to your triggers is definitely a step in the right direction.

That comparision to 0 thing I showed you would not break multi-unit-instanciability. That's the most common recycling trick and all the well-written vJass resources are using it (including the built-in vJass structs).

And what I was suggesting was to remove this:

• For each (Integer A) from 1 to CEMax, do (Actions)
  • Loop - Actions
    • Custom script: set udg_CEDummyGroup[GetForLoopIndexA()] = CreateGroup()
    • Custom script: set udg_CEDebuffGroup[GetForLoopIndexA()] = CreateGroup()

Because it's really not needed when you include that indexing change I suggested.

 

[Marsal]

The sizes, colors and animation speed can be adjusted with triggers, it will make the effects larger or smaller or different colors depending on the properties of the unit they are attached to.

Adding some comments to your triggers is definitely a step in the right direction.

That comparision to 0 thing I showed you would not break multi-unit-instanciability. That's the most common recycling trick and all the well-written vJass resources are using it (including the built-in vJass structs).

And what I was suggesting was to remove this:

• For each (Integer A) from 1 to CEMax, do (Actions)
  • Loop - Actions
    • Custom script: set udg_CEDummyGroup[GetForLoopIndexA()] = CreateGroup()
    • Custom script: set udg_CEDebuffGroup[GetForLoopIndexA()] = CreateGroup()

Because it's really not needed when you include that indexing change I suggested.

That's high for me. How can I add 4 effects, to only one unit, with different sizes, colors and rotation angle?

That CEHas[CELastRecycled] Equal to True part is increasing the maximum number of loops. When I used that CELastRecycled Equal to 0 thing, the trigger was able to overwrite an already running instance.

What indexing change?

 

[Oljin]

works ok, but not with fortress/strongholds - i could get stuck within the fortress if that last point was exactly in the middle of the fortress. So I combined it with CheckWalkability (creating an item at move position, basically) and with that it works better