Nuclear Strike

[Moderator]

10:24, 26th Jul 2014
BPower:
MUI and leakless. Well done.

http://www.hiveworkshop.com/forums/spells-569/nuclear-strike-254203/index2.html#post2562490

old

review: http://www.hiveworkshop.com/forums/spells-569/nuclear-strike-254203/index2.html#post2554822

09:12, 8th Jul 2014
BPower:
http://www.hiveworkshop.com/forums/spells-569/nuclear-strike-254203/index2.html#post2552900

 

[Chaosy]

1. change the event in the constants trigger to map init
2. actions that aren't required for the spell should be in a "this map only" trigger. In your case player gold etc
3. the nuclear strike nuke trigger is really bad, maybe not coding wise but using for each loops like that is really bad. Unless I am missing something.

 

[The6thMessenger]

1. change the event in the constants trigger to map init
2. actions that aren't required for the spell should be in a "this map only" trigger. In your case player gold etc
3. the nuclear strike nuke trigger is really bad, maybe not coding wise but using for each loops like that is really bad. Unless I am missing something.

1.) It's map init. Experience taught me that map init event does not always trigger some triggers.

2.) Then take it off when you use it. It's not part of the entirety of the trigger if you actually give it some thought.

3.) Called MUI, or multi-unit-instance-ability. It allows multiple units use the spell at the same time.

 

[Wrda]

1.) It's map init. Experience taught me that map init event does not always trigger some triggers.

You can't create timers, leaderboards, multiboards and stuff like this in map init, the rest you can do.

 

[The6thMessenger]

You can't create timers, leaderboards, multiboards and stuff like this in map init, the rest you can do.

Okay sure. But then why would it matter? It works, there is no problem. The same thing happens.

If it were to be copied on another map, then of course they will change it to fit what they need. If the trigger is bugged, then that's a concern.

 

[Chaosy]

1. While that's true, those actions are not in your trigger. And sure it works fine but it's not perfect and as a coder you want to have the "perfect" code to achieve your goal.

2. you can't relay on the user knowing stuff, then they could just create the spell themselves right? Your spell should be able to be imported by a person who bought wc3 today basically.
"Always assume people are idiots" - my dad

3.

Spoiler

I am not stupid I know what MUI is,

however instead of using for each integer you should use a loop timer as event. It prevents the loop from bugging. (I am sure you find info on the bug if you google it or search the forum)

 

[The6thMessenger]

1. While that's true, those actions are not in your trigger. And sure it works fine but it's not perfect and as a coder you want to have the "perfect" code to achieve your goal.

2. you can't relay on the user knowing stuff, then they could just create the spell themselves right? Your spell should be able to be imported by a person who bought wc3 today basically.
"Always assume people are idiots" - my dad

3.

Spoiler

I am not stupid I know what MUI is,

however instead of using for each integer you should use a loop timer as event. It prevents the loop from bugging. (I am sure you find info on the bug if you google it or search the forum)

1.) It's perfect to me, and it works without bugs. That is already good for me.

2.) Sure, but then again don't you think you are making a big deal on how people make their constants? That's just being angry why other people don't like orange juice while you love it.

3.) Don't tell me what to do, we have different styles of coding. Just as penmanship, others have different styles of writing. The real issue here is whether YOU think it's perfect, and you are imposing YOUR ideas onto others.

And such bugs only occur when you have altered the used variable of For-loop integer on another trigger, as well as using the preset Integer A or B. http://www.hiveworkshop.com/forums/...u-should-know-when-using-triggers-gui-233242/ (Chapter 4)

Such as using the Temp_Integer_A on two different and currently running triggers. That is why dedicated temporary integers are used as suggested by the link above. otherwise they are fine.

This really falls down into opinion than actual issues.

http://www.hiveworkshop.com/forums/...279/multi-instancible-gui-spell-making-34393/

 

[GywGod133]

@The6thMessenger
I recommend you use 0.03125 instead using 0.04

• Actions
  • Custom script: set udg_Loop = 0.03125
  • Trigger - Add to (This trigger) the event (Time - Every Loop seconds of game time)

 

[The6thMessenger]

@The6thMessenger
I recommend you use 0.03125 instead using 0.04

• Actions
  • Custom script: set udg_Loop = 0.03125
  • Trigger - Add to (This trigger) the event (Time - Every Loop seconds of game time)

