ID:2183415
 
(See the best response by Kaiochao.)
Okay, I am just now learning how to use a datum and was doing an experiment to see if I could use it for my skills.

the variable to hold a players skills is
mob
var
skills[0]

So for the mob I wrote
mob/Login()
var/tmp/skill/trading/TSkill = new /skill/trading()
TSkill.rank = 5
src.skills["Trade"] = TSkill
return ..()

mob/Bump(O)
if (istype(O,/mob))
var/tmp/mob/M = O
var/tmp/skill/trading/TradeSkill = usr.skills["Trade"]
TradeSkill.TradeGoods(usr,M)
return ..()


and for the datum I wrote
skill
var
label
rank
exp

trading
label = "Trade"
proc
TradeGoods(U,T)
var/tmp/mob/User = U
var/tmp/mob/Target = T
var/tmp/skill/trading/UserTrade = User.skills["Trade"]
var/tmp/skill/trading/TargetTrade = Target.skills["Trade"]
var/tmp/UserRoll = rand(1,20) + UserTrade.rank
var/tmp/TargetRoll = rand(1,20) + TargetTrade.rank
UserTrade.exp++
User << "Trade Successful [UserRoll] - [TargetRoll] total exp [UserTrade.exp]"
User << "Your rank is [UserTrade.rank]"


The problem is that this returns runtime error: Cannot read null.rank

Which is crazy because if I exclude the rolls and leave
                User << "Your rank is [UserTrade.rank]"


it tells me Your rank is 5 !!! So it is getting the rank successfully if I am sending it in text but the skill grab turns up null if I try to use it as a number?

My problem with understanding why a datum is useful is due to getting errors like this anytime I have tried to experiment with one. This error doesn't make sense to me.

Best response
1. usr is not appropriate in Bump(). You have src, the object that isn't moving, and O, the object that is moving.
2. tmp is useless for local variables. It marks an instance variable as one that should not be saved automatically by datum.Write(savefile)'s default action.
3. You can define proc parameters with a type like any other variable, so casting U to mob/User in TradeGoods is unnecessary (but casting O to M in Bump is necessary).
4. It's possible that TargetTrade is null, not UserTrade, causing the runtime error in the definition of TargetRoll. (If you enable debugging in your project's Build Preferences, the error will point to the file and line number that the error is on, just like compiler errors.) For example, since the trade skill appears to only be given to players in Login(), NPCs wouldn't have it.

5. UserTrade is unnecessary; it's the same as src.
First, this is not good:

mob/var/skills[0]

That initializes the skills list, in a hidden init proc, for every mob--whether they need it or not. Define it as mob/var/list/skills, and don't actually initialize the list until you need it.

This looks like a possible design flaw to me:

mob/Login()
var/tmp/skill/trading/TSkill = new /skill/trading()
TSkill.rank = 5
src.skills["Trade"] = TSkill
return ..()

Chances are your skill datum does not need to specify the individual types. Do you really need a /skill/trading when /skill is likely more than enough to suffice? The trading proc you put under that should not be a skill proc; it should be a mob proc, because it's the mob who does the trading. The skill info of both parties is really secondary to that whole process.

This is an important thing when you're starting to use datums: learning what data and procs should go where. Skills are basically just a collection of data, keeping track of their individual level and experience; the use of them is the mob's domain. Procs that make use of skills should belong to mobs, not to the skills.

If you set the rank in New() as an optional parameter, then you can make this code a lot simpler:

mob/Login()
..()
if(!skills) skills = new // initialize the skills list here
skills["Trade"] = new/skill(5)

For your trade and Bump() procs, there are a few clear issues. The Bump() of course should not have usr in it, because it's a proc.

The trade proc really should belong to the mob who's initiating the trade, and in it when you check for skills["Trade"], you need to account for places where that's null--one of the mobs has no trade skill defined. Should that case mean that no trade occurs? Or should the skill be treated as level 1 by default? Those are questions only you can answer, but you need to choose so that you can handle this case.

