-
-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a hand game command to the chatbox #580
base: develop
Are you sure you want to change the base?
Conversation
Add a hand game on command to the chatbox, Players must type /handrps to randomly choose between rock, paper or scissors.
string[] handgame = { "Rock", "Paper", "Scissors" }; | ||
Random rand = new Random(); | ||
int index = rand.Next(0, handgame.Length); | ||
string reuslts = " selected " + handgame[index] + "!"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reuslts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't prepare the string for display when handling the internal logic. Make HandleHandRPSResult
accepts the index of R/P/S (not a hardcoded English string)
@@ -521,7 +522,14 @@ private void RollDiceCommand(string dieType) | |||
|
|||
BroadcastDiceRoll(dieSides, results); | |||
} | |||
|
|||
private void HandRPSCommand(string ent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should choose a parameter name that is easy to understand. ent
confuses people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ent is a string that represents pressing Enter similar to a blank input
@@ -581,7 +590,11 @@ protected void HandleDiceRollResult(string senderName, string result) | |||
|
|||
PrintDiceRollResult(senderName, dieSides, results); | |||
} | |||
|
|||
protected void HandleHandRPSResult(string senderName, string result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imagine someone using a different language
|
||
protected void PrintHandRPSResult(string senderName, string result) | ||
{ | ||
AddNotice(String.Format("{0} {1}".L10N("Client:Main:PrintHandRPSResult"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would not correctly localize the string since the parameters of string.Format
are English strings
|
||
channel.SendCTCPMessage($"{Hand_RPS_MESSAGE} {results}", QueuedMessageType.CHAT_MESSAGE, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imagine someone using a different language
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also you shouldn't broadcast the display string. Broadcast only necessary raw parameters
Add a hand game on command to the chatbox, Players must type /handrps to randomly choose between rock, paper or scissors.