Hime-Works / Requests

Bug reports and requests that may require longer discussions and is not suitable to leave on the blog
http://himeworks.com/
GNU General Public License v2.0
7 stars 9 forks source link

Understanding Scenes & Windows?!? #92

Closed Roguedeus closed 10 years ago

Roguedeus commented 10 years ago

I knew this was gonna have to happen sooner or later... Gotta figure out GUI stuff.

I have managed to figure out that whenever a scene calls a window.new it inits with the x,y coordinates of the position, and the width,height sizes of the displayed information.

I have managed to figure our that each window has its own way of filling in what is contains... but for starters I am only referring to the Window_ItemList which knows it needs to draw each item based on the include? method.

Derived classes are the best way to manipulate windows, as it allows for selective overwriting of methods, like include? while maintaining default behaviors.

...

If I am mistaken please tell me.

Roguedeus commented 10 years ago

What I have not figured out...

Roguedeus commented 10 years ago

Ah... Ok...

The index is the courser location in the windows drawn list.

At any moment, grabbing the windows @item, will return the index of the windows @data array of items.

Since the windows draw_item method, draws the names to the selectable list in index order, it always lines up. Items that are not seen, were excluded from the @data array via the windows include? method.

I think.

HimeWorks commented 10 years ago

Here's the logic of the item scene

Here, you want to look at what it's doing. First, it instantiates the appropriate window class, passing in the appropriate x,y,width,height data if needed.

It then assigns some handles to the window handlers. You need to remember what those handles are because they are the ones that get called depending on what button you press.

Now you select a category and hit OK. It will run the method that is assigned to the OK handler

def on_category_ok
  @item_window.activate
  @item_window.select_last
end

So now it activates the item window, and the cursor is set to a particular item. You can move around to change your selection. Finally, you hit OK.

Referring back to the window handles, the item window was assigned the on_item_ok method for the OK handler.

def on_item_ok
    $game_party.last_item.object = item  # ??? what?
    determine_item
  end

The first line is not important; it's just for cursor memory. it then calls another method which does stuff to the item. This is defined in Scene_ItemBase

def determine_item
    if item.for_friend?
      show_sub_window(@actor_window)
      @actor_window.select_for_item(item)
    else
      use_item
      activate_item_window
    end
  end

Let's go straight to use_item

def use_item
    play_se_for_item
    user.use_item(item)
    use_item_to_actors
    check_common_event
    check_gameover
    @actor_window.refresh
  end

What is item?

Roguedeus commented 10 years ago

Thanks for pointing that out Hime.

It seems that in order to allow the crafter to select a specific instance as the ingredient(s) for the scene, I'll need to change the way Bubbles Crafting, processes ingredient availability, as well as how it selects the number of crafted items. Then, the result will need to destroy the original instance, and then call the Instance Items method for instantiating a new one.

And somewhere during that I'll need to allow the player to select the actual instance.

HimeWorks commented 10 years ago

Can you elaborate on why those are necessary?

I don't know how his crafting script works, but destroying and re-creating containers sound a bit strange (unless the containers you're talking about is not the inventory containers)

Roguedeus commented 10 years ago

I used the wrong word... I changed it to instance.

I assume that if I am going to allow the player to select the instance used in the crafting, I can't let the script use the template count as the item count, since each item is a unique instance.

I also assume that I am going to need to disable the script from allowing the player to select multiple results, via the Window_TOCraftingNumber.

The part about destroy/create may not be necessary, as the script calls the typical gain_item and lose_item methods.

Roguedeus commented 10 years ago
  #--------------------------------------------------------------------------
  # do_crafting
  #--------------------------------------------------------------------------
  def do_crafting(item, number)
    return unless item
    play_crafting_se(item)
    lose_ingredients(item, number)
    pay_crafting_fee(item, number)
    gain_crafted_item(item, number)
  end

  #--------------------------------------------------------------------------
  # lose_ingredients
  #--------------------------------------------------------------------------
  def lose_ingredients(item, number)
    item.ingredient_list.each do |ingredient|
      $game_party.lose_item(ingredient, number / @item.tocrafting_amount)
    end
  end

  #--------------------------------------------------------------------------
  # gain_crafted_item
  #--------------------------------------------------------------------------
  def gain_crafted_item(item, number)
    $game_party.gain_item(item, number)
  end
Roguedeus commented 10 years ago

