my attempt to create own quiz script

hello all. this is my probably 5th script i wrote. with limited knowledge i have in ruby, this is what i got so far.
test it and found no bug i guess.
however i do think i included many unrelated things in my scripts. if you guys can help me suggest correct way to write, please to teach me. i would love to improve myself further more. thank you.

Code: [Select]

module BANK
  SOALAN = {}
 
  SOALAN[0] = {
  0 => "which pokemon is third evolution?", #soalan
  1 => [4,5,6,7,8],   #nama gambar
  2 => ["Bulbasour","Squirtle","Blastoise","Raichu","bla bla bla bla bla bla bla bla bla Primate"], #pilihan jawapan
  3 => 2
  }   
 
  SOALAN[1] = {
  0 => "what happen during proses hohohoo?", #soalan
  1 => [1,2,3],   #nama gambar
  2 => ["pokok layu dan dan dan dan dan  ","batman","ultraman"], #pilihan jawapan
  3 => 0
  }

end


class WinQuestion < Window_Base
 
  def initialize(question_no)
    super(0,0,544,150)
    self.windowskin = Cache.system("Window")
    read_question(question_no)
    draw_text
    self.active = false
  end
  #method ini utk read soalan dari map.
  def read_question(question_no = 0)
    @question_no = question_no
  end
  #method utk draw text dari module
  def draw_text
    self.contents.clear
    self.contents.draw_wrap_text(0,0,544,24,BANK::SOALAN[@question_no][0])
  end

end

class WinPicture < Window_Base
 
  attr_accessor :pic_no
 
  def initialize(question_no)
    super(72,150,400,150)
    self.windowskin = Cache.system("Window")
    setup_picture(question_no)
    create_bitmap
    update_picture
  end
 
  def setup_picture(question_no = 0)
    @question_no = question_no
    #get the value of 1st element
    @the_array = BANK::SOALAN[@question_no][1]
    @pic_no = 0

  end
 
  def update_picture
    self.contents.clear
    if @pic_no < 0
      Sound.play_cancel
      @pic_no = 0
    elsif @pic_no > @the_array.size - 1
      Sound.play_cancel
      @pic_no = @the_array.size - 1
    end
    #Sound.play_cancel
    draw_picture(BANK::SOALAN[@question_no][1][@pic_no])
    draw_answer_text
  end
 
####### reusable code draw picture ############
  def draw_the_picture(pic_name, x, y, size = 124)
    bitmap = Cache.picture(pic_name)
    rect = Rect.new(0, 0, 0, 0)
    rect.x = 0
    rect.y = 0
    rect.width = size
    rect.height = size
    self.contents.blt(x, y, bitmap, rect)
    bitmap.dispose
  end
 
  def draw_picture(pic, x = 0, y = 0, size = 124)
    draw_the_picture(pic.to_s, x, y, size)
  end
###############################################

  def create_bitmap
    bitmap = Bitmap.new(265,140)
    @sprite_paper = Sprite_Base.new(@viewport3)
    @sprite_paper.bitmap = bitmap
    @sprite_paper.x = self.x + 150
    @sprite_paper.y = self.y + 12
    @sprite_paper.z = self.z + 1
  end
 
  def draw_answer_text
    @sprite_paper.bitmap.clear
    @sprite_paper.bitmap.draw_wrap_text(0,20,245,24,BANK::SOALAN[@question_no][2][@pic_no])
  end
 
  def dispose_bitmap
    @sprite_paper.bitmap.dispose
    @sprite_paper.dispose
  end
 
  def check_answer
    #p @pic_no
    if @pic_no == BANK::SOALAN[@question_no][3]
      #action happen
      #p "sama"
      $result = "true"
    elsif @pic_no != BANK::SOALAN[@question_no][3]
      #p "tak sama"
      $result = "false"
    end
  end
 
 
end

class WindowConfirm < Window_Selectable
 
  def initialize(question_no)
    super(((Graphics.width - 100)/2),((Graphics.height - 100)/2),100,100)
    self.windowskin = Cache.system("Window")
    @commands = ["YA", "TIDAK"]
    @item_max = @commands.size
    self.active = false
    self.index = 0
    self.back_opacity = 255
    self.visible = false
    self.z = 303
    @question_no = question_no
    refresh
  end
 
  def refresh
    create_contents
    for i in 0...@item_max
      draw_itout(i)
    end
  end
 
  def set_active
    self.index = 0
    self.visible = true
    self.active = true
  end
 
  def set_deactivate
    self.index = 0
    self.active = false
    self.visible = false
  end
 
  def draw_itout(index)
    rect = item_rect(index)
    self.contents.clear_rect(rect)
    self.contents.draw_text(rect, @commands[index], 0)
  end
 
