4
\$\begingroup\$

I cannot understand how I can make two classes work together by passing variables between them.

During the last 5 days, I learned the basics of inheritance, singleton methods and eval methods. I even read about template methods, but since I have no programming background, I'm in no position to learn from other languages.

The only aim here is to learn how to make use of classes. The whole game is an exercise.

The game has 5 classes: Game, Creature, Weapon, Armor, Scene.

Creature class is used to produce the hero and several monsters (currently only a dragon is created).

Weapon and Armor classes (with only name and power attributes) are there to produce equipment and while they may be joined as a single Equipment class with name, defense and attack attributes, currently this is not my concern.

Scene class is there to produce several places/rooms, which is the main point of the exercise. The exercise clearly states "Use one class per room and give the classes names that fit their purpose." Therefore I made a class containing name, history and armor/weapon and monster attributes. The reason for name and history are clearly to be able to give an introduction when the player enters the scene.

The armor/weapon and monster attributes are required to make the player to encounter different armors, weapons to equip and different monsters to fight in different scenes. I tried to pass them as variables (arrays) to the scene class during initialize but couldn't succeed. The roots of this failure probably is the reason why I can't make the two classes work together.

I have to admit that I had a hard time to understand how to pass variables to/between methods in ruby, and this curse is following me in the classes, too.

So, as a last thing, you can see the horrible code in the choices method in Scene class. I cannot exactly explain what I was thinking while coding this. But the only good thing I have done is adding self to the @city = Scene.new step of the Game.initialize(), so that I can pass the game object to the city object. After that I got lost and need your help.

#game.rb
class Game
  attr_accessor :armor_list, :weapon_list, :equipment
  def initialize()
    #Game creates the city and mountain as two separate scenes
    @city = Scene.new("City", "The city.", @cm, @ls, "", self)
    @mountain = Scene.new("Mountain", "The mountain.", @em, @wh, @d, self)
    #Game creates armor and weapon array-name-holders, I couldn't find another way to make the equipment-selection work
    @a = Armor.new("armor_list", 0)
    @w = Weapon.new("weapon_list", 0)
    #Game creates weapons
    @ss = Weapon.new("Short sword", 5)
    @ls = Weapon.new("Long sword", 8)
    @ts = Weapon.new("Two-Handed sword", 12)
    @wh = Weapon.new("Warhammer", 10)
    #Game creates armors
    @la = Armor.new("Leather Armor", 5)
    @cm = Armor.new("Chain Mail", 10)
    @pm = Armor.new("Plate Mail", 15)
    @em = Armor.new("Elven Chain Mail", 50)
    #Game creates a hero and a dragon
    @h = Creature.new(name = "You", level = 5)
    @d = Creature.new(name = "Dragon", level = 12)
    #The default equipment list to provide an arsenal. I wish to separate them and put 1 or 2 into each scene
    @armor_list = [@a, @la, @cm, @pm, @em]
    @weapon_list = [@w, @ss, @ls, @ts, @wh]
    @equipment = []
  end

  def play()
    intro()
  end
  # If I can find a way to pass arguments between scenes and game, I'm going to put a variable to change the starting point. Otherwise I know that both intro() and play() is useless.
  def intro()
    @city.intro()
  end
end

