Samuro Slam V1.1 [MUI]

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

14:24, 11th Feb 2014
BPower:
Needs fix
http://www.hiveworkshop.com/forums/spells-569/samuro-slam-v1-1-mui-247665/#post2485101

 

[IcemanBo]

In cast trigger you make same actin twice:

• Set SS_Dummy_Owner[SS_Dummy_ID] = SS_Caster_Owner
• Set SS_Dummy_Owner[SS_Dummy_ID] = SS_Caster_Owner

In loop you don't need Point[array] variables. You can just use normal point variable, because they get removed immediatly, and you dont refer to them at any other trigger.

• Set SS_Dummy_Loc[SS_Dummy_ID] = (Position of SS_Dummy)
• Set SS_Dummy_Loc2[SS_Dummy_ID] = (SS_Dummy_Loc[SS_Dummy_ID] offset by SS_Speed towards SS_Dummy_Angle[SS_Dummy_ID] degrees)
• ....
• ....
• Custom script: call RemoveLocation(udg_SS_Dummy_Loc[udg_SS_Dummy_ID])
• Custom script: call RemoveLocation(udg_SS_Dummy_Loc2[udg_SS_Dummy_ID])

In loop you forgot to change 'Picked Unit' to 'SS_Dummy'

• Set SS_Dummy_Range[(Custom value of (Picked unit))] = (SS_Dummy_Range[(Custom value of (Picked unit))] - SS_Speed)

________________________________

Spell isnt very special, but looks not bad in game. I would add maybe something more special maybe.

Something like a explosion at end before blademaster disappears would make it more attractive for me. (just for example )

 

[Kazeon]

- needs unit indexer is not a feature
- dont use custom value
- use SetUnitX/Y
- there are some useless variable storings in the cast trigger

• Set SS_Dummy_Owner[SS_Dummy_ID] = SS_Caster_Owner

=>

• Set SS_Dummy_Owner[SS_Dummy_ID] = (Owner of SS_Caster)

- the distance and speed need to be configurable for each level. Also the AoE need to be configurable too
- there is ITE without any action inside the loop trigger

 

[FUJI]

why it does need a Unit Indexer?, you can use some MUI Methods too like Dynamic or Indexing Array and Hashtables ( I think Indexing Array will fit in the spell not a hashtable ).

"- dont use custom value"
This is needed when using Unit Indexer.

• Set SS_Dummy_Damage_UnitGroup = (Units within 250.00 of SS_Loc1 matching ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) is Magic Immune) Equal to False) and ((((Matching unit) belongs to an enemy of SS_Dummy_Owner[SS_Dummy_ID]) Equal to True) and (((Matching unit)

Instead of this heavy long script, you can just filter the units inside of the unit group using the ITE ( If then else condition ).

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • (Number of units in SS_UnitGroup) Equal to 0
  • Then - Actions
  • Trigger - Turn off (This trigger)
  • Else - Actions

Checking if the group is empty is slow, you can just use integers as counters.

This is a quick review, if there's anything wrong or mistake in my review, you can vm me . Sorry because I've forgot how to mod a spell in warcraft 3 now :3.

 

[BPower]

"- dont use custom value"
This is needed when using Unit Indexer.

He isn't manipulating the UnitUserData at all.
UnitIndexers are very powerful resources. There is nothing wrong in using them.

However the ForGroup loop isn't very efficient compared to an array with an index.
This one will most likely never happen. You have like 0 chance to hit excacly 600.00

• SS_Dummy_Range[SS_Dummy_ID] Equal to 600.00

You damage units multiple times.

 

[Destination9747]

He isn't manipulating the UnitUserData at all.
UnitIndexers are very powerful resources. There is nothing wrong in using them.

However the ForGroup loop isn't very efficient compared to an array with an index.
This one will most likely never happen. You have like 0 chance to hit excacly 600.00

• SS_Dummy_Range[SS_Dummy_ID] Equal to 600.00

You damage units multiple times.

Thank you for your comment, actually it's a 100% chance to get in 600 range
because every 0.03 sec it will do this: Range = Range - 25
and the first range is 900, and after .36 sec it will run the actions cause (12 * 25 = 300) and (900 - 300 = 600) and i have to point that the spell does multiply damage. It's like the Invoker's Chaos Meteor in DotA

 

[Kazeon]

okay, it will hits 600, but how if the user modifiy the speed to 33? it wont hit 600

 

[Destination9747]

yeah you are correct, but the user can change the range too

Edit: I try to make a new range variable that if it passes 300 range, do the actions. thanks

 

[BPower]

• Configuration is lacking. You should add alpha (transparency), animations, ability ( looks cleaner and avoids permanent hex to dec conversion),
  dummy, damagetype, attacktype, ...
• Dynamic indexing would be so much nice than your giant ForGroup loop. You also wouldn't need UnitIndexer anymore.
• The warstomp string path should be stored into a variable
• DamageDetection per se is not part of your spell it shouldn't be there.
• You damage units multiple times, hence damage is directly related to the unit collision size. It's also a complete random value depending on the current movement of the target.
• The hotkey is not to R as mentioned in the tooltip it is C.
• The dummy abruptly vanishes, that looks kinda odd.