Cross Reference tool

[Shaz]

So it's like the RPG classes that, in addition to just storing the data, also provide methods to access it in different ways or to do calculations/evaluations based on its contents?

 

[Tsukihime]

Not really. Well, in the context of "splitting responsibilities".

Consider a possible implementation of the RPG::Item class, which does basically the same things.

class RPG::Item attr_accessor :name attr_accessor rice attr_accessor :damage_type attr_accessor :formula def eval eval(@formula) endendInstead of storing a separate "damage" object, we just let the item handle all of it.A "responsibility" is basically what an object needs to do. An item needs to know its name and its price (which involves modifying them and returning them when you need it). Those are two responsibilities.

And then you're telling it to keep track of a damage formula, how to evaluate the damage formula, and what kind of damage it does. That's 3 additional responsibilities. Does an item really need to do that?

When you look at the item object, you will notice that the Scope is stored with the item. Does an item really need to keep track of its scope? It's something I ask often when I look at it, much like how you're looking at your own script and asking whether you really need to deal with this extra data class rather than just storing everything in your Xref module and letting it handle everything.

 

[Shaz]

So, something like this?

Spoiler

class Reference # has type, id, source, location, page, line, reference (code)endclass DataXRefs def initialize @data = {} end def setup(type, id) @data[type] = [] if @data[type].nil? @data[type][id] = [] if !id.nil? && @data[type][id].nil? end def add(type, id, ref) setup(type, id) @data[type][id].push(ref) end def ref(type, id = nil) setup(type, id) # ^ is it good or bad to call this here? # If an element doesn't exist, it'll create it, which will let the return value always be an array, even if it's empty # Classes calling this then just need to check for .empty? and not .nil? as well @data[type] if id.nil? @data[type][id] endendmodule SHAZ # <-- is this even needed? Two levels of modules? module XRef # constants, hashes for symbols def self.load_xrefs # stuff end def save_xref(type, id, reference) ref = Reference.new(args) DataXRefs.add(type, id, ref) end # methods to extract data and call save_xref endendclass Window_XRefSelections < Window_Selectable # uses DataXRefs.ref(symbol)endclass Window_XRefResults < Window_Selectable # uses DataXRefs.ref(symbol, id)end

Obviously much abbreviated.

Should the two classes be inside the module?

 

[Tsukihime]

"Where" you should define your classes and methods is, again, up to you to decide.

If you define it as simply

class ReferenceendThat means if anyone also defined their own class as "Reference", you could potentially run into problems with conflicting definitions and behaviors.If you define it ias

module XRef class Reference endendThere is usually no confusion because you can explicitly refer to it as XRef::Reference, and the chances of there being name conflicts is much lower. Of course, if someone wanted to call their module XRef and they also placed their "Reference" class in there...There are technical reasons why one would use a module, but when it comes to

module SHAZ module XRef class Reference end endendI think it's purely for organization and to avoid name collisions (cause I'm sure no one's going to go and call their module SHAZ).

def ref(type, id = nil)

setup(type, id)

# ^ is it good or bad to call this here?

# If an element doesn't exist, it'll create it, which will let the return value always be an array, even if it's empty

# Classes calling this then just need to check for .empty? and not .nil? as well

@data[type] if id.nil?

@data[type][id]

end

I don't think there is anything wrong with making sure the data is initialized, but it means each time you call the ref method, you're going to call the initialization, which is sort of redundant after you call it once.

Which sort of contradicts my usual practice since I always say things like

load_notetag_something unless @somethingreturn @something

But that's mainly because of loading-from-save issues...

 

[Shaz]

It's more to create it so I can just return something, than to say

return [] (or nil) if @data[type].nil?return [] if id && @data[type][id].nil?return @data[type] if id.nil?return @data[type][id]

One or the other will have to be done every time anyway. Similarly, calling it when saving the data means something will only be done the first time that type and id are used, and every other time the method will be called, but will just skip through.

 

[Tsukihime]

It would skip through, but it still takes processing time to check whether you need to skip or not, and some people don't think that is acceptable.


