Alliance Handler v1.5

[Moderator]

IcemanBo: Changed were made. System works as wanted.
https://www.hiveworkshop.com/threads/alliance-handler-v1-5-a.261543/post-2644405

Old

IcemanBo: https://www.hiveworkshop.com/threads/alliance-handler-v1-4-a.261543/post-2643963

02:13, 20th Jan 2015
IcemanBo: https://www.hiveworkshop.com/threads/alliance-handler-v1-3-a.261543/post-2642167

 

[Chaosy]

Could prove useful. However I have a few concerns if you may call it that.

critism

1. What's the benefit of using this compared to the system you based it on? You might want to list them clearly so people knows without looking at the code.

2. I think you can shorten down the code by half (if not more) unless I am missing something. What are you using all those if conditions for? basically you only need to make the player ally a certain player? and then sort out a few buggy exceptions like allying yourself. That shouldn't take more than a few lines.

I might be completely wrong, but if so, please enlighten me x)

suggestions

1. use a variable for triggering player.
2. merge the two triggers into one.
3. Allying with a player should be more complex. For example, if you ally yourself with player Yellow, and player yellow is allied with player Oranage, you should automatically be allied to him as well. Same goes for enemis, if you're allying someone who is enemy with purple, purple should be your enemy too.

 

[zv27]

Too many conditions.
Using integer a and b.
Given that this is too simple, there is too much line.
Generally poorly coded.

 

[Faith]

Could prove useful. However I have a few concerns if you may call it that.

critism

1. What's the benefit of using this compared to the system you based it on? You might want to list them clearly so people knows without looking at the code.

2. I think you can shorten down the code by half (if not more) unless I am missing something. What are you using all those if conditions for? basically you only need to make the player ally a certain player? and then sort out a few buggy exceptions like allying yourself. That shouldn't take more than a few lines.

I might be completely wrong, but if so, please enlighten me x)

suggestions

1. use a variable for triggering player.
2. merge the two triggers into one.
3. Allying with a player should be more complex. For example, if you ally yourself with player Yellow, and player yellow is allied with player Oranage, you should automatically be allied to him as well. Same goes for enemis, if you're allying someone who is enemy with purple, purple should be your enemy too.

While it may look like a lot of If/Then/Else, using these in GUI makes it big and messy looking. The only ones that are there are the check for allying/unallying yourself, the check for allying/unallying and ally/enemy respectively, and of course whether or not the player is one of specified players to skip. As for merging the two triggers, if you're referring to the ally/unally triggers, that's messy and you wouldn't ever do that, each trigger has it's own purpose. However, if you're referring to merging the two halves of each individual trigger, I was thinking about including that in the next version to shorten things up a bit.

Also, why use a variable for the triggering player? I don't see the purpose of it. And as for the complexity, it's just a base, it wouldn't be hard to add a condition like that. I've done that in the system I use for my own maps, although mine is slightly different.

Too many conditions.
Using integer a and b.
Given that this is too simple, there is too much line.
Generally poorly coded.

Aside from merging the two halves into one larger trigger, you can't reduce the code without sacrificing one of the benefits of using this system over the previous. You can move the main body of the system into where the "Skip Remaining Actions" part is, except into the else function set, however that looks extremely messy as well. There is a compromise to be made here... do you want functionality, or do you want it to look nice. Unless you can point out specific parts that aren't necessary, I'm confident that what is there needs to be there, albeit needing a little bit of tweaking, which I am fully prepared to do. This tweaking will indefinitely reduce the size of the script.

 

[Chaosy]

I did some quick testing, and I think I managed to shorten it down while having the same features as you did. Feel free to take it into account, or not. I feel like I have done my best in helping now anyway

triggers