Here's another reason the trading should be a mob proc: What if you had multiple skills play a factor? For instance, the prices an NPC offers you might be higher if you have a lot of visible wealth, lower if you have a high trade skill, lower if you have a good rapport with them, lower if you have a high persuasion skill (maybe not as much as by the trade skill), higher if you have high marks for rudeness, etc. Many kinds of actions might be governed not by one skill, but by a combination of several.
Thank you everyone who replied! It has helped me learn about datums and some core fundamentals that I had not yet realized. You were right I wasn't thinking and should have used mob/New() for this experiment. I now know about debugging, list initialization, defining type within parameters, datum type declaration necessity, tmp usage, checking for null beforehand. LOTS of stuff thank you both!

On a note the concept for this experiment wasn't for an actual trading skill and the procedure inside was just to figure out if I could even have a procedure inside of a datum and to test the limits of it. Also was mostly to see if I could stop doing the below style of information storage.
Code from before learning about associations & datums
        textswitch[6]//debug,combat,armor,skill detail
eqslot[6]
combatstat[8]
attributes = new /list(3,3)//str,dex,int|current,exp,curve
vitals = new /list(6,2)//health,end,mana,hunger,thirst,oxygen|current,max
skills = new /list(40,4)//Skills1-40|current, exp, curve,name


small follow up question to lummox
    skills["Trade"] = new/skill(5)

How would this know that the 5 is exp and not rank or label because all 3 are variables inside the skill datum? Also would putting skills[0] under mob/player be as good because I plan to eventually initialize the list for all players anyhow or would it be better to do right before it's first use?
In response to Zamargo
Zamargo wrote:
How would this know that the 5 is exp and not rank or label because all 3 are variables inside the skill datum?
Well, doing that won't actually do anything unless you include a constructor definition like so:
skill
New(Rank)
rank = Rank


Also would putting skills[0] under mob/player be as good because I plan to eventually initialize the list for all players anyhow or would it be better to do right before it's first use?
I would just initialize it immediately. There's no harm in invoking the hidden init proc in general, you're not saving much memory by saving it for later, and it's a waste of CPU to always check if it exists before using it.
Thanks for clarifying Kaiochao. I am totally newbie so far as general coding practice so sometimes explanations need embellishment for me to completely understand I just sit and stare at it like the atmospherics room in space station 13 until it finally clicks. Learned lots this morning! :D
In response to Kaiochao
Kaiochao wrote:
Zamargo wrote:
Also would putting skills[0] under mob/player be as good because I plan to eventually initialize the list for all players anyhow or would it be better to do right before it's first use?

I would just initialize it immediately. There's no harm in invoking the hidden init proc in general, you're not saving much memory by saving it for later, and it's a waste of CPU to always check if it exists before using it.

Whoa now! That is very wrong.

First of all, your analysis about the CPU usage of the list check is simply not true. The check to see whether it's null takes very little time, and unless this is happening in a very very tight, time-sensitive loop--which usually is not the case--it's simply the better way to go. While it's true that an init proc will only have an impact the one time, it's the impact of a full proc call--much more overhead than a couple of instructions within another proc--and those checks are very spread-out, having nearly zero impact on game performance. And if nothing else, a sanity check is pretty much always a good idea, except in the aforementioned high-performance-requirements code.

And init procs have a tendency to add up on you fast, especially at startup. If you have a lot of a certain kind of object, and that object type has an init proc, you're calling that proc once for every instance of that object. So if you define a list, or datum, under the generic /mob type, then every single mob in the world--NPCs, monsters, and so on--has the init proc called. This is very bad. It gets far worse for objs, and worse still for turfs.

The memory concern is also not trivial. If you'll only ever have about a dozen of a given kind of object, sure, giving it a single initialized list is not going to impact memory or startup time in any meaningful way. But just last night I went through a user's code who had literally dozens of mob vars that were initialized lists, and his game had a huge, huge problem with list memory. All of those vars were attached to generic mobs. He also had a list--just one--for all objs, which as you can imagine would add up very fast in any big game. If 99% of objs will never need a list, it should not be initialized for them.