Its ironic, that the vast majority of Bubbles Crafting script is simply code to manipulate the scene... So little code accomplishing the actual crafting.

I can understand why so many people avoid GUI code... But then, a good GUI programmer can write their own meal tickets these days.

HimeWorks commented 10 years ago
I assume that if I am going to allow the player to select the instance used in the crafting, I can't let the script use the template count as the item count, since each item is a unique instance.

Not necessarily. Let's say your materials are instance items and you need 5 pieces of wood to make an item. You have 5 pieces of wood in your inventory, each having their own unique instance ID, but all of them have the same template ID.

When you query the number of wood you have, it should say 5, because...well, you have 5 pieces of wood, even if they might be somewhat different from one another (texture, color, size, etc.)

Roguedeus commented 10 years ago

Thanks Hime, I was referring to the choosing of items with different levels of durability. I am looking to use the crafting system as a means of repair and improvement.

Without the instance variables making a difference in which is chosen, I agree that its not really necessary to force their selection.

Now that I am alive again... Long, long, long day yesterday, I will see what I can figure out today. :)

Roguedeus commented 10 years ago

I am lost.

So far no matter what I do, I can;t seem to get past creating the window.

I am trying to interrupt the crafting process at the point that ingredients are lost.

#def @ Line 2322
  #--------------------------------------------------------------------------
  # lose_ingredients
  #--------------------------------------------------------------------------
  def lose_ingredients(item, number)
    item.ingredient_list.each do |ingredient|
      $game_party.lose_item(ingredient, number / @item.tocrafting_amount)
    end
  end

When the ingredient is a template item, I want to open a list of that items catagory, listing only items of the same template, returning the selection to be lost... And repeat for as many instance items are used this way.

Then, allow the craft to complete via pay and gain crafted item.

  #--------------------------------------------------------------------------
  # do_crafting
  #--------------------------------------------------------------------------
  def do_crafting(item, number)
    return unless item
    play_crafting_se(item)
    lose_ingredients(item, number)
    pay_crafting_fee(item, number)
    gain_crafted_item(item, number)
  end

So far, at the point of selecting the item to craft, the number window shows, I select craft x1, and then the lose_ingredients method fires, creating the window I tell it to, listing (currently unfiltered) list of equips. I select the one I want to use, but the window doesn't disappear, and the do_crafting method seems to ignore the window completely and finishes the craft before I'm done interacting with my new window.

gl_crafting_1

lol... I am so ignorant of what I'm doing I feel stupid. :D

Here is what I've managed to hack together.

class Scene_TOCrafting < Scene_MenuBase

  #--------------------------------------------------------------------------
  # lose_ingredients
  #--------------------------------------------------------------------------
  def lose_ingredients(item, number)
    item.ingredient_list.each do |ingredient|
      if ingredient.is_template?
        puts(">>> Ingredient: " + ingredient.name) 
        gw = Graphics.width / 2
        gh = Graphics.height / 2
        @ingredient_window = Window_DismantleShopItemList.new(gw / 2, gh / 2, gw, gh)
        @ingredient_window.viewport = @viewport
        @ingredient_window.category = :weapon
        @ingredient_window.open.activate.select(0)
        @ingredient_window.set_handler(:ok,     method(:on_ingredient_ok))
        #Set ingredient as selected item.
        puts(">>> Losing: " + ingredient.name) 
      end
      $game_party.lose_item(ingredient, number / @item.tocrafting_amount)
    end
  end

  def on_ingredient_ok
    @item = @ingredient_window.item #Remember the selected item for retrieval.
    @ingredient_window.deactivate.hide
    @number_window.activate
    refresh
  end
end
Roguedeus commented 10 years ago

There is so much wrong with that code I made... Even I know it. I am stabbing in the dark...

At the moment I am simply trying to get the window to act correctly. Open, allow selection, and close on selection. Then assign the ingredient as the selected item. Then complete the craft.

HimeWorks commented 10 years ago

Are you sure on_ingredient_ok runs?

Roguedeus commented 10 years ago

Well, it appears its not going to be that easy...

gl_crafting_1b

As you can see... Even when dealing with instance items, the fact that the total available crafts are limited by template count (when other ingredients are limited) it arbitrarily limits the instances selectable from the list.

Roguedeus commented 10 years ago

I am pretty sure on_ingredient_ok is not running. As the item its supposed to set is always nil.

Roguedeus commented 10 years ago