• Init Copy
  • Events
    • Map initialization
  • Conditions
  • Actions
    • Set playerCount = 10
    • Set skip = True
    • Set skipCount = 5
    • Set skipPlayer[0] = 5
    • Set skipPlayer[1] = 6
    • Set skipPlayer[2] = 7
    • Set skipPlayer[3] = 8
    • Set skipPlayer[4] = 9
    • Set playerColor[0] = red
    • Set playerColor[1] = blue
    • Set playerColor[2] = teal
    • Set playerColor[3] = purple
    • Set playerColor[4] = yellow
    • Set playerColor[5] = orange
    • Set playerColor[6] = green
    • Set playerColor[7] = pink
    • Set playerColor[8] = gray
    • Set playerColor[9] = lightblue
    • Set playerColor[10] = darkgreen
    • Set playerColor[11] = brown
    • For each (Integer B) from 1 to skipCount, do (Actions)
      • Loop - Actions
        • Player Group - Add (Player(skipPlayer[((Integer B) - 1)])) to skip_players
    • For each (Integer A) from 1 to playerCount, do (Actions)
      • Loop - Actions
        • Trigger - Add to Ally Copy <gen> the event (Player - (Player((Integer A))) types a chat message containing -ally as A substring)
        • Trigger - Add to Ally Copy <gen> the event (Player - (Player((Integer A))) types a chat message containing -unally as A substring)

• Ally Copy
  • Events
  • Conditions
    • Or - Any (Conditions) are true
      • Conditions
        • (Substring((Entered chat string), 1, 8)) Equal to -unally
        • (Substring((Entered chat string), 1, 6)) Equal to -ally
  • Actions
    • For each (Integer A) from 1 to playerCount, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Player((Integer A))) Not equal to (Triggering player)
            • ((Player((Integer A))) is in skip_players) Equal to False
            • skip Equal to True
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • Or - Any (Conditions) are true
                  • Conditions
                    • (Entered chat string) Equal to (-ally + (String((Integer A))))
                    • (Entered chat string) Equal to (-ally + playerColor[((Integer A) - 1)])
              • Then - Actions
                • Player - Make (Triggering player) treat (Player((Integer A))) as an Ally with shared vision
                • Game - Display to (All players) for 8.00 seconds the text: ((Name of (Triggering player)) + ( has allied themself with + ((Name of (Player((Integer A)))) + .)))
                • Skip remaining actions
              • Else - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • Or - Any (Conditions) are true
                      • Conditions
                        • (Entered chat string) Equal to (-unally + (String((Integer A))))
                        • (Entered chat string) Equal to (-unally + playerColor[((Integer A) - 1)])
                  • Then - Actions
                    • Player - Make (Triggering player) treat (Player((Integer A))) as an Enemy
                    • Game - Display to (All players) for 8.00 seconds the text: ((Name of (Triggering player)) + ( is now an enemy to + ((Name of (Player((Integer A)))) + .)))
                    • Skip remaining actions
                  • Else - Actions
          • Else - Actions

 

[Faith]

I did some quick testing, and I think I managed to shorten it down while having the same features as you did. Feel free to take it into account, or not. I feel like I have done my best in helping now anyway

triggers

• Init Copy
  • Events
    • Map initialization
  • Conditions
  • Actions
    • Set playerCount = 10
    • Set skip = True
    • Set skipCount = 5
    • Set skipPlayer[0] = 5
    • Set skipPlayer[1] = 6
    • Set skipPlayer[2] = 7
    • Set skipPlayer[3] = 8
    • Set skipPlayer[4] = 9
    • Set playerColor[0] = red
    • Set playerColor[1] = blue
    • Set playerColor[2] = teal
    • Set playerColor[3] = purple
    • Set playerColor[4] = yellow
    • Set playerColor[5] = orange
    • Set playerColor[6] = green
    • Set playerColor[7] = pink
    • Set playerColor[8] = gray
    • Set playerColor[9] = lightblue
    • Set playerColor[10] = darkgreen
    • Set playerColor[11] = brown
    • For each (Integer B) from 1 to skipCount, do (Actions)
      • Loop - Actions
        • Player Group - Add (Player(skipPlayer[((Integer B) - 1)])) to skip_players
    • For each (Integer A) from 1 to playerCount, do (Actions)
      • Loop - Actions
        • Trigger - Add to Ally Copy <gen> the event (Player - (Player((Integer A))) types a chat message containing -ally as A substring)
        • Trigger - Add to Ally Copy <gen> the event (Player - (Player((Integer A))) types a chat message containing -unally as A substring)