In short, initializing list vars "at compile-time" is very unwise. In small numbers they're not a problem, but if you get in the habit of doing that, they have a tendency to multiply on you--and to migrate into common types like /obj and /turf where they become vastly worse for the game. Checking for the existence of a list and creating on the fly as needed is not only a good sanity check, but it means you don't have to do those costly init-proc initializations and you don't have to have 10 or 100 times as many lists (or more) than you actually need.
I like to make a Init() proc or use New() to initialize all the lists and such there as to avoid using checks altogether.
In response to Kozuma3
Checks are still good policy for the most part, but if a list can be counted on to exist, their usefulness is indeed limited. But it's crucial that lists not be given out to practically every object that exists.
Just a programming practice rant below!

I actually get what both of you are saying, but wouldn't it be easier to create a new "static" type for anything that didn't require list initialization? For instance all the mobs in my game from the monsters, shopkeeps, players and npc town fluffers all have at least one skill. Don't I have to initialize them all as they are created anyhow, and if I do isn't initializing the list for all mob types in the New() procedure the same as initializing it within the declaration of the variable?

Also if the NPC town fluffers didn't need it couldn't I just make a separate type for them without the list variable all together rather then checking for the existence of the list on all mobs?

Or am I wrong and checking for the list on New() spreads out the initialization more even if they all have it because New() will hit as it distributes the mobs instead of all at startup?

In all I suppose the question is New() proc going to wind up any different then object type variable declaration for the game?

Is it sometimes better to individualize the variables as opposed to making a list? Let's say turf has walkthrough speed,farmable ect, mobs all have skills, object all have usable,weight,value,pushable... is it better to have those as individual values rather than everything having a list?
In response to Zamargo
Zamargo wrote:
Just a programming practice rant below!

I actually get what both of you are saying, but wouldn't it be easier to create a new "static" type for anything that didn't require list initialization? For instance all the mobs in my game from the monsters, shopkeeps, players and npc town fluffers all have at least one skill. Don't I have to initialize them all as they are created anyhow, and if I do isn't initializing the list for all mob types in the New() procedure the same as initializing it within the declaration of the variable?

If all mobs have at least one skill, it does make sense to initialize the list for all of them.

However, if you have a New() proc anyway, it's better to initialize the list in your New() proc instead of in the var, so the engine calls only one proc instead of two. Basically the same instructions are executed either way, but initializing in the var definition gives you extra proc call overhead.

Also if the NPC town fluffers didn't need it couldn't I just make a separate type for them without the list variable all together rather then checking for the existence of the list on all mobs?

Only if the var was defined under a type the NPCs didn't share, like /mob/player.

Is it sometimes better to individualize the variables as opposed to making a list? Let's say turf has walkthrough speed,farmable ect, mobs all have skills, object all have usable,weight,value,pushable... is it better to have those as individual values rather than everything having a list?

This depends on the complexity of your skill system. If all mobs have only a few base stats (e.g., Strength, Dexterity, Constitution, Intelligence, Wisdom, Charisma), then individual vars might make the most sense. If they don't all have these stats, or if you have a great many such stats, a list is probably the easier way to go.

One interesting point worth discussing here: Even though all mobs have skills, they needn't all be datums. For instance an NPC can't level up, so they don't need to track their experience, only their skill level. If their skill was set to a number instead of a datum, you'd save some overhead. The only downside to this is you'd have to check if a skill was a datum or a number before using it.
Ahh! Makes sense! Thanks for discussing it in detail, I want to get used to the most efficient methods so I don't limit my expansion or create wormholes that need to be fixed later. I will probably make an list with associations for npc/monsters then and leave the datums just for players then. I have started initializing lists in the New() procedure when I can depend on them being populated. Every little bit counts when you don't know how large it might get.