Okay, why? The system is modable, it's easy to configure because i added a constants pane, you can just do it yourself when you implement this.

 

[IcemanBo]

I have not read the code yet, but I definitly agree with Chaosy in his second point.
I also think not system related stuff should not be in system code. Just hold the system as clean as possible and seperate it from map-settings or testing stuff.
Will give some feedback for code some later.

 

[Wrda]

• Floating Text - Create floating text that reads at SG_NS_Point[SG_NS_MUI_Index_A] with Z offset 128.00, using font size 12.00, color (100.00%, 0.00%, 0.00%), and 0.00% transparency
• Floating Text - Hide (Last created floating text) for (All players)
• Floating Text - Show (Last created floating text) for (All players matching (((Matching player) is an ally of (Owner of SG_NS_Caster[SG_NS_MUI_Index_A])) Equal to True))
• Floating Text - Change (Last created floating text): Enable permanence
• Set SG_NS_Floating_Text[SG_NS_MUI_Index_A] = (Last created floating text)
• Floating Text - Create floating text that reads . at SG_NS_Point[SG_NS_MUI_Index_A] with Z offset 128.00, using font size 12.00, color (100.00%, 0.00%, 0.00%), and 0.00% transparency
• Floating Text - Change (Last created floating text): Enable permanence
• Set SG_NS_Floating_Text_Dot[SG_NS_MUI_Index_A] = (Last created floating text)

You can only have 100 floating texts max in a map, so if you try to make more, nothing really happens to them. Also, why you enable permanence?

 

[The6thMessenger]

• Floating Text - Create floating text that reads at SG_NS_Point[SG_NS_MUI_Index_A] with Z offset 128.00, using font size 12.00, color (100.00%, 0.00%, 0.00%), and 0.00% transparency
• Floating Text - Hide (Last created floating text) for (All players)
• Floating Text - Show (Last created floating text) for (All players matching (((Matching player) is an ally of (Owner of SG_NS_Caster[SG_NS_MUI_Index_A])) Equal to True))
• Floating Text - Change (Last created floating text): Enable permanence
• Set SG_NS_Floating_Text[SG_NS_MUI_Index_A] = (Last created floating text)
• Floating Text - Create floating text that reads . at SG_NS_Point[SG_NS_MUI_Index_A] with Z offset 128.00, using font size 12.00, color (100.00%, 0.00%, 0.00%), and 0.00% transparency
• Floating Text - Change (Last created floating text): Enable permanence
• Set SG_NS_Floating_Text_Dot[SG_NS_MUI_Index_A] = (Last created floating text)

You can only have 100 floating texts max in a map, so if you try to make more, nothing really happens to them. Also, why you enable permanence?

I'm like that, i like being sure that somethings work. That is why there can be "extra" lines of codes. But i'm just making sure.

I have not read the code yet, but I definitly agree with Chaosy in his second point.
I also think not system related stuff should not be in system code. Just hold the system as clean as possible and seperate it from map-settings or testing stuff.
Will give some feedback for code some later.

Sure, but then seriously if you are an "idiot" (rather uninformed), then you have no business in triggering at all. But then if you can import spells, then you can certainly remove that unnecessary lines of code by a simple delete button.

 

[Wrda]

"Always assume people are idiots" - my dad

This answers your reply to icemanbo.

 

[Daffa]

aka.gywgod133 has a point, either use that or use 0.03, the accuracy is far better and those are mostly used.

Wrda also post important thing, if the floating text doesn't appear it can be ridiculously strange when it appears on another casts.

Chaosy 2nd point is a MUST done for all spells for approval, it makes importing simple for people who wants to use this spell, don't let them get unnecessary pains.

A good note by DIMF is to save Picked Unit and such to variables, recommended to reduce operation lag (as it uses cache instead of recreating data).

 

[IcemanBo]

The naming of the variables make it a pain to read the code. Using one underscope ('_') should be enough. (to seperate system/spell prefix with actual variable name)

Also names like "SG_NS_Temp_Integer_A" are unnecessary long. Something like "NS_Int" would do the job.

So I think the names should be changed, and it will gain a lot of readability.

 

[The6thMessenger]

So I think the names should be changed, and it will gain a lot of readability.

I really disagree. That is how i work.

"SG_" means Spectre Gunner
"NS_" means nuclear Strike
...

everything just makes much more sense.

 

[RealKnight]

Cool! (4/5)

Honestly i can't give rate yet 'till 10 days...

 