• Ally Copy
  • Events
  • Conditions
    • Or - Any (Conditions) are true
      • Conditions
        • (Substring((Entered chat string), 1, 8)) Equal to -unally
        • (Substring((Entered chat string), 1, 6)) Equal to -ally
  • Actions
    • For each (Integer A) from 1 to playerCount, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Player((Integer A))) Not equal to (Triggering player)
            • ((Player((Integer A))) is in skip_players) Equal to False
            • skip Equal to True
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • Or - Any (Conditions) are true
                  • Conditions
                    • (Entered chat string) Equal to (-ally + (String((Integer A))))
                    • (Entered chat string) Equal to (-ally + playerColor[((Integer A) - 1)])
              • Then - Actions
                • Player - Make (Triggering player) treat (Player((Integer A))) as an Ally with shared vision
                • Game - Display to (All players) for 8.00 seconds the text: ((Name of (Triggering player)) + ( has allied themself with + ((Name of (Player((Integer A)))) + .)))
                • Skip remaining actions
              • Else - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • Or - Any (Conditions) are true
                      • Conditions
                        • (Entered chat string) Equal to (-unally + (String((Integer A))))
                        • (Entered chat string) Equal to (-unally + playerColor[((Integer A) - 1)])
                  • Then - Actions
                    • Player - Make (Triggering player) treat (Player((Integer A))) as an Enemy
                    • Game - Display to (All players) for 8.00 seconds the text: ((Name of (Triggering player)) + ( is now an enemy to + ((Name of (Player((Integer A)))) + .)))
                    • Skip remaining actions
                  • Else - Actions
          • Else - Actions

I had already said that I was going to combine both halves of the triggers individually.

I just don't have the time right now. There are a few tweaks I want to make, basically it's what you just did, except I prefer not combining the ally/unally triggers. It doesn't make sense too, they're designed for different things, even if the code is basically the same.

 

[Almia]

Cache Triggering Player( faster == better) into a variable.

Use a custom integer variable for For-Loops instead of Integer A/B


If - Conditions
(Integer A) Equal to skipPlayer[((Integer B) - 1)]
Then - Actions
Set skipValue = True
Else - Actions

->
Set skipValue = (Integer A) Equal to skipPlayer[((Integer B) - 1)]

 

[Faith]

Huge thanks to Chaosy and Almia for helping me reduce the amount of unnecessary script.
It was late at night and looking back I realized I had put in a bunch of absolutely useless things that could have been handled in one-go.

The code has been reduced significantly, and I've added a -vision and -unvision command similar to ones used in "builder"-type games.

 

[Almia]

(skip Equal to True) and (((Player(tempInt[0])) is in skippedPlayers) Equal to False)
->
(skip) and ((Player(tempInt[0])) is in skippedPlayers)

And also whrn using convert player index to player,put it into variable,esp. On those triggers where you used it repeatedly

Also you dont need individual triggers for each chat message events. You can just put seperate if-elses for each condition in just one trigger

 

[Faith]

(skip Equal to True) and (((Player(tempInt[0])) is in skippedPlayers) Equal to False)
->
(skip) and ((Player(tempInt[0])) is in skippedPlayers)

And also whrn using convert player index to player,put it into variable,esp. On those triggers where you used it repeatedly

Also you dont need individual triggers for each chat message events. You can just put seperate if-elses for each condition in just one trigger

I don't think you can shorten booleans like that in GUI.

Why add the player number to a variable? It's already a variable in the loop..

Also, I'm going to include the combination of the ally/unally and vision/unvision in the next update.
I didn't want to do that before because it didn't make sense to. With the addition of the shared vision options, I plan to.

 

[Almia]

I mean like this :
Set thisPlayer = Player(myInteger)


Blah blah. You are like executing it multiple times.
You can also do that i think.