class Creature
  attr_accessor :name, :life, :armor, :weapon, :regen
  def initialize(name, level)
    #Attributes accoridng to level and dice
    @name = name
    @level = level
    @equipment = {:armor => nil, :weapon => nil}
    @armor = 0
    @weapon = 0
    #3.times rand(7) or 3*rand(7) doesn't create the effect, I tried rand(16)+3 but didn't like it.
    @strength = rand(7) + rand(7) + rand(7)
    @condition = rand(7) + rand(7) + rand(7)
    @life = @level * (rand(8) + 1)
    @power = @strength * (rand(4) + 1)
    @regen = @condition
  end

  def select_equipment(equipment)
    introduce_self()
    selection(equipment)
    choice = gets.chomp.to_i
    #@a and @w help to get the name of the array, armor or weapon. 
    if equipment[0].name == "armor_list"
      wear_armor(equipment[choice])
    elsif equipment[0].name == "weapon_list"
      wear_weapon(equipment[choice])
    else
      raise ArgumentError, "Should be armor_list or weapon_list"
    end
    equipment.delete_at(choice)
  end

  def introduce_self()
    if @equipment[:armor] == nil
      puts "You wear no armor!"
    else
      puts "You wear #{@equipment[:armor]}."
    end
    if @equipment[:weapon] == nil
      puts "You carry no weapon!"
    else
      puts "You carry #{@equipment[:weapon]}."
    end 
  end      

  def selection(equipment)
    puts "You wanna some equipment?"
    for i in (1..equipment.length-1) do
      puts "#{i}. #{equipment[i].name}"
      i += 1
    end
  end      

  def wear_armor(armor)   
    @armor = armor.power
    @equipment[:armor] = armor.name
  end

  def wear_weapon(weapon)
    @weapon = weapon.power
    @equipment[:weapon] = weapon.name
  end

  def battle(opp1, opp2)
    #a basic round system depending on even and uneven numbers
    i = 1
    while opp1.life > 0 && opp2.life > 0
      if i % 2 == 0
        attack(opp1, opp2)
      elsif i % 2 == 1
        attack(opp2, opp1)
      else
        #just learning to raise errors, not into rescue, yet
        raise ArgumentError, "The battle is over!"
      end
      i += 1
      round_result(opp1, opp2)
    end
  end

  def round_result(opp1, opp2)
    puts "Hit points:"
    puts "#{opp1.name}: #{opp1.life}"
    puts "#{opp2.name}: #{opp2.life}"
  end



  def attack(attacker, defender)
    #this code below is just to prevent stuff like "hit with -27 points of damage"
    possible_attack = @power + @weapon - defender.armor
    if possible_attack > 0
      attack = possible_attack
    else
      attack = 0
    end

    defender.life -= attack
    puts "#{attacker.name} hit #{defender.name} with #{attack} points of damage!"
    if defender.life <= 0
      puts "...and killed!"
      defender.life = "Dead as cold stone!"
      round_result(attacker, defender)
      #game exits if one of the creatures die
      Process.exit(0)
    else
      defender.life += defender.regen
      puts "#{defender.name} regenerates #{defender.regen} points of life!"
    end
  end
end

#separate classes for weapons and armors, probably unnecessary but still learning

class Weapon
  attr_reader :name, :power
  def initialize(name, power)
    @name = name
    @power = power
  end
end

class Armor
  attr_reader :name, :power
  def initialize(name, power)
    @name = name
    @power = power
  end
end

# I want each scene have its own weapon or armor (scattered on the ground, maybe, according to the story..) but cannot achieve that with armors, weapon variables. The same thing applies to monsters. I would like to have for example rats and thieves in the city, but bats and a dragon in the mountain. However couldn't achieve this exactly. So far I'm only successful in passing the game object to the scene object as the last variable and I think that's a good start.

class Scene
  attr_reader :name, :history, :armors, :weapons, :monsters

  def initialize(name, history, armor_list, weapon_list, monsters, game)
    @name = name
    @history = history
    @armor_list ||= []
    @weapon_list ||= []
    @monsters ||= []
    @game = game
  end

  def intro()
    puts "You are in the " + @name + "."
    puts @history
    choices()
  end

  def choices()
    puts <<-CHOICES
    What would you like to do here?
    1. Look for armor
    2. Look for weapons
    3. Look for monsters to fight
    4. Go to another place!
    CHOICES
    choice = gets.chomp
    #this is where things go really bad! instance_variable_get saves the battle but as for the equipment selection,
    # as soon as I make a choice, it throws this error:
    # "game.rb:193:in 'choices': #<Armor:0x429720 @name="....> is not a symbol (TypeError)" 
    # and I don't think that further addition of : or @ is needed here.
    #The solution should be much simpler but couldn't find it on the web.
    if choice == "1"         @game.send(@game.instance_variable_get(:@h).select_equipment(@game.instance_variable_get(:@armor_list)))
    elsif choice == "2"         @game.send(@game.instance_variable_get(:@h).select_equipment(@game.instance_variable_get(:@weapon_list)))
    elsif choice == "3"         @game.send(@game.instance_variable_get(:@h).battle(@game.instance_variable_get(:@h), @game.instance_variable_get(:@d)))
    elsif choice == "4"
      puts "bad choice, since I am not ready yet!"
    else
      puts "Can't you read?"
    end
  end

  #this is just to show the player a list of equipment found in the scene.
  def equipment_list()
    @armor_list[1..@armor_list.length-1].each {|a| @equipment.push(a.name) }
    @weapon_list[1..@weapon_list.length-1].each {|w| @equipment.push(w.name) }
    puts "You see some #{@room_equipment.join(", ")} lying on the ground."
  end
end

game = Game.new()
game.play()
\$\endgroup\$
3
\$\begingroup\$

The problem is more about the organization of the game. Normally you would not want to have two classes tightly coupled to each other.

