Rolling Cutter v1.8 GUI/MUI

[Rheiko]

The spell description made me chuckled. xD
Dude just throws his blade and hopes for the best.
This is the tale of a desperate yet lucky blademaster,
because the blade somehow comes back to him, lol.

This is awesome.

Anyway! I personally prefer to have a configuration in separate trigger. RoCu INI might be just the perfect place to put all your config at.
I'd suggest to create a proper documentation while you're at it. Both in config trigger and the main trigger (cast and loop). It can be helpful for user to adjust the settings to their likings and for fellow coders to save time trying to understand how things work.

 

[Sindobato]

thx im still new in the preloading stuff xD wish I can use bribe's system

 

[Rheiko]

No worries, you're doing great!

You can try to look at things from the perspective of a normal user who doesn't understand MUI and stuff.
If you were one of them, what would you try to change? From there, you can try to make it configurable. =)

Don't forget to explain what you're doing with different parts of the code. Readability is important!

 

[Rheiko]

Could have bumped the thread to let us know that you made an update! Here comes my full review.

---

| Rheiko | Oct 7, 2024

---

Review:

• • Set RoCu_MaxDuration[1] = (RoCu_Duration[1] x (Integer(33.30)))
  should be at the most bottom section in the config. User doesn't need to know the final calculation. You can also add a warning not to mess with that part of the trigger.
• • Animation - Play RoCu_Caster[RoCu_Mui]'s attack stand animation
  • Animation - Play RoCu_Caster[RoCu_Mui]'s attack slam animation
  • Animation - Queue RoCu_Caster[RoCu_Loop]'s stand ready animation
  all should be easily configurable by the user as different model might have different animation name.
• • RoCu_CastTime[RoCu_Loop] Equal to 21
  "21" should be easily configurable by the user as different model might have different animation timing.
• • Animation - Change RoCu_Dummy[RoCu_Loop]'s size to (RoCu_AoE[RoCu_AbilLVL[RoCu_Loop]]%, RoCu_AoE[RoCu_AbilLVL[RoCu_Loop]]%, RoCu_AoE[RoCu_AbilLVL[RoCu_Loop]]%) of its original size
  is currently tied to RoCu_AoE, the dummy size should have its own configuration. You can name it RoCu_DummySize or something.
• • Set RoCu_TrueSpeed[RoCu_Loop] = (Random real number between 5.00 and 7.00)
  this should also be configurable. MinSpeed & MaxSpeed. Don't forget to put a note when the values are applied.
• You forgot to use your RoCu_Dummy_Loc[1] variable here:
  • Set RoCu_Angle[RoCu_Loop] = (Angle from (Position of RoCu_Dummy[RoCu_Loop]) to RoCu_CasterLoc[RoCu_Loop])
• You forgot to use your RoCu_VictimLoc[RoCu_Loop] here:
  • Set RoCu_Angle[RoCu_Loop] = (Angle from RoCu_Dummy_Loc[1] to (Position of RoCu_Victim[RoCu_Loop]))
• You might want to adjust your RoCu_Group to exclude buildings, I don't see why a blade can cause significant damage to a building and why it would target a building in the first place, but you can make it configurable if you prefer.
• Attack type and Damage type when dealing damage could also be made configurable.
  • Unit - Cause RoCu_Caster[RoCu_Loop] to damage (Picked unit), dealing RoCu_Damage[RoCu_AbilLVL[RoCu_Loop]] damage of attack type Normal and damage type Normal
• • RoCu_Counter[RoCu_Loop] Greater than or equal to 3
  This could also be configurable. Just let the user know that 3 equals to 0.09 second interval of damage dealt

Aside from configurability issue and a small leak issue, this is such a clever and creative way of triggering. Code wise, this also looks good. I would suggest to make this spell chop down some trees along the way, it only makes sense. One possible issue I could think of is the difficulty to balance this spell. sometimes it deals too much damage and other times it deals way less damage than it should, luckily the damage and max damage values are configurable so user can fine tune the spell to suit their likings. I'll rate it 3 stars for now, but if you can fix the things I mentioned and even add a destroying tree capability. I'm going to rate it 4 stars.

---

Status:

Awaiting Update​

---

Useful links:

• GPAG - GUI Proper Application Guide
• Memory Leaks
• How to get your GUI spell approved

---

 

[Sindobato]

oh man im out of the loop okay ill see what i can do xD

 

[Rheiko]

Some changes are made. Great! Just to add a few things:

• You should store the ability ID into a variable and place it inside the config trigger. This can become a failsafe, in case the user make a mistake when importing the spell or if the copied ability has a different ability ID for unknown reason after they import it to their map, they only need to re-assign the variable with the correct ability from config trigger.
  • Set RoCu_AbilityID = Rolling Cutter
  This led to my second point;
  
  
