SC2Mapster

Which way should I code this?

A tutorial I found taught me how to assign players to teams, but since I'm trying to learn how this jolly good business works, I thought I'd try to do it with a loop.

He used 6 seperate actions to assign 6 players like so:

(First column is the team, second is the player number)

This is solution 1 for looping it:

Solution 2 (which I don't like as much, but I'm a noob so it may be better):

So which one is better and can I improve the code?

@FoxyMayhem: Go

The Or is the better way of doing things.......

You generally want to strive for not repeating yourself unless it it necessary.

But after your done coding a few things hard coded that if you step back a bit you can generally clean things up further.

On a side note. Those while loops are a bad sign to me.

Well, if your goal is to just express the statements of your first picture in a loop,I would go for (pseudocode):

for (int i=1;i<=6;i++){
     AddPlayerToTeam((i+2)/3,i);
}

The (i+2)/3 is an integer division, so it should round downwards always, meaning

i = 1 -> (1+2)/3 = 1
i = 2 -> (2+2)/3 = 1.33 = 1
i = 3 -> (3+3)/3 = 1.67 = 1
i = 4 -> (4+2)/3 = 2
... (you get the idea)

But honestly, for triggers you don't exactly need to worry about the most elegant solution. I would recommend to either use the shortest solution to save development time or the most accessible, the easiest to understand, which would be your first, without a loop.

€ In gui triggers, it would look something like this:

    Local Variables
        i = 0 <Integer>
    Actions
        General - For each integer i from 1 to 6 with increment 1, do (Actions)
            Actions
               Tribe Select: AddPlayerToTeam(((i+2)/3),i)

@Kueken531: Go

He should really be setting teams in the Variant file.

Setting teams in game like that will just complicate things....

I have a library available that will read the alliance properties of the players from the teams configured in the game variant file and figure it out for you. It creates

• Player groups for each team that you can refer to.
• Also gives you and index that tells you what Team a specific player is on.

http://www.sc2mapster.com/forums/resources/trigger-libraries/43572-library-team-sort-library-leaderboard/#p1

It's great that you are looking at how to better structure your code. So often people only concern themselves with what works. This usually backfires and they are left with a mess which is impossible to understand and fix/extend/debug. I have to re-learn this lesson every time I code :P

First off: NEITHER of these solutions is correct. They both have bugs in them that will prevent them from working in their current form* (solutions at the back). Example 1 will assign people to the wrong teams and example 2 will loop infinitely until the game client times out and kills the process.

However I can see where you are going and in terms of //**style**// the first one is much better for a two reasons i can think of:

READABILITY- The first one is much more concise, there is only 1 loop and one if statement so the logic is simpler. At a glance you can tell that tmpPlayerInteger ranges from 1 to 6 (can't do this in example 2) and you can tell what logic is used to assign a player to a team.

MAINTAINABILITY - The first one is quite easy to edit if you want changes or if you want to copy+paste it to another map. I cannot stress how much easier and less stressful your time will be if you code with an eye towards maintainability. Say for example you wanted to make it 4v4 or if you wanted to make the teams 4v2. For both of these it would be easy to change the code in example 1, simply edit the loop and if statement. Changing the code in example 2 would very likely introduce bugs as the logic flows from one while loop to the next so you would need to track how each while loop effects the next one a much more error prone task.

A few points (not nescessarily about the code examples you showed but just some advice)
1) Variable names makes a big difference in readability. naming a variable "Player" is simple and can be read at a glance unlike "tmpPlayerInteger".
2) Spread comments throughout your code to make it easier to understand. You might know what is going on but if you take a break and come back in 2 weeks time the comments are going to really help you.
3) use functions whenever there is a block of logic which you want to write once and then forever after think of conceptualy as a "black box". where you can call that function and not have to worry about how it works.

my solution would be

//variables://
Player (integer)
Team (integer)
Team_Members (integer)
Number_of_players (integer)
Number_of_teams (integer)

//Actions://
Number_of_players = 6;
Number_of_teams = 2;

Player = 1;
for each variable ( Team ) from ( 1 ) to ( Number_of_teams )
     for each variable ( Team_Members ) from (1) to (number_of_players / number_of_teams)
          AddPlayerToTeam( Team, Player)
          Player = Player + 1;

Use of "for loops" is easier to read than "while loops" and easier to code (although if you are just begining while loops may be a simpler starting block).
This is much more flexible as changing **Number_of_teams** = 3 will result in a 2v2v2. Changing **Number_of_players** = 8 will result in 4v4 etcetcetc
If you are unfamiliar with the "for loop" it is just like a "while loop" except that it lets you assign the starting vale and how much you increment by.