Also location of some methods are not correct. For example battle method in Creature should not be there. Or you should not need two parameters for that method.

You are suffering from the responsibilities of objects at the moment. I would suggest to rethink this.

For example you can decide that "Battle" is not something creature is responsible but game.

I came up with this code which also requires much refactoring. I tried not to change your code too much.

#game.rb
class World
  attr_accessor :armor_list, :weapon_list, :monsters, :scenes
  def initialize()
    @weapon_list = {
            short:      {name: "Short sword", power: 5 },
        long:       {name: "Long sword", power: 8 },
        two_handed: {name: "Two-Handed sword", power: 12 },
        warhammer:  {name: "Warhammer sword", power: 10 }       
          }

    @armor_list = {
            leather:    {name: "Leather Armor", power: 5 },
        chain:      {name: "Chain Mail", power: 10 },
        plate:      {name: "Plate Mail", power: 15 },
        elven:      {name: "Elven Chain Mail", power: 50 }      
      }

    @monsters = { 
        rat:        {name: "Rat", level: 1 },
        thief:      {name: "Thief", level: 50 },
        dragon:     {name: "Dragon", level: 12 }
     }

    @scenes = [
    { name: "City", history: "The city.", weapons: [@weapon_list[:long]], armors: [@armor_list[:chain]], monsters: [@monsters[:thief], @monsters[:rat]] },
    { name: "Mountain", history:  "The mountain.", weapons: [@weapon_list[:warhammer]], armors: [@armor_list[:chain]], monsters: [@monsters[:dragon]] }
      ]
  end
end 

class Game
  def initialize
    @player = Creature.new name: "You", level: 5
    @world = World.new
  end 

  def play
    @current_scene = Scene.new(@world.scenes.first)
    @current_scene.intro

    while @player.is_alive?
        self.show_choices
    end
  end

  def show_choices
    puts <<-CHOICES
    What would you like to do here?
    1. Look for armor
    2. Look for weapons
    3. Look for monsters to fight
    4. Go to another place!
    CHOICES
    choice = gets.chomp

    case choice
    when "1"
        self.look_for_armor
    when "2"
        self.look_for_weapons
    when "3"
        self.look_for_monsters
    when "4"
          puts "bad choice, since I am not ready yet!"  
    else
          puts "Can't you read?"
          exit
    end
  end

  def look_for_monsters
      monster = @current_scene.monsters.first
      battle [@player, monster]
  end   

  def look_for_armor
      armor = select_equipment @current_scene.armors
      return if armor.nil?
      @player.wear_armor(@current_scene.armor_picked(armor))   
  end 

  def look_for_weapons
      weapon = select_equipment @current_scene.weapons
      return if weapon.nil?
      @player.wear_weapon(@current_scene.weapon_picked(weapon))
  end

  def select_equipment(equipment)
    @player.introduce_self

    puts "You wanna some equipment?"
    equipment.each_with_index do |item, i|
      puts "#{i+1}. #{item.name}"
    end

    gets.chomp.to_i - 1
  end 

  def battle(opponents) 
    attack_turn = 0
    while opponents.all?(&:is_alive?)
    attacker = opponents[attack_turn]
    defender = opponents[(attack_turn + 1) % 2]

    attack = attacker.attack(defender)
    defender.defend(attack)
    puts "#{attacker.name} hit #{defender.name} with #{attack} points of damage!"
    puts "#{defender.name} regenerates #{defender.regen} points of life!" if defender.is_alive?

    puts "Hit points:"
    opponents.each { |o| puts "#{o.name}: #{o.life}" }

    attack_turn = (attack_turn + 1) % 2
    end

    winner = opponents.first(&:is_alive?)
    puts winner.name 
  end
end

class Scene 
  attr_reader :monsters, :armors, :weapons

  def initialize(setup)
    @name = setup[:name]
    @history = setup[:history]
    @armors = setup[:armors].map { |x| Armor.new x }
    @weapons = setup[:weapons].map { |x| Weapon.new x }
    @monsters = setup[:monsters].map { |x| Creature.new x }
  end

  def intro()
    puts "You are in the " + @name + "."
    puts @history
    equipment_list
  end

  def armor_picked(index)
    @armors.delete_at(index)
  end

  def weapon_picked(index)
    @weapons.delete_at(index)
  end

  def equipment_list()
        (@armors + @weapons).each {|a| puts "You see some #{a.name} lying on the ground." }  
  end
end

