# Rock, Paper, Scissors completed game

I have just completed my first Ruby OOP project. Fairly simple but still a bit time consuming as I am still learning the language.

I would like feedback and ways I can improve on this and make the flow of code better or could have done something in a different way to save time or the "right way".

I thank you all in advance!

my code below

class Player
attr_accessor :name

def initialize(name)
@name = name
end

end

class Game

def player_vs_AI?
puts "Hello User..."
puts "Will you be playing against another person or the computer?"
puts "Choose 1 for another person or choose 2 for the computer"

end

def greeting

names = ['Bob', 'Joseph', 'Catherine', 'Paul', 'Tom', 'Baker']
name1 = gets.chomp
@player1 = Player.new(name1)

name2 = gets.chomp
@player2 = Player.new(name2)
name2 = names.sample
@player2 = Player.new(name2)
puts "Player 2 will be named #{@player2.name}"
end
sleep(0.5)
puts "Welcome to Rock-Paper-Scissor"
sleep(0.5)
puts "Welcome #{@player1.name} and #{@player2.name}\n\n"
sleep(0.5)

end

def instruct
puts "The rules of the game are simple\n"
puts "Simply choose Rock(R), Paper(P) or Scissor(S) each turn\n "
puts "Paper beats Rock, Scissor beats paper and Rock beats Scissor\n"
sleep(1.0)
end

def choice
choices = ['r', 'p', 's']

@choice_1=gets.chomp
@choice_1.downcase!

while !(@choice_1 == 'r' || @choice_1 == 'p' || @choice_1 == 's') do
puts "#{@player1.name} That is not a valid choice, please choose from r, p, or s"
@choice_1=gets.chomp
@choice_1.downcase!
end

@choice_2=gets.chomp
@choice_2.downcase!

while !(@choice_2 == 'r' || @choice_2 == 'p' || @choice_2 == 's') do
puts "#{@player2.name} That is not a valid choice, please choose from r, p, or s"
@choice_2=gets.chomp
@choice_2.downcase!
end
@choice_2 = choices.sample
end
end

def logic
puts "Lets see who wins...will it be #{@player1.name} or will it be #{@player2.name}\n"
sleep(2.0)
if @choice_1 == @choice_2
puts "Opps, both players picked the same weapon, It is a TIE!!!"
puts "Lets go back and choose again..."
sleep(3.0)
choice
end
if @choice_1 == 'r' && @choice_2 == 'p'
puts "#{@player2.name} win!!! Paper covers Rock"
elsif @choice_1 == 'p' && @choice_2 == 'r'
puts "#{@player1.name} you Win!!! Paper covers Rock"
elsif @choice_1 == 'p' && @choice_2 == 's'
puts "#{@player2.name} you win!!! Scissor cuts Paper "
elsif @choice_2 == 's' && @choice_1 == 'p'
puts "#{@player1.name} you win!!! Scissor cuts Paper"
elsif @choice_1  == 'r' && @choice_2 == 's'
puts "#{@player1.name} you win!!! Rock beats Scissor"
else @choice_2 == 'r' && @choice_1 == 's'
puts "#{@player2.name} you win!!!! Rock beats Scissor"
end
end

def play_again?
sleep(0.5)
puts "\n"
puts "Would you like to play again?(yes/no)  "
input = gets.chomp
if input == 'yes'
start
elsif input == 'no'
puts "Thank #{@player1.name} and #{@player2.name} for playing!"
end
end

end

def start
game = Game.new
game.player_vs_AI?
game.greeting
game.instruct
game.choice
game.logic
game.play_again?
end
start
$$$$

• You can simplify the task of determining which player, if either, wins a given throw, by creating a hash winners = { r: :s, :p :r, :s :p }. Then if player 1 throws p1 (:r, :p or :s) and player 2 throws p2, player 1 wins if winners[p1] == p2, else player 2 wins if winners[p2] == p1 else it's a tie and they throw again. – Cary Swoveland Feb 27 at 3:49

The biggest thing that will allow the code to clean up a fair bit is if you had made a subclass of Player for AIPlayer:

class AIPlayer < Player
NAMES = ['Bob', 'Joseph', 'Catherine', 'Paul', 'Tom', 'Baker']

def initialize
super NAMES.sample
end
end


that will allow you to add a method get_weapon to Player, which would ask for the user input and override it in AIPlayer to return a random selection. Your choice method then can become cleaner:

def choice
@choice_1 = @player1.get_weapon
@choice_2 = @player2.get_weapon
end


and not care about if it's player vs player or vs ai. Your player_vs_AI? method can then return true or false and not have to assign an instance variable at all:

def player_vs_AI?
puts "Will you be playing against another person or the computer?"
puts "Choose 1 for another person or choose 2 for the computer"

gets.chomp.to_s == '2'
end


and can be then used to clean up like:

def greeting
name1 = gets.chomp
@player1 = Player.new(name1)

if player_vs_AI?
@player2 = AIPlayer.new
puts "Player 2 will be named #{@player2.name}"
else
name2 = gets.chomp
@player2 = Player.new(name2)
end
sleep(0.5)
puts "Welcome to Rock-Paper-Scissor"
sleep(0.5)
puts "Welcome #{@player1} and #{@player2}\n\n"
sleep(0.5)
end


and you no longer need to call player_vs_AI? during start.

Your play_again? method also shouldn't call start itself, instead the start method should check if they want to play again and just keep getting choices and checking the results:

def start
game = Game.new
game.greeting
game.instruct
loop do
game.choice
game.logic
break unless game.play_again?
end
end


