[vJASS] Berserkers Leap ver. 0.01.01[BETA]

[Moderator]

12th Dec 2015
IcemanBo: For too long time as NeedsFix. Rejected.

04:35, 12th Dec 2015
IcemanBo:

 

[Deuterium]

Some minor issues that don't even matter whether you fix them or not, just for the sake of pointing them out to you:

To start off with, I don't think you need to remove the ability of a preloaded unit before removing it from the game, just remove the unit

call SetUnitTimeScale( .Caster, 100 * 0.01)

100 * 0.01 = 1... just put 1

            loop
                set int = int - 1
                exitwhen int == 0
                set Degrees = Degrees + 30
                set DummyX = .CasterX + 100 * Cos(Degrees)
                set DummyY = .CasterY + 100 * Sin(Degrees)
                set Dummy = CreateUnit(GetOwningPlayer(.Caster), DUMMY_ID, DummyX, DummyY,  Cos(Atan2(DummyY - .CasterY, DummyX - .CasterX)))
                set CastX = DummyX + Cos(Atan2( DummyY - .CasterY, DummyX - .CasterX))
                set CastY = DummyY + Sin(Atan2( DummyY - .CasterY, DummyX - .CasterX))
                call UnitAddAbility( Dummy, DA_RAWCODE)
                call SetUnitAbilityLevel( Dummy, DA_RAWCODE, .level)
                call IssuePointOrder( Dummy, "shockwave", CastX, CastY)
                call UnitApplyTimedLife( Dummy, 'BTLF', 0.50)
                set Dummy = null
            endloop

no need to null the Dummy local in the loop as it will be overwritten when you use it...
just null locals at the very end of the function you're using them in

WEAPON_TYPE_WHOKNOWS

just use null

Didn't test it but that's what i got from skimming the code

 

[naitsirk]

Thanks^^ Not something big, I might fix that later in case I get a huge bug I didn't know of that I need to fix then I'll just implement that too while I'm at it
(I will fix it, don't worry)

 

[Magtheridon96]

Just skimmed through the code:

- Since this has requirements, I'd recommend using a library instead of a scope.
- structMembersShouldBeWrittenLikeThis
- Use null instead of WEAPON_TYPE_WHOKNOWS. They both do the same thing but WEAPON_TYPE_WHOKNOWS consumes RAM unlike 'null'
- Use a local trigger in the Init function cause using the global is way too messy and depends on the name of the trigger.
- 0 == int is faster than int == 0 for unknown reasons, but it's most likely due to Hardware

 

[naitsirk]

Just skimmed through the code:

- Since this has requirements, I'd recommend using a library instead of a scope.
- structMembersShouldBeWrittenLikeThis
- Use null instead of WEAPON_TYPE_WHOKNOWS. They both do the same thing but WEAPON_TYPE_WHOKNOWS consumes RAM unlike 'null'
- Use a local trigger in the Init function cause using the global is way too messy and depends on the name of the trigger.
- 0 == int is faster than int == 0 for unknown reasons, but it's most likely due to Hardware

So let me see, the 0 == int is in the Absolute if/then/else and why do I have to write struct members in that way? I haven't seen any requirements for it anywhere, but I can do it, I just don't see why.

I'll use the Local trigger inside the init function, I usually do but I don't know why I didn't here, obviously I did not^^

Update: I updated the spell with the things you guys said, and I now hope you're happy^^

 

[Pharaoh_]

• There are some casts that don't land the hero back, unless he casts the spell again. What is more, the parabola doesn't always happen.

• You had better make the shockwave order a global and the amount of dummies to cast the shockwave(s) (which by extension requires the modification of angle: 360/NumberOfDummies).

•

        call UnitAddAbility(.Caster, 'Arav')
        call UnitRemoveAbility(.Caster, 'Arav')

I suggest viewing this tutorial: http://www.hiveworkshop.com/forums/tutorial-submission-283/how-give-unit-ability-fly-201936/

• Important: Check if the SpellTargetX() and SpellTargetY() are equal to GetUnitX(.caster) and GetUnitY(.caster), because it moves him out of map's boundaries.

• Make the dummies face Degrees variable, no need for that double function call.

 

[naitsirk]

• There are some casts that don't land the hero back, unless he casts the spell again. What is more, the parabola doesn't always happen.

• You had better make the shockwave order a global and the amount of dummies to cast the shockwave(s) (which by extension requires the modification of angle: 360/NumberOfDummies).

•

        call UnitAddAbility(.Caster, 'Arav')
        call UnitRemoveAbility(.Caster, 'Arav')

I suggest viewing this tutorial: http://www.hiveworkshop.com/forums/tutorial-submission-283/how-give-unit-ability-fly-201936/

• Important: Check if the SpellTargetX() and SpellTargetY() are equal to GetUnitX(.caster) and GetUnitY(.caster), because it moves him out of map's boundaries.

• Make the dummies face Degrees variable, no need for that double function call.

hmm... Well I'll do that, but I don't really know why the parabola doesn't always happen, I have a small clue why though, I think it's because sometime it's a negative - a positive coordinate which makes the parabola not work properly, I think I can make it work though, but it needs testing so can take a while.
and I'm not really sure what you meant by the important notice, I think you meant when the spell was complete but I am not sure^^

Hmm I would like if you explained a bit more on the dummies/shockwave thing but I might know what you mean, I believe I know what you mean.

And Thanks for the Pheed back, I love it^^

Update: Hmm, I find that strange, I just added a BJDebugMsg and then suddenly the code stopped working as it should, It would never stop moving the unit. then I put the BJDebugMsg into a comment and all ran just fine. I wonder why...

 

[Bribe]

0.01 for the timeout ? You don't need something so fast!

1) Don't use TimerUtils for something so low of a timeout

2) That should be configurable.

variablesAndMethodsAreWrittenLikeThis NotLikeThis.

Your indentation is messed up.

You don't need "call UnitRemoveAbility(bj_lastCreatedUnit, RAWCODE)" in the init function.

The type of dummy unit should be configurable.