class Creature
  attr_reader :name, :life, :weapon, :armor
  def initialize(config)
    @name = config[:name]
    @level = config[:level]
    @equipment = {:armor => nil, :weapon => nil}
    @armor = 0
    @weapon = 0
    #3.times rand(7) or 3*rand(7) doesn't create the effect, I tried rand(16)+3 but didn't like it.
    @strength = rand(7) + rand(7) + rand(7)
    @condition = rand(7) + rand(7) + rand(7)
    @life = @level * (rand(8) + 1)
    @power = @strength * (rand(4) + 1)
    @regen = @condition
  end

  def introduce_self()
      puts "You wear "  + (@equipment[:armor]  || "no armor!")
      puts "You carry " + (@equipment[:weapon] || "no no weapon!")
  end 

  def wear_armor(armor)   
    @armor = armor.power
    @equipment[:armor] = armor.name
  end

  def wear_weapon(weapon)
    @weapon = weapon.power
    @equipment[:weapon] = weapon.name
  end

  def attack(defender) 
    [@power + @weapon - defender.armor, 0].max
  end

  def defend(attack)
    @life -= attack
  end

  def regen
    @life += @regen
    @regen
  end

  def is_alive?
    @life > 0
  end 
end

class Weapon
  attr_reader :name, :power
  def initialize(setup)
    @name = setup[:name]
    @power = setup[:power]  end
end

class Armor
  attr_reader :name, :power
  def initialize(setup)
    @name = setup[:name]
    @power = setup[:power]
  end
end

game = Game.new()
game.play()
\$\endgroup\$
  • \$\begingroup\$ Good refactoring! Just one thing, you may want to change some of the methods that take on array to taking two arguments or a * (splat) argument. \$\endgroup\$ – Linuxios Mar 25 '12 at 13:44
  • \$\begingroup\$ I am examining your code with great pleasure. So I have 2 questions: 1) I see 99.9% of code in the classes and just 'game = Game.new().play()' to start the game. It's just like clicking start button on a web page. Is this the proper/preferred way to use classes? I mean, weapons, armors and monsters could be separate hashes outside classes, just before the 'game = ' part. 2) I'm a newbie and working on Zed Shaw's tutorial, exercise 43. Could you mind to tell me if the complexity of this game is appropriate to the level of the exercise? \$\endgroup\$ – barerd Mar 25 '12 at 14:17
  • \$\begingroup\$ @Linux_iOS.rb.cpp.c.lisp.m.sh you are right, habits from Javascript :) \$\endgroup\$ – Can Hanhan Mar 25 '12 at 14:38
  • \$\begingroup\$ @barerd I did not wanted to change your logic but only the "ruby" part. I could do the same without any classes. The usage of classes in my example is not really something you would like to have. The code outside is just "entry-point". \$\endgroup\$ – Can Hanhan Mar 25 '12 at 14:47
  • \$\begingroup\$ @barerd There are many things that you can do. For example instead of using hashes to setup the world; you can create separate classes for each equipment to give them different behaviours. (i.e: you can add a blade that can attack twice or a mice that will lower to regen; or a special armor that will hurt the attacker on random chance) You would put these classes in separate folders (./armors, ./weapons, ./scenes) and require and create in the game (runner class). I guess you are building something bigger than the author asked for. It is always hard to manage the interactions in the games. \$\endgroup\$ – Can Hanhan Mar 25 '12 at 14:51
2
\$\begingroup\$

Don't use instance_variable_get unless you really need it. And here you don't need it.

When you need the content of an attribute outside the class, then define a getter:

class Game
  attr_accessor :armor_list, :weapon_list, :equipment
  #additional getter
  attr_reader :h, :d
  #....continue

and then in approx line 200:

#this is where things go really bad! instance_variable_get saves the battle but as for the equipment selection,
# as soon as I make a choice, it throws this error:
# "game.rb:193:in 'choices': #<Armor:0x429720 @name="....> is not a symbol (TypeError)" 
# and I don't think that further addition of : or @ is needed here.
#The solution should be much simpler but couldn't find it on the web.
if choice == "1"         
  @game.h.select_equipment(@game.armor_list)
elsif choice == "2"         
  @game.h.select_equipment(@game.weapon_list)
elsif choice == "3"         
  @game.h.battle(@game.h, @game.d)
elsif choice == "4"
  puts "bad choice, since I am not ready yet!"
else
  puts "Can't you read?"
end

I wouldn't use h and d. Use hero and dragon (and you have to replace @h and @d in your program).

  #The hero (me)
  attr_reader :hero
  #The dragon
  attr_reader :dragon

If you have a problem, could you post a play sequence?


in approx line 90 you have:

def selection(equipment)
  puts "You wanna some equipment?"
  for i in (1..equipment.length-1) do
    puts "#{i}. #{equipment[i].name}"
    i += 1
  end