And also i forgot. Prefixes. Temp blah blah is the most overused variable names.

 

[Faith]

What's the purpose of doing that though?

Yeah but it's named tempInt for a reason. It's a temporary integer.

 

[Almia]

For safetiness and Spell Rules


(skip) and ((Player(tempInt[0])) is in skippedPlayers)

I was wrong. Just compare them with Boolean comparison Equal to

 

[Faith]

That's what I did. Thank you.

 

[Almia]

I meant replace "and" and use "Equal to" instead

 

[Faith]

You can't replace the 'and', it specifies that it's checking both conditions, and they both have to be true.
It's basically like putting one condition under the other, I just prefer it this way. It looks nicer.

Updated again with the more efficient code, and the trigger merge.

 

[Chaosy]

You're missing skip remaining actions, otherwise it seems to look somewhat good now ;p

 

[Faith]

There's no actions to skip, is it necessary I include that?

 

[Almia]

I mean remove that line and instead use skip is Equal to (the other condition). Get it?

I cant show you the example because im using my phone right now

 

[Faith]

skip and the other condition can return two seperate values.
I think it's unnecessary to combine that haha.
It's not a logical thing to do. Reading it, it doesn't look right.

 

[Almia]

Not reading. It is all about optimizing.

 

[Faith]

But it won't speed anything up by more than 0.000000000000000000000001 seconds.

 

[Almia]

at least, it is faster

 

[Faith]

Well if people have complaints, I will definitely change it. But for those wanting to learn, I'll leave it as it is for now.
It makes more sense this way, and it honestly doesn't affect it at all.

Thanks a ton for helping me with everything else.

 

[IcemanBo]

I think user should just be informed he can add/remove players to the "SkipPlayergroup", instead of setting it up once onInit and making it not dynamic.

There is no loop needed in main code.

Always use the variable instead of using "TriggeringPlayer()" again.

Remove the "test" message.

 

[Faith]

I think user should just be informed he can add/remove players to the "SkipPlayergroup", instead of setting it up once onInit and making it not dynamic.

The idea is say the map doesn't use a player, or they are in a force that shouldn't be allied/unallied, then anyone would be able to use this to disallow the allying/unallying/whatever of a certain player(s).
I don't think I understand why you would be able to add/remove players to the system in the middle of the game. Or maybe I misunderstood what you said.

There is no loop needed in main code.

What do you mean?

Always use the variable instead of using "TriggeringPlayer()" again.

My mistake, fixed, I just didn't see that. Will update when the rest of the problems have been dealt with.

Remove the "test" message.

Didn't mean to leave that in there haha. Fixed.

 

[IcemanBo]

You currently use a loop to find the matching player, that need to be allied/unallied with. No loop is needed.

I mean you have all these "skip" variables. But I think only the "SkipPlayerGroup" is needed at all. User may just add/remove player to this player force when ever he wants.
At the moment he sets the SkipPlayers once at start, and it's static. Not really changeable anymore, and more effort.

 

[Faith]

You currently use a loop to find the matching player, that need to be allied/unallied with. No loop is needed.

I mean you have all these "skip" variables. But I think only the "SkipPlayerGroup" is needed at all. User may just add/remove player to this player force when ever he wants.
At the moment he sets the SkipPlayers once at start, and it's static. Not really changeable anymore, and more effort.

Without the loop it i believe it undermines the entire player-skipping part unless it was completely redone.
And the players that are being skipped aren't supposed to be changed once they are set. That's the point.

Maybe if you give me an example of what you're talking about I would understand. What you're saying isn't making any sense to me.

 

[Chaosy]

I think I get what Iceman is trying to suggest, at least half way.

You can convert the number entered to an integer and use as an index and therefore skipping the loop.

However, since this system should support "red", "blue" and so on, you need to do something more.

 

[IcemanBo]

Maybe if you give me an example of what you're talking about I would understand. What you're saying isn't making any sense to me.

With only using playernumber as identifier it could be shorten a bit, but with accepting player colour + player names, too... you need some extra lines, but which is okay.

