HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps
Wc3C.net » Warcraft III Modding » Developer's Corner » Triggers & Scripts » Critique Request - vJASS code

Critique Request - vJASS code

06-29-2009, 04:14 PM#1
Zaraf
I finished my first complete tower that I coded myself, and I'd like if I could get some critiques on it please.

Here is a description of how the tower works:

You upgrade the generic elemental tower into a Generator tower. The generator tower has a "Build Link Tower" ability which you use to construct a link tower (based upon the build mini-structure item ability). There is a limited range in which you can construct the link tower, which is displayed by a blue ring of special effects (the ring is only shown when the generator tower is selected). Once you construct the link tower, a lightning effect link is established and any ground enemy unit that touches it gets damaged. If your builder touches the lightning, then he gets slowed for a short duration. After a cooldown, you can destroy and rebuild the link tower in a different location if you wish.

This tower is designed so that each player can only have ONE at a time, so I realize that it's not MUI to have multiple copies of the tower per player, but it doesn't need to be for the purposes of my map. Also, the generator can only build one link tower at a time, and must destroy the old link tower before you can build a new one.

This code uses Ammorth's LineSegment library and Vexorian's SimError.

Some globals that are used (that are not part of the code) are:

1. BuilderChosen (which is an array that contain's every player's builder)

2. Generator tower ids (GENERATOR_TOWER_ID, GENERATOR_2_TOWER_ID, GENERATOR_3_TOWER_ID)

3. SELL_ID and DEMOLISH_ID (abilities for selling and demolishing towers)

4. ENUM_GROUP is an empty global group that is used, but never filled.

5. PlayerArea is an array that contains the player's entire playing area rect. This is to prevent the range circle thing from appearing outside of the player's area.

Generator main code:
Expand JASS:

Color Init: (this is where the RedColor, GreenColor and BlueColor arrays are initialized)
Expand JASS:

Here is a screenshot of the tower in action:

Hidden information:
Zoom (requires log in)


Please keep in mind that I'm new to vJASS coding, and so I'm asking for this critique to help improve my coding. If you would like to try the tower out, please let me know and I'll send you the map and show you how to test the tower. I can't attach the map here since this is part of the rest of my map.

Thanks for any help!

EDIT: Whoops forgot to add in description of tower. Thanks Anitarf.
Attached Images
File type: jpgGenerator.jpg (252.7 KB)
06-29-2009, 05:12 PM#2
Anitarf
I really don't understand how you aren't able to make a MUI code in vJass.

Also, what is the tower supposed to do, I don't see any explanation of it.
06-29-2009, 05:29 PM#3
Zaraf
Whoops forgot to add in a tower description. Now added, thanks!

As for the MUI, it's not that I can't make it MUI for multiple towers per player, but it's that my map doesn't require it. Every player is limited to constructing only ONE of this tower at a time (if they sell it, they can build it again). It is not possible for a player to have 2 generator towers at the same time. So then why complicate the code further to add a functionality that would never be used?

If I ever did decide to allow more than one (which I don't expect I ever will), then I could go back and modify it accordingly.

However, I do believe that this code is MPI. So that each player can have a single generator without any problems. Please don't make an issue out of "You should make it MUI even if you don't need it!" Thanks.
06-29-2009, 06:13 PM#4
Archmage Owenalacaster
Quote:
Originally Posted by Zaraf
Please don't make an issue out of "You should make it MUI even if you don't need it!" Thanks.
I've already tried; his vision is narrowed to the scope of his map.

Summary
A "It's more efficient to program for MUI."
Z "But I don't need it, so how is that more efficient?"
A "It will save you time in the future if you decide you want to reuse the code."
Z "Then I can just go back and modify it."
A "But it's good practice for a good programming habit."
Z "I don't need it and I'd like to avoid confusing structs."
06-29-2009, 06:13 PM#5
Blubb-Tec
well, if you like it, and if you like how it looks(which i think is cool btw), and if there are no bugs, i don't really feel like reading all of your code, its just long. however, tell me if you'd like feedback on specific parts of the code... that would be a bit more detailed then.
also, there exists a tag called [ hiddenjass] and [ /hiddenjass], that one looks better.

edit: oh yeah, detailed and good first post btw, explaining a bit more about the internal workings of your code would definitely be helpful though.
06-29-2009, 06:49 PM#6
Archmage Owenalacaster
Quote:
Originally Posted by Blubb-Tec
i don't really feel like reading all of your code, its just long
This is bad form. You ought not to post something useless.

I expect there should be a more efficient approach to handling the colors, Z. Have you considered using a formula instead of such unwieldy arrays?
06-29-2009, 07:11 PM#7
Zaraf
Quote:
Originally Posted by Archmage Owenalacaster
This is bad form. You ought not to post something useless.

I expect there should be a more efficient approach to handling the colors, Z. Have you considered using a formula instead of such unwieldy arrays?

Yeah, but wasn't sure what kind of a formula to use. Despite being an engineer, I suck at creating formulas ;) Probably cause I'm a chemical engineer and I'm better at making dangerous chemcials than I am at mathematical formulas ;)

But these are the steps in the way the color changes with regards to RGB.

The numbers refer to percentages. R 100 = Red 100%, G 0 to 100 = Green incrementing from 0% to 100%, B 0 = Blue 0%

The increments I used were 10% increases each time.

step 1 (starting): (R 100) (G 0) (B 0)
step 2: (R 100) (G 0 to 100) (B 0)
step 3: (R 100 to 0) (G 100) (B 0)
step 4: (R 0) (G 100) (B 0 to 100)
step 5: (R 0) (G 100 to 0) (B 100)
step 6: (R 0 to 100) (G 0) (B 100)
step 7: (R 100) (G 0) (B 100 to 0)

and then you're back to step 1. However, there is one difference in my array. I've eliminated some of the color steps. Mainly on step 5, instead of Green going from 100% to 0%, I have it go from 100% to 50%, and Red also has a sudden jump to 50% and increases to 100% (instead of 0% to 100%). The reason for this is that this range of color looks very bad for the lightning effect I used. It becomes syncronized with the lightning's default colors, and just looks like fat blue lightning effects. So I just cut those out, and when the lightning starts turning blue, it reaches a point where it just jumps to a purple. This was another reason I thought to avoid a formula since I would have to account for this exception.

I realize this might be a bit confusing to understand, but I hope it was clear enough :)
06-29-2009, 07:22 PM#8
Anitarf
Quote:
Originally Posted by Zaraf
Please don't make an issue out of "You should make it MUI even if you don't need it!" Thanks.
Okay, then I won't.
06-29-2009, 08:01 PM#9
Zaraf
Quote:
Originally Posted by Litany
It's not that you should specifically make it MUI so much as you should make it as generalizable as possible. Tomorrow you might decide "Hey, I could use the same setup to create tripwires that slow units for a short duration," and had you coded this better in the first place it would only take you a few extra minutes to implement the tripwires.

Not really the case with this tower. Since it's quite specialized. The things that could be generalized have already been established (LineSegment). My lightning color thing and the range thing are things which I might want to add to other towers, but the way they are right now, they by themselves are already MUI if I were to put them on other towers.
Wc3C.net archive — static reconstruction. No accounts, no posting.
Hosted by www.hiveworkshop.com