Also, non-template items are returning is_template? true for some reason...

gl_crafting_1c

That should only be printing out instance templates. But the potion is being printed... and its not instance capable.

edit: Ok, I get it. the is_template? method is meant only to check for an actual instance as a false return, rather than whether the item is instance able... Thus all items NOT an instance returns is_template? true.

HimeWorks commented 10 years ago

First, even when a different window activates, the method still finishes running. This means that your lose_ingredients is going to open the window, but then deduct your ingredients. You need an else block in there.

Second, I don't think it really matters, but you should set handlers when you create the window. It is much cleaner. Unless you are planning to change your handles for some reason. It MIGHT be a reason why it doesn't run.

Roguedeus commented 10 years ago

I'm sorry Hime but I don't understand what you just said...

And I thought I did set the handler... @ingredient_window.set_handler(:ok, method(:on_ingredient_ok))

HimeWorks commented 10 years ago

You set your handler in lose_ingredients. Where are you creating the window? That's probably where you should be doing it.

Code doesn't automatically stop executing when you decide to open or activate a new window.

def myFunc
   @myWindow.activate
   p "cool"
end

If I call myFunc, my window is going to be activated, but then it will still print "cool" out to console.

Roguedeus commented 10 years ago

Please assume that I have no clue what you mean. As right now my brain is tied in knots.

I understand that the code doesn't stop just because I open a window. But I have no clue how to make it work otherwise. I have never worked with windows, or user input, past what was already default behavior. I have eyeballed the window code and can't recognize how I can get the cursor in the ingredient window to be anything more than visible, nor wait for the selected instance to be returned.

Roguedeus commented 10 years ago

I've tried to define my own item window, so its less confusing.

#===============================================================================
# 
#===============================================================================
class Window_LoseIngredientList < Window_ItemList

  #--------------------------------------------------------------------------
  def initialize(x, y, width, height)
    super(x, y, window_width, height)
  end

  #--------------------------------------------------------------------------
  def window_width
    Graphics.width / 2
  end

  #--------------------------------------------------------------------------
  def enable?(item)
    return false if item.nil?
    #Add check for item template ID.
    return !item.is_template?
  end

  #--------------------------------------------------------------------------
  def col_max
    return 1
  end

  #--------------------------------------------------------------------------
  #List only instanced items...
  def include?(item)
    case @category
    when :item
      item.is_a?(RPG::Item) && !item.key_item? && !item.is_template?
    when :weapon
      item.is_a?(RPG::Weapon) && !item.is_template?
    when :armor
      item.is_a?(RPG::Armor) && !item.is_template?
    when :key_item
      item.is_a?(RPG::Item) && item.key_item? && !item.is_template?
    else
      false
    end
  end
end
Roguedeus commented 10 years ago

Ok, I am now creating the window in alias start with the other windows. activating and deactivating via method calls. and Setting handler, on creation, rather than at the moment its activated.

#===============================================================================
# 
#===============================================================================
class Scene_TOCrafting < Scene_MenuBase

  #--------------------------------------------------------------------------
  alias :start_rb :start
  def start
    start_rb
    create_ingresients_window
  end 

  #--------------------------------------------------------------------------
  def create_ingresients_window
    gw = Graphics.width / 2
    gh = Graphics.height / 2
    #@ingredient_window = Window_DismantleShopItemList.new(gw / 2, gh / 2, gw, gh)
    @ingredient_window = Window_LoseIngredientList.new(gw / 2, gh / 2, gw, gh)
    @ingredient_window.viewport = @viewport
    @ingredient_window.category = :weapon #Will make this check template category.
    @ingredient_window.hide
    @ingredient_window.set_handler(:ok,     method(:on_ingredient_ok))
  end
  #--------------------------------------------------------------------------
  def activate_ingresients_window
    @ingredient_window.show.activate.select(0)
    refresh
  end
  #--------------------------------------------------------------------------
  def deactivate_ingresients_window
    @ingredient_window.hide.deactivate.unselect
    refresh
  end

  #--------------------------------------------------------------------------
  def lose_ingredients(item, number)
    item.ingredient_list.each do |ingredient|
      if ingredient.is_template?
        puts(">>> Ingredient: " + ingredient.name) 
        activate_ingresients_window
        #Set ingredient as selected item.
        puts(">>> Losing: " + ingredient.name) 
      end
      $game_party.lose_item(ingredient, number / @item.tocrafting_amount)
    end
  end

  #--------------------------------------------------------------------------
  def on_ingredient_ok
    @item = @ingredient_window.item #Remember the selected item for retrieval.
    deactivate_ingresients_window
    @number_window.activate
  end