I used hashtable for it. This is an example for ally/unally. You also can see what I meant with only using SkipGroup variable in there.

P.s.: I think it's much more readable to split commands, and make one trigger for each of them.

 

[Faith]

With only using playernumber as identifier it could be shorten a bit, but with accepting player colour + player names, too... you need some extra lines, but which is okay.

I used hashtable for it. This is an example for ally/unally. You also can see what I meant with only using SkipGroup variable in there.

I understand now haha. I will remake it like this, it seems a lot smoother looking at the very least.

P.s.: I think it's much more readable to split commands, and make one trigger for each of them.

I thought so as well, but everyone wanted me to combine it all into one, so I compromised and combined the ally/unally, and combined the vision/unvision, but kept those two seperate.

 

[IcemanBo]

You can combine them if you really want. I won't reject it because of this.

But it doesn't hurt if you create a seperate trigger, and makes it more readable.

 

[Chaosy]

You can combine them if you really want. I won't reject it because of this.

But it doesn't hurt if you create a seperate trigger, and makes it more readable.

Systems should be easy to import and not readable tbh

Just look at Bribes Unit Indexer, I have never, ever, seen such ugly GUI. But it's easy to import, and since I don't need to modify the code, it's fine.

 

[Almia]

Systems should be easy to import and not readable tbh

Just look at Bribes Unit Indexer, I have never, ever, seen such ugly GUI. But it's easy to import, and since I don't need to modify the code, it's fine.

Still needs to be readable and expressive

 

[Faith]

Updated again, a huge thanks the IcemanBo for showing me a better way to handle this.
I know the triggers seem much larger now, but I added a lot of extra documentation to help anyone trying to learn from the system.

Systems should be easy to import and not readable tbh

3 triggers isn't at all hard to import, and it's extremely readable now.

Still needs to be readable and expressive

Yes.

 

[maddeem]

I did some quick testing, and I think I managed to shorten it down while having the same features as you did. Feel free to take it into account, or not. I feel like I have done my best in helping now anyway

triggers

• Init Copy
  • Events
    • Map initialization
  • Conditions
  • Actions
    • Set playerCount = 10
    • Set skip = True
    • Set skipCount = 5
    • Set skipPlayer[0] = 5
    • Set skipPlayer[1] = 6
    • Set skipPlayer[2] = 7
    • Set skipPlayer[3] = 8
    • Set skipPlayer[4] = 9
    • Set playerColor[0] = red
    • Set playerColor[1] = blue
    • Set playerColor[2] = teal
    • Set playerColor[3] = purple
    • Set playerColor[4] = yellow
    • Set playerColor[5] = orange
    • Set playerColor[6] = green
    • Set playerColor[7] = pink
    • Set playerColor[8] = gray
    • Set playerColor[9] = lightblue
    • Set playerColor[10] = darkgreen
    • Set playerColor[11] = brown
    • For each (Integer B) from 1 to skipCount, do (Actions)
      • Loop - Actions
        • Player Group - Add (Player(skipPlayer[((Integer B) - 1)])) to skip_players
    • For each (Integer A) from 1 to playerCount, do (Actions)
      • Loop - Actions
        • Trigger - Add to Ally Copy <gen> the event (Player - (Player((Integer A))) types a chat message containing -ally as A substring)
        • Trigger - Add to Ally Copy <gen> the event (Player - (Player((Integer A))) types a chat message containing -unally as A substring)