I would prefer dynamically building a hash as I go rather than building everything initially (eg: all types, for all ID's), but it really depends on what you're doing. If it's related to graphics, and that extra processing time will lead to noticeable delays, you should re-consider it. If it's just running a script and letting it run for a minute, doesn't really matter whether the user waits an extra 0.01 seconds or not.

 

[Shaz]

You are building it as you go, but you still have to check, before you push something to an array, that the array actually exists. You have to check that it exists every single time.

 

[Tsukihime]

Is there a way to avoid having to check it every time?

 

[Shaz]

Only by setting them all up right at the beginning. I can't think of any other way. Can you?


mmm ... unless I just assume they will be there, and have some exception handling to create them if they're not.

 

[Tsukihime]

I don't know the scope of your tool, but there are a few general ways to avoid loading everything whenever you need to access it


1. Load everything initially (assumes all data exists already)


2. Load it whenever you instantiate a new type, or a new ID (assumes data can be generated at run-time, but it's ok because you'll only initialize it once)


Preferably, I would consider any of these options, cause lazy loading to me is the same as brute forcing your way through.

 

[Liak]

I'm here to answer to the lot I have to answer for. While I don't understand anything of what you are talking about (but reading it nonetheless), I have to say the project looks great so far. I tried it in a new project and it worked just fine. In my real project, it gave me an error, though. The script you suggested did not work for some reason, or I could not find where anything would have popped up that could help you find the error. But now that all the discussion and changes are going on, I guess I'll just wait for the next/final version and try running that one, reporting any errors that might occur? I wish you much success working on it, and I'm glad you can see uses for it in your own projects, too.

 

[Shaz]

Did it give you an error message? I'd be interested to see it. You're trying the script just as it is in pastebin, right? Not with any of the code snippets we've been discussing along the way?


I'd like to get this finished, and get the swapping script done - for my own project too now. So I'm continuing on with this

 

[Liak]

I used this: http://pastebin.com/kEuTzgSr

Nothing else besides it, not even my custom script.

And it gave me this error: http://image-upload.de/image/Vszamz/90a861a0b2.png

Hope it helps.

 

[Shaz]

Awesome!


Can you change line 345 to say cond.actor_id instead of cond.actor_index? That should solve it


Zeriab will be along soon to remind me to make a map to test every piece of data that might be picked up (or, in this case, a troop event)

 

[Liak]

That worked! Great! I checked a few things of which I knew where I used them, everything was correct. Thank you, that already helps a lot, as it saves a lot of time playtesting!

I'm really looking forward to the complete tool (with swapping function), and I hope many people will find that as useful as the two of us.

 

[Zeriab]

I am trying to teach you about object-oriented design ^_^

Zeriab will be along soon to remind me to make a map to test every piece of data that might be picked up (or, in this case, a troop event)

I will indeed XD

Quite useful such a project would be. Both for testing your current script and change new versions. I say project rather than map because your script is not only looking at the maps.

Now I do not really like your focus so let me try to guide it back with restrictions. You are not allowed to do nested calls anymore. i.e. @data[type][id].push(ref) is not allowed.
This rules do allow unraveling the code like this.

type_data = @data[type]xrefs = type_data[id]xrefs.push(ref)Another rule is that direct access to data structure is disallowed except in methods that only gets, sets or check existence. Whenever you are creating a hash/array do create a class that internally takes care of that hash/array. Only those get/set/exist methods in that class is allowed to touch the hash/array. This includes any other method you may decide put in the class. Your code could look something like this

# Top objecttype_data = this.get_data(type)type_data.add(id, ref)

# Method in other objectxrefs = this.get_id(id)xrefs.push(ref)

Get methods deliberately changed to put conceptual distance between the normal practices. The names are poorly chosen. You will do better. Choose good names is hard, no doubt about that, but at the same time it is important, particularly as your code grows.

To rephrase:

• No nested method/function calls
• No direct access to the raw data structure (array/hash)

Oh, and try to put code where it belongs according to the abstractions.
 

*hugs*

 - Zeriab

 

[Liak]

Another error! :/

Line 146: TypeError occured.

Can't convert nil into Integer

 

[Shaz]

I'll see if I can get a newer version of this up soon. There are a number of places on that line that could have triggered the error, so it's a bit hard to tell.

 

[Liak]

Oh, that's bad. :/ Then I'd like to wish you good luck!