[BPower]

Delete

• Player - Set Player 1 (Red) Current gold to 4000
• Game - Set the time of day to 11.99
• Game - Set time of day speed to 0.00% of the default speed

The "Constants" trigger should run on map init.

Any (Conditions) is not required, also store the ability into a variable.

• Or - Any (Conditions) are true
  • Conditions
    • (Ability being cast) Equal to Nuclear Strike

As a logical consequence "Ability beeing cast" is Nuclear Strike.

Put this line directly below the one where you create the floating text.
Use the variable instead of "Last Created Floating text".

• Trigger - Turn on Nuclear Strike Loop <gen>

You may check first if the trigger is already enabled. In my signature you will find a tutorial about proper dynamic indexing.

• Special Effect - Create a special effect at SG_NS_Point[SG_NS_Temp_Integer_A] using war3mapImported\NuclearStrike3.mdx
• Special Effect - Create a special effect attached to the chest of (Picked unit) using Abilities\Weapons\RedDragonBreath\RedDragonMissile.mdl

Please support these effects within the config trigger. We do not appreciate hard-coded spells.

"Picked Unit" could be stored into a variable to avoid needless function calls.

• Set SG_NS_Floating_Text[SG_NS_MUI_Index_A] = (Last created floating text)

 

[The6thMessenger]

Mods?

 

[IcemanBo]

What do you mean? BPower is a mod.

 

[BPower]

First of all change all variable names. C_SG_NS_.... is too much. Label them NS_...

1. Remove this line, it has no use.

• Trigger - Turn off (This trigger)

2. Change this

• Set C_SG_NS_Loop_Interval = 0.04

to

• Custom script: set udg_C_SG_NS_Loop_Interval = 0.04

3. Add an ability variable to the configuration trigger and set it to Nuclear Strike.
--> on spell cast compare the casted ability with this variable
--> Replace "ability beeing casted" with this variable.

4. This line exists two times in the cast trigger

• Set SG_NS_Floating_Text_Dot[SG_NS_MUI_Index_A] = (Last created floating text)

5. This is so-so, an integer comparison of your MUI_Index == 1 would be much better.

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • (Nuclear Strike Loop <gen> is on) Equal to False
  • Then - Actions
    • Trigger - Turn on Nuclear Strike Loop <gen>
  • Else - Actions

6. Put the IsUnitDeadBJ condition on the top

• Or - Any (Conditions) are true
  • Conditions
    • (Current order of SG_NS_Caster[SG_NS_Temp_Integer_A]) Not equal to SG_NS_Order[SG_NS_Temp_Integer_A]
    • (SG_NS_Caster[SG_NS_Temp_Integer_A] is Polymorphed) Equal to True
    • (SG_NS_Caster[SG_NS_Temp_Integer_A] is Stunned) Equal to True

7. Replace "picked unit" with Temp_PickedUnit. (Multiple times below aswell)

• Unit - Cause SG_NS_Caster[SG_NS_Temp_Integer_A] to damage (Picked unit), dealing SG_NS_DMG_Structure[SG_NS_Temp_Integer_A] damage of attack type Magic and damage type Magic

 

[The6thMessenger]

1.) No, that is how i name my variables. That is because they are categorized deeply.

4.) No, there are really two Floating Text Variables. The first one is for the count down, the other is the laser dot.

Updated.

 

[The6thMessenger]

This is going to rot in pending status...

 

[BPower]

There are a lot of variables in the variable editor which are not related to the spell (udex, c_moon_base_damage, ...). They won't get imported, so a fix is not required here.
Don't initialize your array variables with a size of 999. Leave it with the default setting of 1.
Your variable naming kills a little bit of read-ability. (We already talked about this)
Damagetype and Attacktype could be configurable.

• SG_NS_MUI_Index_A Less than or equal to 0

should be

• SG_NS_MUI_Index_A Equal to 0

Effects and tooltips are nice. The spell itself is fun to cast and works flawless. Good Job.

 

[Kam]

If you use a dummy unit with a timed life the explosion effect won't be cut short.

Why are there so many extraneous imports?

 

[EyeCandy]

Hmm...could add a delay that is proportional to cast distance? I like this a lot!

 

[The6thMessenger]

Hmm...could add a delay that is proportional to cast distance? I like this a lot!

Yes and no. You have to add the calculation yourself upon the "Cast" trigger. It will not be a good idea, as it would have been very hard to time.