• Ally Copy
  • Events
  • Conditions
    • Or - Any (Conditions) are true
      • Conditions
        • (Substring((Entered chat string), 1, 8)) Equal to -unally
        • (Substring((Entered chat string), 1, 6)) Equal to -ally
  • Actions
    • For each (Integer A) from 1 to playerCount, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Player((Integer A))) Not equal to (Triggering player)
            • ((Player((Integer A))) is in skip_players) Equal to False
            • skip Equal to True
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • Or - Any (Conditions) are true
                  • Conditions
                    • (Entered chat string) Equal to (-ally + (String((Integer A))))
                    • (Entered chat string) Equal to (-ally + playerColor[((Integer A) - 1)])
              • Then - Actions
                • Player - Make (Triggering player) treat (Player((Integer A))) as an Ally with shared vision
                • Game - Display to (All players) for 8.00 seconds the text: ((Name of (Triggering player)) + ( has allied themself with + ((Name of (Player((Integer A)))) + .)))
                • Skip remaining actions
              • Else - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • Or - Any (Conditions) are true
                      • Conditions
                        • (Entered chat string) Equal to (-unally + (String((Integer A))))
                        • (Entered chat string) Equal to (-unally + playerColor[((Integer A) - 1)])
                  • Then - Actions
                    • Player - Make (Triggering player) treat (Player((Integer A))) as an Enemy
                    • Game - Display to (All players) for 8.00 seconds the text: ((Name of (Triggering player)) + ( is now an enemy to + ((Name of (Player((Integer A)))) + .)))
                    • Skip remaining actions
                  • Else - Actions
          • Else - Actions

Doesn't GUI create a whole different function for if/then/else comparisons? No use in putting Skip remaining actions if I am correct :3

 

[Faith]

Doesn't GUI create a whole different function for if/then/else comparisons? No use in putting Skip remaining actions if I am correct :3

Yep.

 

[Chaosy]

Not really.

function Trig_Melee_Initialization_Func001C takes nothing returns boolean
    return true
endfunction

function Trig_Melee_Initialization_Actions takes nothing returns nothing
    if ( Trig_Melee_Initialization_Func001C() ) then
    else
    endif
endfunction

//===========================================================================
function InitTrig_Melee_Initialization takes nothing returns nothing
    set gg_trg_Melee_Initialization = CreateTrigger(  )
    call TriggerAddAction( gg_trg_Melee_Initialization, function Trig_Melee_Initialization_Actions )
endfunction

• Melee Initialization
  • Events
    • Map initialization
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
      • Then - Actions
      • Else - Actions

 

[Faith]

I don't know about JASS but I was referring to the skip remaining actions.

 

[IcemanBo]

It might sound not important, as your keywords "ally", "unally" and so on were well chosen. But you should make them configurable.

It might happen, that user wants to customize it. In my opinion, for such things it's always: configurable variable > hardcoded constant.

In your vision/unvision trigger you leak a force (player group) when you display a message. That must be fixed.

In init, do explain what a "skipped player" is. Explain that you can't use alliance commands with players in skip group.

 

[Faith]

Updated again. I've added everything you suggested.

 

[IcemanBo]

Changes were made. I like that you user can choose between colour, name or player number. Approved.

Site notes:

• Length of key words could be stored into varibales, as well the "keyword" + " " to reduce same function calls over and over again.
• It probably could be a small improvement if you allowed visison modification also for unallied players. It's a small restriction in my eyes, but it's acceptable.

 

[Faith]

Thank you for the help!

 

[IcemanBo]

Greetings.

It would be nice if it would be updated to support 24 players.

 

[BladeMaster3 (TVWK)]

Could prove useful. However I have a few concerns if you may call it that.

critism

1. What's the benefit of using this compared to the system you based it on? You might want to list them clearly so people knows without looking at the code.

2. I think you can shorten down the code by half (if not more) unless I am missing something. What are you using all those if conditions for? basically you only need to make the player ally a certain player? and then sort out a few buggy exceptions like allying yourself. That shouldn't take more than a few lines.

I might be completely wrong, but if so, please enlighten me x)

suggestions

1. use a variable for triggering player.
2. merge the two triggers into one.
3. Allying with a player should be more complex. For example, if you ally yourself with player Yellow, and player yellow is allied with player Oranage, you should automatically be allied to him as well. Same goes for enemis, if you're allying someone who is enemy with purple, purple should be your enemy too.

nha not a good idea for rp stuff

 

[BladeMaster3 (TVWK)]

yeah like iceman said it will be really usefull to make this up to 24 players