* //answers in the back of the book//
Example 1 - oops, sorry I misread your code, it might actually work... but here is a much cleaner and simpler way of doing what you are doing:

While player <= 6
     if player <= 3
          AddPlayerToTeam(1, Player)
     else
          AddPlayerToTeam(2, Player)
     Player = Player + 1

In fact I think this is what you should do. It is still clean and elegant but less confusing than the solution I gave so is probably better for you while still learning.

example 2 in the code you gave is broken at the second "while loop". While player == 3 you are incrementing team. However player is not being changed inside the loop so player == 3 will always be true and the while loop will continue to loop forever (except that SC2 detects this after a few seconds and kills the process). remove the while loop and just put "team = team + 1" and example 2 should work.

EDIT: I misread the code example 1 that you gave. But read the solution any way because it is a lot simpler and cleaner.

Thanks for the posts guys! I love your novel finite, very helpful.

Few questions:
1) Why do we set the varibles Number_of_teams and Number_of_players with an action, instead of setting their start values?

2) In the "Common Sense 101 (triggering edition)" the writer goes on about how while loops are superior. I prefer the look of For loops, but I've been learning to use while loops because of his advice. Why should I use For loops, if while loops are so much better?

3) The "Team_Members" variable starts at 0 and is not modified in the actions. I don't understand how "For Each Integer" works when there are 0 Integers in the variable.

Also, your "back of the book" answer is so clean and simple! I face palmed when I read it.

Thanks again.

Quote from FoxyMayhem: Go

1) Why do we set the varibles Number_of_teams and Number_of_players with an action, instead of setting their start values?

For organization purposes. You could set the initial values as well, it would work just fine.

Quote:

2) In the "Common Sense 101 (triggering edition)" the writer goes on about how while loops are superior. I prefer the look of For loops, but I've been learning to use while loops because of his advice. Why should I use For loops, if while loops are so much better?

For loops used to generate huge amounts of code for no good reason, especially when nested. Nesting like 5+ for loops would result in thousands of unneeded code lines in the resulting map script. However, that got fixed, and for loops are specifically designed for this kind of situation, where you want to loop over an incrementing variable.

Quote:

3) The "Team_Members" variable starts at 0 and is not modified in the actions. I don't understand how "For Each Integer" works when there are 0 Integers in the variable.

The "For Each Integer" loop takes a variable, sets it to the start value, then repeats the associated action and increases the variable by the increment for each repeat, as long as the condition is met. This is all done internally, without you having to set start values or increment the variables manually.

Also, as you can see, everyone does his stuff differently, for different reasons. In the end, just do it the way you like the most.

@FoxyMayhem: Go

Quote from FoxyMayhem: Go

1) Why do we set the varibles Number_of_teams and Number_of_players with an action, instead of setting their start values?

Part of the reason you would do it with actions is if your making it semi-dynamic.

Such as 4 people are playing your map online instead of the 6 expected players.

This is part of the reason you Need to define teams in the Game Varient File for online games.

As I said before My TeamSort Library Will Handle sorting any combination of players from the Game Varient File Into Usable PlayerGroups and more

Quote from FoxyMayhem: Go

2) In the "Common Sense 101 (triggering edition)" the writer goes on about how while loops are superior. I prefer the look of For loops, but I've been learning to use while loops because of his advice. Why should I use For loops, if while loops are so much better?

When your looping through a set of numbers 0-100

• use a for loop

1. For each i from 0 to 100 (do actions inreference to i) where your using the value of i to do things

When you need to do something for an indefinite amount of times such as moving a dialog items x.y values until they are no longer with in the bounds of its parent dialog. You would want to use a While Loop

• use a while loop

1. While X offset of dialog item < width of parent dialog (Set X offset of dialog item = X offset of dialog item + 10)

Quote from FoxyMayhem: Go

3) The "Team_Members" variable starts at 0 and is not modified in the actions. I don't understand how "For Each Integer" works when there are 0 Integers in the variable.

Team_Members in FiniteTurtules code is just a temp interger variable no different then saying

for i from 0 to 10

the for statement runs 11 times and each time through i is incremented by 1 through the specified range.

Thank you guys. I actually like while loops better, I feel I have more control without sacrificing much readabliity.

Soul, yeah I'll be defining teams the way you suggest. I'm just using these triggers as a way to get familiar with how they run.

I used what I learned here to create an awesome scaling dialog, where you only have to change total number of buttons, button size, row length, etc, and it will all scale correctly. Man does that feel good.

Now I'm trying to reference a button press in 1 dialog to affect another. How do I specify a specific dialog, rather than just using "Last Created Dialog"?

@FoxyMayhem: Go