• If you do store the ability into a variable. You should use that variable in your trigger. Some of them are:
  • (Ability being cast) Equal to Rolling Cutter
  =>
  • (Ability being cast) Equal to RoCu_AbilityID
  and also this:
  • Set RoCu_AbilLVL[RoCu_Mui] = (Level of (Ability being cast) for RoCu_Caster[RoCu_Mui])
  =>
  • Set RoCu_AbilLVL[RoCu_Mui] = (Level of RoCu_AbilityID for RoCu_Caster[RoCu_Mui])
  
  
• • Set harvest_loc = ((Point(0.00, 0.00)) offset by (0.00, 0.00))
  Location with offset is such an unnecessary thing to do to create your harvester because it calls an unnecessary function to do an unnecessary calculation. You can simply replace it with (Center of (Playable map area)) or Point(0.00, 0.00), whichever you prefer.
  
  
• You should store the initial angle into a variable. I believe RoCu_Angle is usable for that purpose. Use it here:
  • Set RoCu_TempLoc[RoCu_Mui] = (RoCu_CasterLoc[RoCu_Mui] offset by 45.00 towards (Angle from RoCu_CasterLoc[RoCu_Mui] to RoCu_TargetLoc[RoCu_Mui]) degrees)
  and then you should use it again in your loop trigger right here:
  • Unit - Create 1 RoCu_Summon[RoCu_Loop] for (Owner of RoCu_Caster[RoCu_Loop]) at RoCu_TempLoc[RoCu_Loop] facing (Angle from RoCu_CasterLoc[RoCu_Loop] to RoCu_TargetLoc[RoCu_Loop]) degrees
  It's usually a good practice to call a function once and then use a variable to store that information so you can keep reusing it whenever needed. Using a variable is generally faster than calling a function. For this case, you only need to do the angle calculation once and you can use it twice.
  
  
• You have this in your If Condition in your loop trigger:
  • ((Picked unit) is A structure) Equal to False
  You probably forgot to remove it since you already use ((Matching unit) is A structure) in your group variable.
  
  
• In your loop trigger right here:
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • RoCu_RightSpin[RoCu_Loop] Equal to True
    • Then - Actions
      • Set RoCu_Angle[RoCu_Loop] = ((Facing of RoCu_Dummy[RoCu_Loop]) - (Random real number between 20.00 and 45.00))
    • Else - Actions
      • Set RoCu_Angle[RoCu_Loop] = ((Facing of RoCu_Dummy[RoCu_Loop]) + (Random real number between 20.00 and 45.00))
  it should be:
  • Set RoCu_Angle[RoCu_Loop] = (RoCu_Angle[RoCu_Loop] - (Random real number between 20.00 and 45.00))
  • Set RoCu_Angle[RoCu_Loop] = (RoCu_Angle[RoCu_Loop] + (Random real number between 20.00 and 45.00))
  because RoCu_Angle already contains Facing Angle of the dummy.
  
  
• You made Attack Type configurable but forgot about Damage Type.

That should be it for now. I'll look into this again when you update the bundle. If everything's good at that point, this should be approvable.

 

[Sindobato]

thanks again I also fixed some other things I missed xd

 

[Rheiko]

RoCu_AbilityID shouldn't be an array. Also, can you please elaborate more on some parts of the documentation? Explain every option in the config as concise and as clear as possible. We gotta make sure even a 5-year old can understand how to import and configure the spell. Right now, some of them doesn't have explanation and some that do is not clear enough.

Instead of just saying things like "you can experiment with this but I would just leave it as is", you can say something more descriptive like "This is the distance at which the blade will start to drift / move in circular motion". Be more descriptive is what I'm saying.

You can group the changes you made for each version of the spell inside a changelog, you can use the [hidden=Changelog][/hidden] tags for that and place the changelog at the most bottom of your description. You must also include an importing instruction and spell description as well as a spell preview in your description.

 

[Sindobato]

updated again lol xdd

 

[Rheiko]

Nice! I see you've added some things I requested. But perhaps you can explain a little more some of the stuff in config. Like what is DummyType and what should the user do with it. There are multiple of them, what does that mean. For some of the straight forward ones like Damage, MaxDamage, or Aoe, simply put a little note above it on what it does, e.g. This is how much damage you will deal to the enemy units or this is the area of effect of the projectile. And explain what Ability Code is and the reason it's there.

 

[Sindobato]

ok very quick fix XD

 

[Rheiko]

