Feedback How to shorten series of multiple inputs

(Newbie here) Im making a two-player game centered around rock paper scissors. essentially, I need the players to have a state of either rock, paper, scissors, or null until they press a different key. Right now I have 16 lines of code that contain specific key inputs and enum cases for each state. (Also If you know of a way to shorten this huge switch statement for the rock-paper-scissors logic it would be a big help) thank you in advance!

Code (CSharp):

1.  
2. using UnityEngine;
3.  
4. public class Inputs : MonoBehaviour
5. {
6.     //Im importing the "RPSID" enum from RPS_Brain
7.     //thes Ids are made into variables known as Player1 and Player2
8.     //'HideInInspector' is making it so that the Player Ids are not presented in the inspector
9.     [HideInInspector] public RPSID Player1 = RPSID.Null;
10.     [HideInInspector] public RPSID Player2 = RPSID.Null;
11.     public PlayerInputs P1;
12.     public PlayerInputs P2;
13.     public float matchwin;
14.  
15.     //This is the control center, where key binds change the Players 1&2 variables to the different RSID cases
16.     public void Update()
17.     {
18.         if (Input.GetKeyDown(P1.Null))
19.         {
20.             Player1 = RPSID.Null;
21.         }
22.         if (Input.GetKeyDown(P1.Paper))
23.         {
24.             Player1 = RPSID.Paper;
25.         }
26.         if (Input.GetKeyDown(P1.Scissors))
27.         {
28.             Player1 = RPSID.Scissors;
29.         }
30.         if (Input.GetKeyDown(P1.Rock))
31.         {
32.             Player1 = RPSID.Rock;
33.         }
34.         if (Input.GetKeyDown(P2.Null))
35.         {
36.             Player2 = RPSID.Null;
37.         }
38.         if (Input.GetKeyDown(P2.Paper))
39.         {
40.             Player2 = RPSID.Paper;
41.         }
42.         if (Input.GetKeyDown(P2.Scissors))
43.         {
44.             Player2 = RPSID.Scissors;
45.         }
46.         if (Input.GetKeyDown(P2.Rock))
47.         {
48.             Player2 = RPSID.Rock;
49.         }
50.         //This switch casees every interation between player 1 and 2
51.         /*The switch first defines every case of action player1 can perform within the RPSID enum.
52.         Then in each player1 case, it has a another switch for player2 and its cases within the RPSID,
53.         The cases then state whos winnig through the variable 'boutwin'.
54.         */
55.         switch (Player1)
56.         {
57.             case RPSID.Paper:
58.             {
59.                 switch (Player2)
60.                 {
61.                     case RPSID.Paper:
62.                     {
63.                         matchwin = 0;
64.                         break;
65.                     }
66.                     case RPSID.Scissors:
67.                     {
68.                         matchwin = 2;
69.                         break;
70.                     }
71.                     case RPSID.Rock:
72.                     {
73.                         matchwin = 1;
74.                         break;
75.                     }
76.                 }
77.                        
78.                 break;
79.             }
80.             case RPSID.Scissors:
81.             {
82.                 switch (Player2)
83.                 {
84.                     case RPSID.Paper:
85.                     {
86.                         matchwin = 1;
87.                         break;
88.                     }
89.                     case RPSID.Scissors:
90.                     {
91.                         matchwin = 0;
92.                         break;
93.                     }
94.                     case RPSID.Rock:
95.                     {
96.                         matchwin = 2;
97.                         break;
98.                     }
99.                 }
100.  
101.                 break;
102.             }
103.             case RPSID.Rock:
104.             {
105.                 switch (Player2)
106.                 {
107.                     case RPSID.Paper:
108.                     {
109.                         matchwin = 2;
110.                         break;
111.                     }
112.                     case RPSID.Scissors:
113.                     {
114.                         matchwin = 1;
115.                         break;
116.                     }
117.                     case RPSID.Rock:
118.                     {
119.                         matchwin = 0;
120.                         Debug.Log(matchwin);
121.                         break;
122.                     }
123.                 }
124.  
125.                 break;
126.             }
127.         }
128.     }
129. }
130. [SerializeField]
131. public enum RPSID : short { Null = 0, Scissors = 1, Paper = 2, Rock = 3 };
132.  
133. [System.Serializable]
134. //These Keycodes act as
135. public class PlayerInputs
136. {
137.     public KeyCode Null;
138.     public KeyCode Paper;
139.     public KeyCode Scissors;
140.     public KeyCode Rock;
141.  
142.     public PlayerInputs(KeyCode N, KeyCode S, KeyCode P, KeyCode R)
143.     {
144.         Null = N;
145.         Paper = P;
146.         Scissors = S;
147.         Rock = R;
148.     }
149. }
150.  

 