end

class WinResult
 
  def initialize
    @image = Cache.picture("reward")
    @bitmap = Bitmap.new(200,200)
    bitmap = @bitmap
    @rewardsprite = Sprite_Base.new(@viewport3)
    @rewardsprite.bitmap = bitmap
    @rewardsprite.visible = false
  end
 
  def correct_or_false
    @rewardsprite.bitmap.clear
    bitmap = @bitmap
    @rewardsprite.bitmap = bitmap
    @rewardsprite.visible = true
    if $result == "true"
      rect = Rect.new(200,0,200,200)
    elsif $result == "false"
      rect = Rect.new(0,0,200,200)
    elsif $result == nil
      rect = Rect.new(0,0,0,0)
    end
    bitmap.blt(0, 0, @image, rect)
    @rewardsprite.z = 305
    @rewardsprite.x = (Graphics.width - 200)/2
    @rewardsprite.y = (Graphics.height - 200)/2
    @rewardsprite.opacity = 20
   
  end
 
  def rise_opacity
    @rewardsprite.opacity += 2
  end
 
  def down_opacity
    @rewardsprite.opacity -= 4
  end
 
 
  def dispose
    @rewardsprite.bitmap.dispose
    @rewardsprite.dispose
  end
 
  def hidden
    @rewardsprite.visible = false
  end
 
end


class Scene_F < Scene_Map
 
  alias f_init initialize unless $@
  def initialize(question)
    @question = question
    $gamestate = "Question"
    $result = nil
  end
 
  alias f_start start unless $@
  def start
    @winq = WinQuestion.new(@question)
    @winp = WinPicture.new(@question)
    @winc = WindowConfirm.new(@question)
    @winr = WinResult.new
    f_start
  end
 
  def terminate
    @winq.dispose
    @winp.dispose_bitmap
    @winp.dispose
    @winc.dispose
    @winr.dispose
  end
 
  def update
    @winq.update
    @winp.update
    @winc.update
    if $gamestate == "Question"
      if Input.trigger?(Input::RIGHT)
          Sound.play_decision
          @winp.pic_no += 1
          @winp.update_picture
            #break
      elsif Input.trigger?(Input::LEFT)
          Sound.play_decision
          @winp.pic_no -= 1
          @winp.update_picture
      elsif Input.trigger?(Input::X) #A
          Sound.play_decision
          $gamestate = "Try"
          @winc.set_active
      elsif Input.trigger?(Input::Y) #S
          Sound.play_cancel
          $scene = Scene_Map.new
      end #if inner
    end #if outer
   
    if $gamestate == "Try"
      if Input.trigger?(Input::C)
        case @winc.index
        when 0
          Sound.play_decision
          @winp.check_answer
          @winc.set_deactivate
          $gamestate = "Reward"
        when 1
          Sound.play_cancel
          @winc.set_deactivate
          $gamestate = "Question"
        end
      elsif Input.trigger?(Input::Y)
        Sound.play_cancel
        @winc.set_deactivate
        $gamestate = "Question"
      end
    end #if outer
   
    if $gamestate == "Reward"
     
      if $result == "true"
        Sound.play_load
        @winr.correct_or_false
        buffer = 200
        until buffer < 0
          $scene.update_basic
          buffer -= 2
          @winr.rise_opacity
        end
        buffer = 1
        until buffer > 200
          $scene.update_basic
          buffer += 2
          @winr.down_opacity
        end
        @winr.hidden
        #$gamestate = "Question"
        $scene = Scene_Map.new

      elsif $result == "false"
        Sound.play_buzzer
        @winr.correct_or_false
        buffer = 180
        until buffer < 0
          $scene.update_basic
          buffer -= 2
          @winr.rise_opacity
        end
        buffer = 1
        until buffer > 180
          $scene.update_basic
          buffer += 2
          @winr.down_opacity
        end
        @winr.hidden
        #$gamestate = "Question"
        $scene = Scene_Map.new

      end #if inner
    end #if outer
  end #def
end #class

 

If I was to give a noticeable suggestion, it would be to write more comments. I generally try to make sure I have at least 1 line of comment for every 5-8 lines of code. But even in cases of obvious code, I usually write something simple as a comment. I also do the same for every method and class I write, even if it's fairly obvious what is going on. The terminate method might have a comment which simply says "disposes windows".

I do like that you have commented the "end" keyword with what code/line the "end" relates to. However, I'd suggest using better words than "outer" and "inner". If I had 5 nested if/unless/while statements, it might be hard to know which is which. What I tend to do is write part of the line the if/unless/while etc says:

Code: [Select]

if apple_pie
  if four_serves

  else six_serves

  end #if four_serves
end #if apple_pie


Another thing that jumps out:

Code: [Select]

$result


One thing you should try to avoid is the use of $ to create global variables. There's only one case in RGSS where creating a global variable is acceptable - and even that is barely acceptable, and could be done better, in my opinion.

If you look at the default scripts you'll notice that only Game containers and the RM Data objects are accessible like this, like $game_party to access the instance of Game_Party, or $data_items to access the database version of an Item. This isn't necessary, but it allows scripters to access important game data easily. Note that word, "easily". Your $result variable is just as "easily" accessed, which is not a good thing for anything.

General rule of thumb: avoid the temptation to create global variables.

Instead, look to encapsulate it inside a class. You could use one of the Game containers (even though those are accessed through global themselves) like Game_Temp (the instance being accessed by $game_temp). The difference, however, is that these variables are references to a class instance, not just a value. This means you can protect the data a little better because we can add methods to Game_Temp.

Code: [Select]

class Game_Temp

  attr_reader :question_result

  def question_result=(result)
    @question_result = result if ( result.is_a?(TrueClass) || result.is_a?(FalseClass) )
  end

end


As a globally accessed variable, we can do:

Code: [Select]

$result = true
$result = false
$result = 5


This is the problem with it. It's a global variable which means it can reassigned at any time, which could cause issues.

By having to go through Game_Temp, we can ensure that result (in the example, @question_result) has to be a TrueClass (true) or FalseClass (false) object. If we tried to do:

Code: [Select]

$game_temp.question_result = 5


It wouldn't change question_result to 5. It can only be changed if we assign true or false.

The same applies to other global variables you have, like $gamestate.

Finally for the moment: whilst there aren't any official coding standards as far as I'm aware, there are some common conventions that can be followed. One of them is with regards to default method parameters. For example:

Code: [Select]

def default_param(default_value=5)
  #some method code
end


Notice that the default parameter assignment contains no spaces between the parameter name, the equals sign and the default value. That would be the common way to see it written, rather than:

Code: [Select]

def default_param(default_value = 5)
end


Its not a real issue with how the code works, but I thought I'd mention it anyway. On the same note, it's always good to try follow existing convention if you can.

For example, the class names for your custom windows.

WinPicture, WinQuestion, WindowConfirm.

Try to write them consistently. Instead, you should write these as:

Window_Picture, Window_Question, Window_Confirm

Of course, you might run into problems by using common looking names that other scripts may likely use. Instead, you could use:

Window_BANKPicture, Window_BANKQuestion, Window_BANKConfirm

It does mean you have to type a few more letters, but by writing the names like this it will reduce the chance of conflicting names.

Thank you soo much LoganF for analyzing my scripts.
i really appreaciate it.

i did my best to write the scripts like an essay, to improve readability, i'll follow what you have suggested.
i actually abit not confident with the way i wrote codes. i've read someone else codes in scripting section and to be honest i can see the level of difference between me and them.

i will wrote more codes to become familiar with it.

ok one more thing, if you notice the uses of $gamestate, i actually trying to control the flow of scripts by controlling the $gamestate. is that the correct way to do it? or any better suggestion for me. i use such way becouse i did learn LUA LOVE 2D in 1 months. i tend to see people use some kind of $gamestate to control their if/end/unless. i try to apply it in ruby.

and also, if you notice to,(i was thinking like) i do want to make a class become an object that have different properties(refer to methods) that can be called in scene later on.

is this correct way to do think like this? thank you again

i did my best to write the scripts like an essay, to improve readability, i'll follow what you have suggested.
i actually abit not confident with the way i wrote codes. i've read someone else codes in scripting section and to be honest i can see the level of difference between me and them.

You've done well already. Better code writing comes with practice.

Ruby is always about objects. Every thing that's a thing is an object.

In the case of gamestate, you can make simple classes. Something like:

Code: [Select]

class GameState

  attr_accessor :state

  def initialize(initial_state=:question)
     @state = initial_state
  end

end


Next, we need to make an instance of that to use within Scene_F:

Code: [Select]

class Scene_F < Scene_Map

  def initialize(question)
    @question = question
    @gamestate = GameState.new
  end

end


We don't need to alias a method unless it already exists. Because we are using a class which doesn't exist yet (Scene_F is something you created just now), initialize method doesn't exist.

In Scene_F, you can replace all $gamestate with @gamestate and it will still run the same. The difference is that @gamestate can only be accessed by Scene_F and nothing else, unlike $gamestate which can be accessed anywhere, by anything.