end      

Normally you should use:

def selection(equipment)
  puts "You wanna some equipment?"
  equipment[1..-1].each_with_index do |equip, i|
    puts "#{i}. #{equip.name}"
  end
end      

The [1..-1] is needed to skip the first element ("armor_list"). It is not a good idea to mix description and content inside one array. (Especially if your @equipment is a hash) I haven't analyzed your code in depth, but I think you should replace

def select_equipment(equipment)

with

def select_equipment(equipmentname, equipment)
\$\endgroup\$
  • \$\begingroup\$ Thank you for your reply: "When you need the content of an attribute outside the class" I wish I could see this when I read about getter and setter methods before. \$\endgroup\$ – barerd Mar 25 '12 at 20:26
1
\$\begingroup\$

Also, in addition to the other two answers, you should never use instance variables to store static content (such as the stats of a short sword). Not only does it imply that the static data (Weapon.new("Short Sword", 5)) will change between objects (I doubt that the starting stats of a weapon will change between objects). In addition to this, reinitializing the Weapon object is a wast of reassures to do every time.

Instead of instance varibles named after each weapon, here is one soulution:

@@weapon={ss: Weapon.new("Short Sword", 5), ls: Weapon.new("Longsword", 10), etc...}

Also, on lines 7 and 6, you are passing nil for every parameter starting with @. No instance variables have been declared yet, and Ruby gives nil as the value of initialized instance, class, and global variables. You must put that initialization after the instance variables are declared if you want any value other than nil (and if you want nil, just use the litteral).


Also, there is a slight matter of Ruby form here: When a method takes no arguments, omit the parenthesis unless it is an issue of ambiguity between names.


In the selection method, you should put the gets.chomp in the method and return it. A method that prints the prompt but doesn't read the data is not worth having.


In that battle method, don't throw an ArgumentError. The battle ending is no reason to think that the arguments are wrong. Instead, make your own error class:

class BattleOverException < StandardError; end

And throw it:

raise BattleOverException, "The battle is over!!"

Although this works, exceptions should be used for the comunication of an error at the file level (integer passed, expecting float, file not there, wrong number of arguments, etc.), not user problems (entered number expecting string, etc.). If you are using exceptions to exit the method, just use return. If you are trying to terminate the program, use puts for a message and use exit. And if you want to signal some condition to the calling code that isn't an error, take a look at [catch][1] and [throw][2].


round_result should not exist. There is no reason to factor out some simple printing to a method called once. The code is not complex enough to be its own method or enough of its own concept.


As for your problem of class communication, you can boil it down to this: Give the Game class the methods that the Scene class must use to interact with it. Continue passing self to the Scene constructor. Also, in the future, you may want to consider one manager class that manages all of the data, or a more decoupled class design. I recommend the second.

\$\endgroup\$
  • \$\begingroup\$ Thank you for these nice points. As I mentioned to @CanHanhan, I am a newbie and a physician. So I'm not going to write professional games. My interest in ruby stems from my efforts to organize patient/research data into mysql db, and rails made it very easy to create/use them. I just want to improve my grasp on the language, so that I can get rid of stealing from other people's code. So is there any document about decoupling classes you can suggest to me, which is simpler than this one? \$\endgroup\$ – barerd Mar 25 '12 at 14:37
  • \$\begingroup\$ I don't have one at my disposal at this moment (I should write one on my blog), but all I mean is that you should make your classes individual units: One class should not be controlling the object that created it, but objects should form a hierarchy. \$\endgroup\$ – Linuxios Mar 25 '12 at 14:45
  • \$\begingroup\$ Okay, I'll be watching your blog. I'm using gedit by the way and after I read your blog about it, I am going to add a ruby console to it. \$\endgroup\$ – barerd Mar 25 '12 at 16:14
  • \$\begingroup\$ I read Programming Ruby. And saw there: "..the only way to change an object's state in Ruby is by calling one of its methods. Control access to the methods and you've controlled access to the object. A good rule of thumb is never to expose methods that could leave an object in an invalid state." This made me think that in order to eliminate coupling, all methods capable to modify an object's attributes, must only be put into that object's class, and not into any other class. Is that what you mean? \$\endgroup\$ – barerd Mar 25 '12 at 20:22
  • \$\begingroup\$ True. But the main thing is two make it so that somewhat unrelated classes (maybe you want to use class Scene in another game) can be moved and used without the other. It is more about making things more general. (Post is almost done). \$\endgroup\$ – Linuxios Mar 26 '12 at 0:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.