Bug finding: actor initialization

● ARCHIVED · READ-ONLY
Started by Tsukihime 10 posts View original ↗
  1. Tsukihime
    #1 Source
    This is the default script

    Code:
    #==============================================================================# ** Game_Actors#------------------------------------------------------------------------------#  This is a wrapper for an actor array. Instances of this class are referenced# by $game_actors.#==============================================================================class Game_Actors  #--------------------------------------------------------------------------  # * Object Initialization  #--------------------------------------------------------------------------  def initialize    @data = []  end  #--------------------------------------------------------------------------  # * Get Actor  #--------------------------------------------------------------------------  def [](actor_id)    return nil unless $data_actors[actor_id]    @data[actor_id] ||= Game_Actor.new(actor_id)  endend
    Can you see any problems or come up with a scenario where problems would occur?
  2. Another Fen
    #2 Source
    I don't know if I get your point here, but I think there are some possibilites to mess up:

    When using a negative number as an argument, [] could create an actor it is not supposed to.

    $game_actors[5]; $game_actors[-2]
    may cause the creation of actor 'second-last' in the slot of actor 4.

    In theory you could provoke that using event commands.

    You can also create actors with fractional IDs the same way, although I don't know if that would even cause an error when using the standard scriptset only.

    Other classes may rely on $game_actors not to return nil for certain indexes. For example, when you delete some database actors in the editor, then load an old savestate where one of the deleted actors is a party member, you will run into an error sooner or later.
  3. TheoAllen
    #3 Source
    I have no objection with this current default structure.

    If the problems come up, it means that you just did something you shouldn't
  4. Tsukihime
    #4 Source
    The way it is setup, it is REALLY easy to throw yourself in a recursive loop without even realizing.


    I've gotten hit by it several times.
  5. Shaz
    #5 Source
    Really? How would you manage to throw yourself into a recursive loop with this? $game_actors and $data_actors are two completely different things.
  6. nio kasgami
    #6 Source
    Shaz said:
    Really? How would you manage to throw yourself into a recursive loop with this? $game_actors and $data_actors are two completely different things.
    I assume he using the Data_Actors for handle some extra information? but i am not sure I rarely use the Data_Actor myself 
  7. Tsukihime
    #7 Source
    Shaz said:
    Really? How would you manage to throw yourself into a recursive loop with this? $game_actors and $data_actors are two completely different things.
    $game_actors[x] is supposed to return actor x.

    So you would expect that if you accessed $game_actors[2], it would always give you actor 2.

    And this is a reasonable assumption.

    However, every so often I would see code like this

    Code:
    class Game_Actor  alias :old_refresh :refresh  def refresh    old_refresh    CustomModule.check_something(id)  endendmodule CustomModule  def self.check_something(actor_id)    $game_actors[actor_id].some_attr == some_var  endend
    And now you have perfect conditions for system stack error.
  8. Shaz
    #8 Source
    Oh yeah, that's a disaster waiting to happen. But that's not because of the way Game_Actors is designed (and it would happen even if there was no reference back to $data_actors at all) - it's because of the way that particular scripter designed the script. You could probably cause the same problem with ANY class - even something as basic as variables and switches. I wouldn't consider that a bug in actor initialization - I'd consider it a bug in the script you just provided.
  9. Engr. Adiktuzmiko
    #9 Source
    I'd probably just have another version of that which is already inside Game_Actor and doesn't take any parameter.
  10. DoubleX
    #10 Source
    In this particular case, I'd change that code to something like this:

    class Game_Actor alias :old_refresh :refresh def refresh old_refresh some_attr == CustomModule.some_var endendAnd to avoid the aforementioned recursive loop, never reference an actor via $game_actors before that actor's completed its initialization. Usually an actor's initialized first before it''ll be accessed by something else.

    If, in some cases, an actor does need to be accessed by something else during that actor's initialization, I'd try something like this:

    module CustomModule def self.check_something(actor) # Pass the whole game actor instead of that actor's id # do_something endendThis way $game_actors isn't used at all, avoiding the aforementioned recursive loop.