Great! But still no explanation for RoCu_AbilityID as well as Attack Type and Damage Type. =(

AT and DT handle the damage modifier when they interact with armor type. I believe I already made a rough explanation on AbilityID and its purpose.

Edit: also post the preview images on your description using the [IMG][/IMG] tag.

 

[Sindobato]

No worries =( I added it. I have a condition called short attention span I really appreciate when someone point something out to me =)

 

[Rheiko]

Changes were made. A pretty unique spell that can cut down any foe if you're lucky enough.

Approved

 

[Sindobato]

Recommended...Great! XD thanks

 

[Rheiko]

Here's a thread explaining how Attack Type and Damage Type work, in case you're interested:

WC3's Damage System

Source: This Wc3c thread. Table of Content I. Introduction 1. Attack type (AT) 2. Damage type (DT) 3. Process for damage calculation II. Detailed information about Attack types and Damage types 1. Attack types 2. Damage types 3. Some known examples 4. Notes I. Introduction Damage in Warcraft 3...

www.hiveworkshop.com

Could be handy.

 

[Bleeq]

No worries, you're doing great!

You can try to look at things from the perspective of a normal user who doesn't understand MUI and stuff.
If you were one of them, what would you try to change? From there, you can try to make it configurable. =)

Don't forget to explain what you're doing with different parts of the code. Readability is important!

I used to know what MUI meant. Now I've devolved back down to peasant status

 

[Rheiko]

No need to be so hard on yourself, @Bleeq . MUI stands for Multiple Unit Instanceability.
It simply means an ability can be used at any time by any reasonable number of units without any unexpected behavior occurring.

Let's say we have a DoT (Damage overtime) spell, that last for 3 seconds.
If unit A cast the spell first and unit B cast the spell one second later.
unit B shouldn't interrupt unit A's spell duration.
Unit A's effect should still have 2 seconds left before it expires and unit B's should have 3 seconds to expire as it just begins.
Let's say for some reason, unit A's effect is now permanent after unit B's spell cast or unit A's effect duration restarts when a unit B cast the spell, it's definitely not the expected behavior. It's not MUI. We have some methods to make a spell MUI.

One of the methods is Visualize: Dynamic Indexing, and I want to take this opportunity to recommend it to @Sindobato . Because currently, your way of indexing is, although not a problem, could be optimized further. Currently, your way of indexing keeps increasing the index and waits until there's not a single instance left before the index resets back to zero. Although it is pretty unrealistic to hit the max array index (which is 32768, quite a high number), I believe it's a good practice to reuse the unused instance instead, so you don't unnecessarily loop through inactive instances. It is achievable with Dynamic Indexing and Linked List. Bribe's Unit Indexer also works.

 

[Sindobato]

yea xD my method is an evil shortcut I saw someone used it in their trigger and I thought it was brilliant and it has been stuck with me. I must be rid of this bad habit one day.

Edit: Changed indexing and added some visual effect for vertical projectiles.
Edit 2: quickfix

 

[Fireranger-007]

Looks cool

 

[ReizoBleide]

hi, why does the spell always stops working after upgrading it beyond lvl 3? ive already attempted to adjust the corresponding real arrays to each levels in the world editor, except ofc, the RoCu_MaxDuration integer arrays (essentially triggers in "Do not touch" areas) but to no avail, the problem persists. i would appreciate it if someone can help me thx very much.

 

[Sindobato]

hi, why does the spell always stops working after upgrading it beyond lvl 3? ive already attempted to adjust the corresponding real arrays to each levels in the world editor, except ofc, the RoCu_MaxDuration integer arrays (essentially triggers in "Do not touch" areas) but to no avail, the problem persists. i would appreciate it if someone can help me thx very much.

Hi, there is no problem in the loop but maybe in the INI Config you forgot to adjust specifically this line

• Set RoCu_DriftDis[4] = (RoCu_Dis[4] x 1.50)

and

• Set RoCu_MaxDuration[4] = (RoCu_Duration[4] x (Integer(33.30)))

You must change the array for variable name and the array inside the math too

I know this is the problem because the spell runs on timer, if you did not declare the variable for duration it will automatically be 0, therefore the spell will end as soon as you start it.

Forgive my wording for do not touch, I meant to say you shouldn't alter the calculation but if you increase the level then you should definitely copy and paste then update the array accordingly.

 

[ReizoBleide]

Hi, there is no problem in the loop but maybe in the INI Config you forgot to adjust specifically this line

• Set RoCu_DriftDis[4] = (RoCu_Dis[4] x 1.50)

and

• Set RoCu_MaxDuration[4] = (RoCu_Duration[4] x (Integer(33.30)))

You must change the array for variable name and the array inside the math too

I know this is the problem because the spell runs on timer, if you did not declare the variable for duration it will automatically be 0, therefore the spell will end as soon as you start it.

Forgive my wording for do not touch, I meant to say you shouldn't alter the calculation but if you increase the level then you should definitely copy and paste then update the array accordingly.

ayyy, thx for clarifying!! the spell works above lvl 3 now!