end

Its acting exactly as it did before. With the exception that its no longer limiting the selectable items via 'other' ingredient limits... Now the list is showing all instanced items, rather than the other window I was using, with its enable? method.

HimeWorks commented 10 years ago

Are you saying that the logic is: if it sees a template item, it should open the lose ingredients window, WAIT for the player to choose an ingredient, and THEN print out "losing: ingredient" and finally deducting the item from the inventory?

Roguedeus commented 10 years ago

Yes, that's what I am trying to do. I can't seem to figure out how to get the :ok handler to call the on_ingredient_ok method. Or even wait for me to select from the list...

I don't blame you if you are getting frustrated with me. I learn by doing, and by figuring out examples (stare and compare) but this window/scene order of operations has me all mixed up.

HimeWorks commented 10 years ago

You don't tell the engine to wait. The engine is constantly updating its graphics and input, so it is always waiting until something happens. This is why when you say something like

if condition
   activate window
end
lose item

It doesn't matter whether the window activates or not, you're going to lose items. This is because you're telling the engine to deduct items. The solution is NOT to figure out how to tell the engine to wait.

Just from looking at it, I don't see anything wrong with the way the handler is set. There are likely other pieces of code that's changing your handles.

Roguedeus commented 10 years ago

Telling me to make it wait, doesn't help if I say for a third time that I have no idea how.

I am not sure what else I can do for the handler. I have isolated the window with my own class, which is the extent of my understanding.

HimeWorks commented 10 years ago

That's what I'm saying: you don't need to tell it to wait.

I can't see an issue with the code without running it myself, so you should consider posting it on one of the forums.

Roguedeus commented 10 years ago

Somehow, somewhere, my lines got crossed.

crossed

I am saying that I need the scene to wait for me to select an item from the new windows list... And I have no idea how to do that. I am speaking figuratively, as it relates to the way item lists normally work. All item list windows I have used, wait for a selection BEFORE doing anything. but mine isn't. And I am trying to understand what I need to do to make it.

Right now, the scene displays the window, but completes the crafting action, without waiting for me to select anything from it. Otherwise the window seems to function as it should. I can move the cursor, and even select from the list. But the window never goes away afterwords, likely because the :ok handler isn't firing. Which is another thing that's not working.

...

The problem I am sure to have, if I ask this on the forums, is that I am interacting with your instance items script, and Bubbles crafting script... So for anyone to answer me they must be familiar with both. Or at the very lest, familiar with the crafting scene as its what I am in when I create the window.

This is going to get my question ignored by most.

At any rate... Which forum do you suggest I ask the question?

Roguedeus commented 10 years ago

Found it... /facepalm

Roguedeus commented 10 years ago

Would this be a good time to mention that I am 100% self taught. Stare and compare. A few YouTube tutorials on fundamental code concepts... But otherwise I have never had an actual instructional class nor had a teacher to question. So if there are some basic things that you think I should know, and I don't. its probably because its never been a problem for me to get what I need done, without knowing it. Or I'd probably already know it.

I realize that makes me look silly sometimes, and I get stuck on relatively simple things, but I am always willing to be wrong and told how to do it right. I operate from a place of assuming I don't know, and working towards eventually knowing.

The times I make assumptions about things, are usually where I get into trouble.

HimeWorks commented 10 years ago

Your issue is related to general scene/window development, and not so much the actual scripts themselves. You should avoid trying to debug a scene/window script without being able to successfully create your own.

If you have a better understanding of the scene/window flow, you'll understand why you don't need the scene to wait for you. As I'm saying, the scene is always waiting until something happens, so as long as you don't tell it to do something, it won't go and do something by itself.

This is why it's important to break your logic into individual methods. The set method is defined in Window_ShopNumber

Right now, the scene displays the window, but completes the crafting action, without waiting for me to select anything from it. 

I'm not sure how else to explain it. This is what your code is doing

def lose_ingredients(item, number)
    item.ingredient_list.each do |ingredient|
      if ingredient.is_template?
        puts(">>> Ingredient: " + ingredient.name) 
        activate_ingresients_window
        #Set ingredient as selected item.
        puts(">>> Losing: " + ingredient.name) 
      end

      ####
      $game_party.lose_item(ingredient, number / @item.tocrafting_amount)
    end
  end

