3
\$\begingroup\$

This is my RubyWarrior solution. I think RubyWarrior is well known to Ruby coders. I am starting to learn Ruby and I would appreciate some help from you to improve the organization or the mistakes that I could have. I don't need another example as the Internet is full of them, but what I need is some advice that I can use as a beginner.

class Player
  attr_accessor :health, :direction, :n, :view, :warrior
  def initialize
    @health, @direction, @warrior  = health , direction , warrior
    @direction  = [:forward , :backward]
    @health, @view = [] , [] 
  end

  def play_turn(warrior)
    @health << warrior.health
    @view = warrior.look
    @backview = warrior.look :backward
    senses = warrior.feel @direction[@n]
      if ( senses.wall? && next_empty?(warrior) ) || senses.wall? 
        warrior.pivot!      
      elsif @backview[1].to_s.include?("Captive")
        warrior.walk! :backward        
      elsif @backview[0].to_s.include?("Captive")
        warrior.rescue! :backward      
      elsif next_empty?(warrior) && senses.stairs? || safe_step?
        warrior.walk!
        puts "Hero :D Finally I made it!"
      elsif senses.captive?
        warrior.rescue! 
        puts "Hero ;)   feel free to face your destiny"
      elsif seek_and_shoot?
        warrior.shoot!
        puts "Hero >< BAAANNG!! Die already ranged scumbag"
      elsif warrior.health < 15 && safe?(warrior)
        warrior.rest!
        puts "Hero :(   I need to heal my wounds"
      elsif next_empty?(warrior)
        warrior.walk! 
        puts "Hero :|   go go go!"
      else
        warrior.attack!
        puts "Hero :x   I will kill you evil #{warrior.feel.to_s.upcase}!"
      end

    life_bar_on_screen
    puts @view
    puts @backview
  end 

  def loosing_life? warrior
    @health[-1] < @health[-2] ? true : false
  end

  def next_empty? warrior
    warrior.feel.empty?
  end

  def safe? warrior
    next_empty?(warrior) && !loosing_life?(warrior)
  end

  def life_bar_on_screen 
    puts "_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ LIFE #{@health.last}"
    (1..@health.last).each { |x| print "▒ "} 
    puts ""
  end

  def seek_and_shoot?
    foe = [ "Wizard" , "Archer", "Sludge"] 
    o, oo, ooo = @view[0].to_s , @view[1].to_s , @view[2].to_s
    case 
      when o == "nothing" && foe.include?(oo) then true 
      #when o == "nothing" && ranged_foe.include?(oo) then true 
      #when o == "nothing" && oo == friend then false
      when o == "nothing" && oo == "nothing" && foe.include?(ooo) then true
      else false
    end
  end

  def safe_step?
    friend = "Captive"
    o, oo, ooo = @view[0].to_s , @view[1].to_s , @view[2].to_s
    case 
      when o == "nothing" && oo == friend then true 
      when o == "nothing" && oo == "nothing" && ooo == friend then true
      else false
    end
  end

end

Shell output

CONGRATULATIONS! You have climbed to the top of the tower and rescued the fair maiden Ruby.
Level Score: 74
Time Bonus: 6
Clear Bonus: 16
Level Grade: A
Total Score: 503 + 96 = 599
Your average grade for this tower is: A

  Level 1: S
  Level 2: A
  Level 3: S
  Level 4: A
  Level 5: A
  Level 6: A
  Level 7: A
  Level 8: S
  Level 9: A
\$\endgroup\$
  • \$\begingroup\$ Congratz for your implementation and welcome to Ruby! The first thing that I looked into were those ifs inside play_turn(warrior) method. It would be more readable if you extract all those ifs to a Strategy or Chain of Responsibility class whom receives warrior as parameter. \$\endgroup\$ – hlegius Nov 17 '14 at 16:35
2
\$\begingroup\$
  def loosing_life? warrior
    @health[-1] < @health[-2] ? true : false
  end

  def next_empty? warrior
    warrior.feel.empty?
  end

  def safe? warrior
    next_empty?(warrior) && !loosing_life?(warrior)
  end

Why I see no reason not to just make these part of the warrior class. It would make calling these much cleaner as you're never passing anything but a warrior into them.

  def life_bar_on_screen 
    puts "_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ LIFE #{@health.last}"
    (1..@health.last).each { |x| print "▒ "} 
    puts ""
  end

The life bar is a really cool idea. I like that a lot.

  elsif @backview[1].to_s.include?("Captive")
    warrior.walk! :backward        
  elsif @backview[0].to_s.include?("Captive")

Anytime the same string literal shows up more than once, I recommend using a constant instead. It will protect you from spelling errors that can cause runtime problems. Apply the same advice to your seek_and_shoot? method.

  def seek_and_shoot?
    foe = [ "Wizard" , "Archer", "Sludge"] 
    o, oo, ooo = @view[0].to_s , @view[1].to_s , @view[2].to_s
    case 
      when o == "nothing" && foe.include?(oo) then true 
      #when o == "nothing" && ranged_foe.include?(oo) then true 
      #when o == "nothing" && oo == friend then false
      when o == "nothing" && oo == "nothing" && foe.include?(ooo) then true
      else false
    end
  end

The variable names here are pretty obtuse. What exactly are o, oo, and ooo? They're related somehow, but that's all I can readily gather from the names.

Also, commented out code is dead code. Remove dead code. If you're worried that you'll need it, then I recommend you start using source control of some kind. Personally, I like Git.

\$\endgroup\$

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.