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()
3 Answers 3
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()
-
\$\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– Linuxios2012年03月25日 13:44:16 +00:00Commented Mar 25, 2012 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– barerd2012年03月25日 14:17:25 +00:00Commented Mar 25, 2012 at 14:17
-
\$\begingroup\$ @Linux_iOS.rb.cpp.c.lisp.m.sh you are right, habits from Javascript :) \$\endgroup\$Can Hanhan– Can Hanhan2012年03月25日 14:38:59 +00:00Commented Mar 25, 2012 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– Can Hanhan2012年03月25日 14:47:17 +00:00Commented Mar 25, 2012 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– Can Hanhan2012年03月25日 14:51:29 +00:00Commented Mar 25, 2012 at 14:51
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)
-
\$\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– barerd2012年03月25日 20:26:57 +00:00Commented Mar 25, 2012 at 20:26
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.
-
\$\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– barerd2012年03月25日 14:37:08 +00:00Commented Mar 25, 2012 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– Linuxios2012年03月25日 14:45:45 +00:00Commented Mar 25, 2012 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– barerd2012年03月25日 16:14:14 +00:00Commented Mar 25, 2012 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– barerd2012年03月25日 20:22:03 +00:00Commented Mar 25, 2012 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– Linuxios2012年03月26日 00:27:21 +00:00Commented Mar 26, 2012 at 0:27
Explore related questions
See similar questions with these tags.