The last line is always executed, regardless whether you activate another window or not. You need to have two methods: one method checks whether you need to select items or not, and the other method to actually deduct items.

# When you select an item to craft, the window will pass control to the scene
def on_item_ok
   if item.is_template?
      activate_window
   else
      lose_ingredients(item, ... )
   end
end

You don't need to modify lose_ingredients at all.

Roguedeus commented 10 years ago

I am attempting to trace the order of operations in the crafting scene...

SceneManager.call(Scene_TOCrafting) is called, and SceneManager.scene.prepare(args)

I see that the call method, simply creates a Scene_TOCrafting .new object, I can find where @scene is defined. I can find Scene_TOCrafting > prepare

The very first selectable window is the recipelist_window. I can see that the reason this window is the first one that is selectable, is that its the only selectable window that is not set hide or close upon its creation.

    create_help_window              <--- 
    create_gold_window              <--- Hide & Close
    create_cover_window             <--- 
    create_info_window              <--- Hide Only
    create_itemlist_header_window   <--- Hide & Close
    create_itemlist_window          <--- Hide & Close
    create_recipelist_window        <--- 
    create_number_window            <--- Hide & Close
    create_result_window            <--- Hide & Close

Here is the rough order of operation within the Crafting Scene...

    Select a Recipe Book:
      @recipelist_window.set_handler(:ok,     method(:on_recipelist_ok))
      @recipelist_window.close
      activate_itemlist_window      <--- 

    Item List is Activated:
>     refresh
      @itemlist_window.show.open.activate.select(0)

    Select the Item to Craft:
      @itemlist_window.set_handler(:ok,     method(:on_itemlist_ok))
      @item = @itemlist_window.item

    Item List is Closed & Hidden:
      @itemlist_window.close.hide

    Number Window is Setup & Activated:
      @number_window.set(@item, max_craft, crafting_fee)
      @number_window.show.open.activate

    Number Window is Accepted (OK):
      @number_window.set_handler(:ok,     method(:on_number_ok))

    Number Window is Closed & Hidden:
      @number_window.close.hide

    Crafting Action is performed:
      do_crafting(@item, @number_window.number)

    Result Window is Setup & Activated:
      @result_window.set(@item, @number_window.number)
      @result_window.show.open.activate

    Item List is Opened & Shown:
      @itemlist_window.show.open
>     refresh

The first thing that pops out at me is the fact that out of the entire process, do_crafting is the only method called inside a handler method, besides check_common_event, that isn't simply updating a window data...

Roguedeus commented 10 years ago

I am going to try and digest what I have figured out today (as little as that is) and then attack this anew in the morning... Right now I am so scrambled and frustrated I am afraid I'm going to piss you off. :)

Once again, I must thank you for putting up with me!!

HimeWorks commented 10 years ago

My recommendation is to get a simple scene and window working, so that you at least know that you can successfully assign a method to a handler and switch between windows.

Otherwise, you have no idea where to start debugging.

Roguedeus commented 10 years ago

Good idea... I was so brain locked I hadn't considered that the primary reason it may not be working was that I had not figured out how to even make a one shot scene/window in the first place.

Baby steps. :)

So far I am able to call, display, and cancel out of the scene. However, the item list doesn't populate at all...

gl_testscene_1

edit: It would help if I had inventory to list... /facepalm

Roguedeus commented 10 years ago

Done... Works fine. gl_testscene_1b

I can even retrieve the selected item.

Note: I didn't altar my window at all... besides removing the hide and close calls on creation, and adding a cancel event_handler.

Roguedeus commented 10 years ago

Holy shit I think I've got it...

Roguedeus commented 10 years ago

Im not going to pretend I understand exactly why, but it SEEMS that the primary issue I was having was the fact that every action in an active Scene_MenuBase has to end with one of these scenes windows waiting on a handler.

And I was calling a new window on top of an already active window, waiting on its handler.

So far, I think I have a FUNCTIONAL plugin for bubbles crafting using instance items. I'm not going to pretend that its bug free enough to release for open use, but so far its enough. I am going to spend the next few hours poking it, to see if I can break it somehow.

Roguedeus commented 10 years ago

Video: http://youtu.be/zs9JsVce-x0