having trimmed down the choice method, it could be combined with the logic method and perhaps renamed play (or something more meaningful) and start could be renamed run.

On a last note, I would try to make things consistent for the user. You're asking for input options 3 times, the first it's enter 1 or 2 (player_vs_AI?), the second it's r, p, or s (choice) and the last time in play_again? they have to enter yes or no. Only in choice is there input validation (what happens in play_again? if I try to enter y instead of yes?). This could be more consistent (always allow the full word to be entered, or just the first letter, for instance).

• awesome! thank you for the feedback and great suggestions that i will make in my code. Also, do you think there would have been a better way to do my logic method instead of all the if-else loops – nimpa Jan 2 at 14:11

I am not a professional game developer, just trying to help you base on my experience with Ruby so far. Glad to see your willingness of coding correctly from the beginning.

Some points to improve:

1.Move the bot names and game mode options to CONSTANTS

2.Prompting for user's input is duplicated many times, you should make a function that take prompting lines and accepted values as arguments then return only when user input is in accepted values.

3.Beside name, a Player can have weapon as its instance variable also.

4.About the logic method, I found a way to improve it, please consider. If we put all the options to an array by a specific order that the option at higher index will win ([r, p, s] or [p, s, r] or [s, r, p]) then the logic can patternized. The only exception is when the higher index is the last element but the smaller index is the first element, we can solve the exception with a simple if

5.You should also learn how to write test spec and apply it here especially for logic method

6.Last but not least, your indentation :)

class Player
attr_accessor :name, :weapon

def initialize(name)
@name = name
end
end

class Game
BOT_NAMES = ['Bob', 'Joseph', 'Catherine', 'Paul', 'Tom', 'Baker']
WEAPONS   = {'r' => 'Rock', 'p' => 'Paper', 's' => 'Sissors'}

def prompt_for_user_input(accepted_inputs=[], prompt_lines=[], is_case_sensitive=true)
loop do
prompt_lines.each { |line| puts line }
user_input = gets.chomp
return user_input.to_s if accepted_inputs.empty?
is_valid_input = is_case_sensitive ? accepted_inputs.include?(user_input) : accepted_inputs.any?{ |s| s.casecmp(user_input) == 0 }
return user_input.to_s if is_valid_input
end
end

def mode_selection
prompt_lines = [
"Hello User...",
"Will you be playing against another person or the computer?",
"Choose 1 for another person or choose 2 for the computer"
]

@mode = prompt_for_user_input(['1', '2'], prompt_lines)
end

def player_creation
@player2 = if (@mode == '1')
else
bot_name = BOT_NAMES.sample
puts "Player 2 will be named #{bot_name}"
Player.new(bot_name)
end

sleep(0.5)
puts "Welcome #{@player1.name} and #{@player2.name} to Rock-Paper-Scissor"
end

def instruct
puts "The rules of the game are simple\n"
puts "Simply choose Rock(R), Paper(P) or Scissor(S) each turn\n "
puts "Paper beats Rock, Scissor beats Paper and Rock beats Scissor\n"
sleep(1.0)
end

def weapon_selection
@player1.weapon = prompt_for_user_input(WEAPONS.keys, ["#{@player1.name}, please chooose your choice of weapon: r(Rock), p(Paper), s(Sissor)"], false)
@player2.weapon = if (@mode == '1')
prompt_for_user_input(WEAPONS.keys, ["#{@player2.name}, please chooose your choice of weapon: r(Rock), p(Paper), s(Sissor)"], false)
else
bot_weapon = WEAPONS.keys.sample
puts "#{@player2.name} chooses #{WEAPONS[bot_weapon]}"
bot_weapon
end

puts "Let see who wins... will it be #{@player1.name} or will it be #{@player2.name}"
sleep(2.0)
decider
end

def decider
if @player1.weapon == @player2.weapon
puts "Opps, both players picked the same weapon, It is a TIE!!!"
puts "Lets go back and choose again..."
sleep(3.0)
weapon_selection
end

weapon_keys = WEAPONS.keys
weapon_1_index = weapon_keys.find_index(@player1.weapon)
weapon_2_index = weapon_keys.find_index(@player2.weapon)
if weapon_1_index > weapon_2_index
if (weapon_1_index == weapon_keys.length - 1) && (weapon_2_index == 0)
@winner, @loser = @player2, @player1
else
@winner, @loser = @player1, @player2
end
else
if (weapon_2_index == weapon_keys.length - 1) && (weapon_1_index == 0)
@winner, @loser = @player1, @player2
else
@winner, @loser = @player2, @player1
end
end

end

def result
puts "#{@winner.name} you win !!! #{WEAPONS[@winner.weapon]} beats #{WEAPONS[@loser.weapon]}"
end

def again?
prompt_for_user_input(['y', 'n'], prompt_lines=["Would you like to play again?(y/n)"]) == 'y'
end

def play
loop do
mode_selection
player_creation
instruct
weapon_selection
decider
result
break unless again?
end
puts "Thank #{@player1.name} and #{@player2.name} for playing!"
end
end

g = Game.new
g.play

• Awesome! All the suggestions were great and made my code a lot better. I do have a question for the logic method that you suggested. When you wrote weapon_keys.length - 1 is that because we are comparing it to the index of the array which starts at 0 so to make it equal we have to do .length-1? – nimpa Jan 2 at 17:51
• No, the weapon_keys.length - 1 is to check if the weapon is the last element of the array. Last index of the last element is array length - 1, a common trick when working with array – Thang Jan 3 at 4:00
• Update: All the Game methods except for play` should be private – Thang Jan 3 at 7:31