Since it's basically just an input being directly mapped to an output you can use an array to represent them. Input keycodes have also been moved into a dictionary for same reason.

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class Inputs : MonoBehaviour
6. {
7.     [HideInInspector] public RPSID Player1 = RPSID.Null;
8.     [HideInInspector] public RPSID Player2 = RPSID.Null;
9.  
10.     public PlayerInputs P1;
11.     public PlayerInputs P2;
12.  
13.     public float matchwin;
14.  
15.     private int[,] matchResults = new int[,] {
16.         // N, R, P, S
17.         {  0, 0, 0, 0 }, // N
18.         {  0, 0, 2, 1 }, // R
19.         {  0, 1, 0, 2 }, // P
20.         {  0, 2, 1, 0 }  // S
21.     };
22.  
23.     public void Update()
24.     {
25.         foreach (var pair in P1.InputToState)
26.             if (Input.GetKeyDown(pair.Key))
27.                 Player1 = pair.Value;
28.  
29.         foreach (var pair in P2.InputToState)
30.             if (Input.GetKeyDown(pair.Key))
31.                 Player2 = pair.Value;
32.     }
33.  
34.     matchwin = matchResults[(int)Player1, (int)Player2];
35. }
36.  
37. public enum RPSID : short { Null = 0, Scissors = 1, Paper = 2, Rock = 3 };
38.  
39. [Serializable]
40. public class PlayerInputs
41. {
42.     public Dictionary<KeyCode, RPSID> InputToState = new Dictionary<KeyCode, RPSID>();
43.  
44.     public PlayerInputs(KeyCode N, KeyCode S, KeyCode P, KeyCode R)
45.     {
46.         InputToState[N] = RPSID.Null;
47.         InputToState[R] = RPSID.Rock;
48.         InputToState[P] = RPSID.Paper;
49.         InputToState[S] = RPSID.Scissors;
50.     }
51. }

 

The only thing that needs manual service is the key logic table, the cells denote the winner (or a draw)
(Rows are player 1, columns are player 2)

Code (csharp):

1.     R   P   S
2. R   D   2   1
3. P   1   D   2
4. S   2   1   D

To code this logic you start with a simple convention, R = 0, P = 1, S = 2.
When it comes to outcomes, P1 = 0, P2 = 1, and Draw = -1.

We can rewrite the table with this in mind

Code (csharp):

1.     0   1   2
2. 0  -1   1   0
3. 1   0  -1   1
4. 2   1   0  -1

Then we can build the following function

Code (csharp):

1. int resolveRPSMatch(int v1, int v2) {
2.   // we do argument validation
3.   if(v1 < 0 || v1 > 2 || v2 < 0 || v2 > 2) throw new ArgumentException();
4.  
5.   // then we immediately recognize the draw state
6.   if(v1 == v2) return -1;
7.  
8.   // next we do the table logic row by row, which makes it more readable
9.   return v1 switch {
10.     0 => v2 == 1? 1 : 0, // P2 wins if against the rock (0) he shows the paper (1), but loses otherwise
11.     1 => v2 == 2? 1 : 0, // P2 wins if against the paper (1) he shows the scissors (2), but loses otherwise
12.     _ => v2 == 0? 1 : 0  // P2 wins if against the scissors (2) he shows the rock (0), but loses otherwise
13.   };
14. }

Now we have the heart of it. Next you can introduce an enum for your coding convenience.

Code (csharp):

1. public enum RPSLabel {
2.   Rock = 0,
3.   Paper,
4.   Scissors
5. }
6.  
7. public enum MatchOutcome {
8.   Draw = -1,
9.   Player1,
10.   Player2
11. }

Now you can promote the function to appear more code-friendly

Code (csharp):

1. MatchOutcome resolveRPSMatch(RPSLabel player1, RPSLabel player2) {
2.   // we don't need argument validation any more
3.   var v1 = (int)player1;
4.   var v2 = (int)player2;
5.   var outcome = -1; // expects a draw until proven otherwise
6.  
7.   if(v1 != v2) {
8.     outcome = v1 switch {
9.       0 => v2 == 1? 1 : 0,
10.       1 => v2 == 2? 1 : 0,
11.       _ => v2 == 0? 1 : 0
12.     };
13.   }
14.  
15.   return (MatchOutcome)outcome;
16. }

All that remains is to parse your input appropriately and pass the enumerated selections through this function.

Edit: Ryiah's answer above is probably more holistic in every regard. Mine is only about getting rid of the annoying if..else structure.

Edit2: I forgot a semi-colon after switch expressions.

 

Since you do rock-paper-scissors you probably should watch this part of Ryan Hipples ScriptableObject talk. Because his example is "rock paper scissors (dynamite)" ^^. So the logic becomes data driven and not code driven

 

@Bunny83 thanks for the reminder, it's a particularly good time for me to revisit that talk because it's been a while.

Oh and the example you mentioned is at 45:32 (not 21:35 you linked). Here's the correct timestamp.

 

My bad. Youtube hadn't adjusted the timestamp when you click on a link in a comment ^^. I've fixed my link as well. Though if you have the time, as you said, watching the whole thing (again) doesn't hurt

 

Hi, Thank you so much! I used this in my game and it makes the most sense. Its working really well. TY