In action... Later on I'll place the window in the space left open and not leave it in the center of the screen.

Time to rest my noggin.

HimeWorks commented 10 years ago

Scenes don't check which window is active to determine which method to call. Rather, it's the other way around: the window will determine which method to call, and then basically call it.

All actions (or more commonly referred to as "events", as in event-driven programming, not RM events) are registered via handles.

RM selectable window provides 8 handlers that you can set: ok, cancel, up, down, left, right, pageup, pagedown. You can easily add and define your own.

When you press ok on your window, the window will look up which method (or methods, if you modify it to accept multiple handles per handler)

The demo looks pretty nice. The window disappearing before your custom item selection window appeared was kind of strange but that's probably something that's easily resolved.

Roguedeus commented 10 years ago

Here is what I have managed to complete. Including placing the ingredients window in the correct spot in the scene.

#RogueDeus  14_0117

#===============================================================================
# 
#===============================================================================
class Game_Interpreter
  def call_testselect_scene
    SceneManager.call(Scene_TestSelect)
  end
end

#===============================================================================
# 
#===============================================================================
class Window_LoseIngredientList < Window_ItemList
  attr_accessor :template_id
  attr_accessor :ingredients
  attr_accessor :count

  #--------------------------------------------------------------------------
  def initialize(x, y, height)
    super(x, y, window_width, height)
  end

  #--------------------------------------------------------------------------
  def window_width
    Graphics.width / 2
  end

  #--------------------------------------------------------------------------
  def enable?(item)
    return false if item.nil?
    return !item.is_template? #List only instances.
  end

  #--------------------------------------------------------------------------
  def col_max
    return 1
  end

  #--------------------------------------------------------------------------
  #List only instanced items...
  #Add check for item template ID.
  def include?(item)
    case @category
    when :item
      item.is_a?(RPG::Item) && !item.key_item? && !item.is_template? && @template_id == item.template_id
    when :weapon
      item.is_a?(RPG::Weapon) && !item.is_template? && @template_id == item.template_id
    when :armor
      item.is_a?(RPG::Armor) && !item.is_template? && @template_id == item.template_id
    when :key_item
      item.is_a?(RPG::Item) && item.key_item? && !item.is_template? && @template_id == item.template_id
    else
      false
    end
  end

  #--------------------------------------------------------------------------
  def draw_item(index)
    item = @data[index]
    if item
      rect = item_rect(index)
      rect.width -= 4
      draw_item_name(item, rect.x, rect.y, enable?(item), rect.width - 24)
      #Removed item count draw... We are only listing instances.
    end
  end

  # alias :refresh_rd :refresh
  # def refresh
  #   puts(">>> Window_LoseIngredientList: *** REFRESH ***")
  #   refresh_rd
  # end

  # alias :make_item_list_rd :make_item_list
  # def make_item_list
  #   puts(">>> Window_LoseIngredientList: *** MAKING ITEM LIST ***")
  #   make_item_list_rd
  # end  

end

#===============================================================================
# Test Scene to verify window functions.
#===============================================================================
class Scene_TestSelect < Scene_MenuBase

  #--------------------------------------------------------------------------
  def start
    super
    create_ingredients_window
    activate_ingredients_window(:weapon)
  end

  #--------------------------------------------------------------------------
  def create_ingredients_window
    puts(">>> Creating Ingredients Window...")
    gw = Graphics.width / 2
    gh = Graphics.height / 2
    @ingredient_window = Window_LoseIngredientList.new(gw / 2, gh / 2, gh)
    @ingredient_window.viewport = @viewport
    @ingredient_window.category = :armor 
    @ingredient_window.set_handler(:ok,     method(:on_ingredient_ok))
    @ingredient_window.set_handler(:cancel, method(:on_ingredient_cancel))

  end

  #--------------------------------------------------------------------------
  def activate_ingredients_window(catagory = :armor)
    puts(">>> Activating Ingredients Window...")
    @ingredient_window.category = catagory
    @ingredient_window.refresh
    @ingredient_window.show.activate.select(0)
  end
  #--------------------------------------------------------------------------
  def deactivate_ingredients_window
    puts(">>> De-Activating Ingredients Window...")
    puts(">>> #{@ingredient_window.item.name} - Selected.")
    @ingredient_window.hide.deactivate.unselect
    return_scene #<--- Returns to main scene. 
  end

  #--------------------------------------------------------------------------
  def on_ingredient_ok
    puts(">>> Ingredients Window: OK")
    @item = @ingredient_window.item #Remember the selected item for retrieval.
    deactivate_ingredients_window
  end

  #--------------------------------------------------------------------------
  def on_ingredient_cancel
    return_scene
  end