You'll have to make variables to store dialogs/dialog items to reference them later. Go something like this, make dialog, assign that to a dialog variable using last created dialog. Make dialog item, make dialog item variable using last created dialog item, etceteras. Arrays are good for dialog items if you aren't using them already.

Quote from FoxyMayhem: Go

Thank you guys. I actually like while loops better, I feel I have more control without sacrificing much readabliity.

Soul, yeah I'll be defining teams the way you suggest. I'm just using these triggers as a way to get familiar with how they run.

I used what I learned here to create an awesome scaling dialog, where you only have to change total number of buttons, button size, row length, etc, and it will all scale correctly. Man does that feel good.

Now I'm trying to reference a button press in 1 dialog to affect another. How do I specify a specific dialog, rather than just using "Last Created Dialog"?

You actually have less control in while loop. Where as in a for loop you specify the start point and end point within the actual for statement.

• for i = 0 to 10
  • {
    • Get Player(i) name
  • }

or

• i = 0
• while( i <= 10)
  • {
    • Get Player(i) name
    • set i = i + 1
  • }

See how many more lines of code are required to do the looping of 0-10. While loops are designed for more complex logic then.

If you worked for me and you coded like that .... I would flay a small piece of skin off your one of your fingers for every while loop of this manner I found in your code.

Er, how is that any less control? They do the exact same thing. Maybe it's just my unfamiliarity with For loops and that 101's bible bashing on using while loops that's making be biased, though.

So... I really should start using For loops, huh?

@FoxyMayhem: Go

you have more control in a for loop because....

in a while loop you have to set the increment seperately

where as a For loop is more controlled because you cant accidentally forget to increment the variable.

When you do more advanced stuff such as

• For each Player i in playerGroup
• {
  • Get Player(i) Name
• }

pick each function work the same way

• Pick each player in PlayerGroup
• {
  • Get Player(Picked Player) Name
• }

the logic stays the same and you should seriously get used to using them

It is extremely rare you will need to use a while loop.

@SouLCarveRR: Go

Boo, bad while loop example. Be much better to go

{
int i = 0;
while(i < 10)
{
do some important action
i += 1; if this wasn't for stupid SC2 scripting, I'd instead say + +i
}
}

You are absolutely correct FoxyMayhem, there is no difference, for loops are nothing more than a specialized while loop. They are both interchangeable, for loops are just more compact typically. I prefer to think in while/dowhile loops really, for loops only provide more compactness, and a more easily recognized temporary variable.

For loops give you no more control then a while loop when you get down to it, I have no idea why someone would think that. What reasoning for this do you have soul, if anything a for loop tends to be more restrictive to me.

@Deadzergling: Go

Restrictive is more control.....

That's just how programming works kids.

You can do more things with a While loop. Such as more complex logic.

Basically using a while loop makes things more complicated and should only be used when necessary.

KISS = Keep it simple stupid..... a rule all programmers live by

@SouLCarveRR: Go

Lol, thanks for the word definitions reminder. I think we are pretty much in agreement really, on all things besides the use of the word 'control'. Still, I am a bit hesitant to tell beginners to start with for loops, find the learning goes better if I force them to use lots of if statements, then while loops, then let them use for loops. Sinks into their memory a lot better this way from my experience.

@FoxyMayhem: Go

Anyways feel free to chime in any time FoxyMayhem, don't mind us arguing minor details.

I feel very comfortable with IfThenElse and While loops, so I'll play around with For loops.

The interface is going really well (what this practice has gone to). Now I'm just trying to figure out how to reference a dialog aside from Last Created Dialog.

Dead, what does int i mean in your example? I don't understand it.

lol.

I think while loops are less scary for people who are just starting to learn how to code. Even if they use them exactly like a for loop at least it gets them thinking about the actual logic of the problem.

If you worked for me and used while loops instead of for loops for simple tasks... I would wait until you'd had a few months of experience behind you before I started flaying skin :P join my hypothetical coding team instead, I'm less of a harsh task master hahahaha.

EDIT:
When he wrote "int i = 0;" he is blending into other languages. It is very common in C like languages to declare an integer variable like that.

It is just like creating a variable in the trigger editor of type "integer" and setting the default value to 0.

@FoxyMayhem: Go

A bit offtopic perhaps, but:

I don't know if the tutorial mentioned it, but I would include a check inside the for/while loop to make sure that the player with index i is a human and active (if thats important for the game)

Quote from finiteturtles: Go

When he wrote "int i = 0;" he is blending into other languages. It is very common in C like languages to declare an integer variable like that.

It is just like creating a variable in the trigger editor of type "integer" and setting the default value to 0.

Thats just galaxy script, the language triggers are converted in ;) It is indeed C-like.