If you need to change @gamestate, you can just write:

Code: [Select]

@gamestate = :new_state


and it will change to the state you provide.

You will notice that I use text that starts with a colon :

This creates an object called a symbol (its class is Symbol). These are a cheap (in terms of system memory) way of creating unique objects. When we use them in our code we can do something like:

Code: [Select]

if @gamestate == :question
  # do question stuff
elsif @gamestate == :answer
  # do answer stuff
else
  # do other stuff
end


or a little neater:

Code: [Select]

case @gamestate
when :question
  # do question stuff
when :answer
  # do answer stuff
else
  # do other things
end


Here, the symbols replace the use of strings, and are very useful for controlling flow. In VX Ace, symbols are also written in an orange color text which makes them stand out from the rest of the code. That, too, is useful for you to see program control easily.

Play around with it, and you'll get the idea a little better.

Result is a bit different because it is simply a true or false state. There'd be no reason to make a custom class for it (although you could if you wanted to). I'd have to spend a bit more time looking into the code to see how best to change it. It may require some changes on how you design the program.

LoganF, i suddenly faced another problems here. i was wondering why i cant call the scene correctly?
did i miss something?

here is new scripts

Code: [Select]

module BANK
  SOALAN = {}
 
  SOALAN[0] = {
  0 => "which pokemon?", #soalan
  1 => [4,5,6,7,8],   #nama gambar
  2 => ["Bulbasour","Squirtle","Blastoise pokemon","Raichu","bla bla bla bla bla bla bla bla bla Primate"], #pilihan jawapan
  3 => 2
  }   
 
  SOALAN[1] = {
  0 => "what happen during proses hohohoo lalala lalala lalala lalala lalala?", #soalan
  1 => [1,2,3],   #nama gambar
  2 => ["cool","batman","ultraman"], #pilihan jawapan
  3 => 0
  }

end


class Window_Question
 
  def initialize(question_no=0)
    @question_no = question_no
    create_bitmap
    setup_sprite
    draw_question
  end
 
  def create_bitmap
    @width = 243
    @height = 110
    @bitmap = Bitmap.new(@width,@height)
    @sprite = Sprite_Base.new(@viewport3)
    @sprite.z = 306
    @sprite.x = 0
    @sprite.y = 0
  end
 
  def setup_sprite
    bitmap = @bitmap
    @sprite.bitmap = bitmap
    @sprite.bitmap.font.size = 18
    @sprite.bitmap.font.bold = true
  end
 
  def draw_question
    @sprite.bitmap.draw_text(0,0, @width, 22, BANK::SOALAN[@question_no][0])
    p "so this working?"
  end
 
  def dispose
    unless @sprite.nil?
      @sprite.bitmap.dispose
      @sprite.dispose
      @sprite = nil
    end
  end
 
end

class Scene_F < Scene_Map
 
  alias f_initialize initialize unless $@
  def initialize(question_no)
    @question_no = question_no
    f_initialize
  end
 
  alias f_start start unless $@
  def start
    @window_q = Window_Question.new(@question_no)
    f_start
  end
 
  alias f_terminate terminate unless $@
  def terminate
    @window_q.dispose
    f_terminate
  end
 
  alias f_update update unless $@
  def update
    f_update
    @window_q.update
  end
 
end



There's a couple of things you need to do.

Firstly, make sure that Window_Question extends from Window_Base, so that you can inherit the function of creating window objects.

Code: [Select]

class Window_Question < Window_Base


Next, you will need to create a window by sending information about the size of the window and the start draw x and y.

Code: [Select]

def initialize(question_no=0)
  super(0, 0, Graphics.width, Graphics.height)
  @question_no = question_no
  create_bitmap
  setup_sprite
  draw_question
end


That will create a window that is the same size as the game screen. You can play around with the positioning as you see fit.

You can remove all of those aliases in Scene_F as well. Having them isn't necessary because Scene_F is your own class. By using the "super" keyword, you can call the method in the base class (in this case Scene_Map) that shares the same name.

You also want to move the line:

Code: [Select]

@window_q = Window_Question.new(@question_no)


into the initialize method.

Code: [Select]

class Scene_F < Scene_Map
 
  def initialize(question_no)
    super
    @question_no = question_no
    @window_q = Window_Question.new(@question_no)
  end
   
  def terminate
    super
    @window_q.dispose
  end
 
  def update
    super
    @window_q.update
  end
 
end


that was really fast reply and very detail explanation.
ok i gonna apply what that have been suggested now.

especially that super part.

thank you so much for very fast support sir!