end

#===============================================================================
# 
#===============================================================================
class Scene_TOCrafting < Scene_MenuBase

  #--------------------------------------------------------------------------
  alias :start_rb :start
  def start
    start_rb
    create_ingredients_window
  end 

  alias :refresh_rd :refresh
  def refresh
    refresh_rd
    @ingredient_window.refresh
  end

  #--------------------------------------------------------------------------
  def create_ingredients_window
    #puts(">>> Creating Ingredients Window...")
    wx = 0
    wy = @help_window.height + @itemlist_header_window.height
    wh = Graphics.height - wy
    wh = wh - @gold_window.height if gold_window?
    @ingredient_window = Window_LoseIngredientList.new(wx, wy, wh)
    @ingredient_window.viewport = @viewport
    @ingredient_window.category = :armor 
    @ingredient_window.ingredients = []
    @ingredient_window.hide.close
    @ingredient_window.set_handler(:ok,     method(:on_ingredient_ok))
  end

  #--------------------------------------------------------------------------
  def activate_ingredients_window
    # puts(">>> Activating Ingredients Window...")

    index = @ingredient_window.count - 1
    ingredient = @item.ingredient_list[index]
    # puts(">>> Ingredients Index: #{index} = #{ingredient.name}")
    @ingredient_window.template_id = ingredient.id

    if ingredient.is_a?(RPG::Weapon)
      ingredient_weapon_select
    elsif ingredient.is_a?(RPG::Armor)
      ingredient_armor_select
    else
      @item.tocrafting_amount.times {@ingredient_window.ingredients.push(ingredient)}
      @ingredient_window.count -= 1
      @ingredient_window.hide.deactivate.unselect

      if @ingredient_window.count <= 0
        deactivate_ingredients_window 
      else
        activate_ingredients_window
      end
    end
  end

  #--------------------------------------------------------------------------
  def ingredient_weapon_select
    @ingredient_window.category = :weapon
    @ingredient_window.refresh
    @ingredient_window.show.open.activate.select(0)
  end

  #--------------------------------------------------------------------------
  def ingredient_armor_select
    @ingredient_window.category = :armor
    @ingredient_window.refresh
    @ingredient_window.show.open.activate.select(0)
  end

  #--------------------------------------------------------------------------
  def on_ingredient_ok
    @ingredient_window.hide.close
    # puts(">>> Ingredients Window: #{@ingredient_window.item.name}")
    @ingredient_window.count -= 1
    @ingredient_window.ingredients.push(@ingredient_window.item)

    if @ingredient_window.count <= 0
      deactivate_ingredients_window 
    else
      activate_ingredients_window
    end
  end

  #--------------------------------------------------------------------------
  def deactivate_ingredients_window
    # puts(">>> De-Activating Ingredients Window...")

    @ingredient_window.ingredients.each do |ingredient|
      # puts(">>> Losing: " + ingredient.name)
      $game_party.lose_item(ingredient, 1)
    end

    play_crafting_se(@item)
    pay_crafting_fee(@item, 1)
    gain_crafted_item(@item, 1)

    @ingredient_window.ingredients = []
    @ingredient_window.hide.close

    @result_window.set(@item, 1)
    @result_window.show.open.activate
    @itemlist_window.show.open
    @info_window.page_change = true
    @gold_window.number = @info_window.number = 1
    refresh
    @result_window.show.open.activate
  end

  #--------------------------------------------------------------------------
  def on_number_ok
    @number_window.close.hide
    #@number_window.number

    if uses_instance_ingredient?(@item)
      @ingredient_window.count = @item.ingredient_list.length
      # puts(">>> Ingredients: #{@ingredient_window.count}")
      activate_ingredients_window
    else
      normal_crafting
    end
  end

  #--------------------------------------------------------------------------
  def uses_instance_ingredient?(item)
    item.ingredient_list.each do |check|
      #Needs logic to check instance items script for category truth.
      return true if check.is_a?(RPG::EquipItem)
    end  
  end

  #--------------------------------------------------------------------------
  def normal_crafting
    do_crafting(@item, @number_window.number)
    @result_window.set(@item, @number_window.number)
    @result_window.show.open.activate
    @itemlist_window.show.open
    @info_window.page_change = true
    @gold_window.number = @info_window.number = 1
    refresh
    @result_window.show.open.activate
  end

  #--------------------------------------------------------------------------
  # alias :gain_crafted_item_rd :gain_crafted_item
  # def gain_crafted_item(item, number)
  #   puts(">>> Gained: " + item.name)
  #   gain_crafted_item_rd(item, number)
  # end

end

Its my first attempt at understanding scene windows. I'd appreciate it if you can critique it and tell me if I did anything unnecessarily, or could do something better... Even where I can methodize parts I haven't yet and why.

I think I will look for reasons to revisit this so i can reinforce this scene/window stuff. As its so rarely done, yet so damned important. :)

Roguedeus commented 10 years ago

Damn Hime. I am seeing your influence in almost all the code I write now... :)

HimeWorks commented 10 years ago

There are a number of "best practices" blog posts available on the internet to read about, and since you've actually done some coding and run into problems, you should be able to better understand what they're talking about.

When it comes to methods, I write a little comment that summarizes that the method does in one sentence. Preferably, less than a line or two. The rule I follow is

The reason why doing multiple things in one method can cause problems is because if I wanted to change how some of those things behave, I'm probably going to have to overwrite your method. Thus, incompatbility. Therefore, if you simply call a method that returns the value you need, I can minimize overwrites.

Window logic tend to be long because of how you're defining how things look and feel etc, but scene logic shouldn't. For example, I skimmed through it and saw a large method

def deactivate_ingredients_window
    # puts(">>> De-Activating Ingredients Window...")

    @ingredient_window.ingredients.each do |ingredient|
      # puts(">>> Losing: " + ingredient.name)
      $game_party.lose_item(ingredient, 1)
    end

    play_crafting_se(@item)
    pay_crafting_fee(@item, 1)
    gain_crafted_item(@item, 1)

    @ingredient_window.ingredients = []
    @ingredient_window.hide.close

    @result_window.set(@item, 1)
    @result_window.show.open.activate
    @itemlist_window.show.open
    @info_window.page_change = true
    @gold_window.number = @info_window.number = 1
    refresh
    @result_window.show.open.activate
  end

Can you say this is doing one thing? Just from looking at, you're

First, this method is called "deactivate ingredients window". It doesn't sound like it should have anything to do with deducting items...or playing sounds...or setting variables...or refreshing the scene.

What if I wanted to deduct items in another method? Would I have to duplicate the same code because I don't want to actually de-activate the particular window?

Roguedeus commented 10 years ago

I expected that method to pop out. But I am affraid to touch it... That is the crux of my problem here as I am not EXACTLY sure what I did to make this work as well as it is. I best guessed, based on what I saw other scripts doing, and didn't want to jinx it (jokingly) by messing with it once I got it to do what I wanted.

That method started as doing nothing more then deactivating the window... But I added all the rest, to see if it would work. And when it did, I just left it be.

As for the notion regarding methods and explanations, I agree completely. As a matter of fact, the comment I made about seeing your influence in my code, was based on that very thing. This Golem Travel script I am writing, is full of do-one-thing methods. :)

Would that be your only suggestion?

HimeWorks commented 10 years ago

It's the biggest issue I see with regards to compatibility, which usually what I'm concerned about when it comes to scripting. Performance is important as well, but there doesn't seem to be anything really inefficient. People can probably nitpick a bunch of things but I don't really care how people want to code as long as it doesn't affect others' stuff.

Roguedeus commented 10 years ago

I realize that many, if not most, people that seek feedback are really only asking for agreement. But I may be unusual in that I really only ask for feedback when I want to improve my work, and thus I expect as honest feedback as I can get. Even opinions have merit as I may have never thought of it.

Please, if there is anything constructive or even pet peevish about something I request feedback on, feel free to share.

HimeWorks commented 10 years ago

If I see something bad I will point it out.

But if it's too long I probably will choose not to read it or just look for things that are obviously bad rather than looking at each method closely.

Roguedeus commented 10 years ago

It seems I may have an opportunely to practice this sooner than I thought...

Tomorrow I'll try to find out how to display free text in the main scene. Nothing fancy.

Roguedeus commented 10 years ago

Its not the prettiest thing, but its functional. I will refine it later.

gl_simplehud_8