11 Nov, 2008, David Haley wrote in the 242nd comment:
quixadhal said:
So, a few years down the road we need an if(new_new_old_new_format)?
I've actually seen this in the "real world": backup, new_backup, old_backup, new_old_backup, and so on. I mean, it does happen. :smile:
11 Nov, 2008, quixadhal wrote in the 243rd comment:
I'm spoiled. We stored our data in an SQL database, and even IT had a version number, so when you connected you first grabbed the version number, then you potentially applied schema updates to get the database updated from whatever it was to the current code version.
All done via update_db(old, new) – which itself would dispatch each incremental update routine, many of which did nothing but were there to make the auto-updating code simple. If the code was version 20, and the db was 15, you'd run 15-16, 16-17, on up to 19-20… and 16-17 might just be a NOP.
That way we didnt' need legacy code mixed into our codebase, only in the update module.
BTW: slot_lookup() is ONLY used in load_old_obj() – which is ONLY used when a "OBJOLD" section is found. I would hazard a guess that nothing shipping has such a thing, and that if you really had an old style area file, it wouldn't say OBJOLD, it would say OBJECTS and crash when the format wasn't right.
All done via update_db(old, new) – which itself would dispatch each incremental update routine, many of which did nothing but were there to make the auto-updating code simple. If the code was version 20, and the db was 15, you'd run 15-16, 16-17, on up to 19-20… and 16-17 might just be a NOP.
That way we didnt' need legacy code mixed into our codebase, only in the update module.
BTW: slot_lookup() is ONLY used in load_old_obj() – which is ONLY used when a "OBJOLD" section is found. I would hazard a guess that nothing shipping has such a thing, and that if you really had an old style area file, it wouldn't say OBJOLD, it would say OBJECTS and crash when the format wasn't right.
11 Nov, 2008, David Haley wrote in the 244th comment:
Oh, this wasn't in an enterprise environment. It was at a university research lab. Where I am now, we have something similar to you where (nearly) every change is audited automatically and recorded.
I think that SMAUG does some other stuff with slots but I don't quite remember at the moment. But IIRC it does actually use them for things other than legacy.
I think that SMAUG does some other stuff with slots but I don't quite remember at the moment. But IIRC it does actually use them for things other than legacy.
11 Nov, 2008, quixadhal wrote in the 245th comment:
Ok, here's a puzzle…. One of the special procs calls spell_poison() to poison the victim. It passes gsn_poison, which is the same as skill_lookup("poison").. namely, the array index of the skill/spell.
Why?
If the spell adds an affect, like this one does, can't the spell code itself do this lookup? Setting aside for a moment, my dislike for "magic numbers", it seems stupid to pass in a number that will never change. Is there ever a case where you want to lie and pass in some other skill number? I can see that for spells that do different things, but ROM doesn't have any.
Why?
If the spell adds an affect, like this one does, can't the spell code itself do this lookup? Setting aside for a moment, my dislike for "magic numbers", it seems stupid to pass in a number that will never change. Is there ever a case where you want to lie and pass in some other skill number? I can see that for spells that do different things, but ROM doesn't have any.
11 Nov, 2008, David Haley wrote in the 246th comment:
Well, you might have different kinds of poisons, no? Even if ROM doesn't have that now, maybe somebody was planning ahead for the possibility.
I think it actually makes sense to modularize the application of some skill effect…
I think it actually makes sense to modularize the application of some skill effect…
11 Nov, 2008, quixadhal wrote in the 247th comment:
Oh, I agree. I just think it should be a proper parameter to pass some kind of modifier/option/etc. NOT a skill number.
Shoot, if anything pass a pointer to the element in the skill table, so you can look up ALL the skill data right there.
Shoot, if anything pass a pointer to the element in the skill table, so you can look up ALL the skill data right there.
11 Nov, 2008, David Haley wrote in the 248th comment:
I'm not sure I see the difference between a "proper parameter" and a parameter whose value is the skill. I agree that you shouldn't be sending around an identifier, but the actual skill structure. Still, the skill number is a unique identifier (at least intra-session) so it's not that terrible. In some sense, a unique identifier and the data structure should be interchangeable from an API's perspective (the former being a nicer way of referring to the latter than a pointer).
11 Nov, 2008, quixadhal wrote in the 249th comment:
No, I meant passing the skill number is, itself, worthless. If you're calling spell_fireball(), you already know the skill number – which you had to lookup to find the pointer to call spell_fireball() through. If you're IN spell_fireball(), you also already know what skill you are… you're spell_fireball().
Passing an option parameter might be useful (I'm casting a wide-area, lower damage fireball, or I'm casting a tight, high damage fireball), but ROM doesn't currently do that.
In no case that I can find is the sn that's passed to the function every different from the result of skill_lookup( "foo" ), where foo is the name of the function (IE: spell_foo). *shrug*
Passing an option parameter might be useful (I'm casting a wide-area, lower damage fireball, or I'm casting a tight, high damage fireball), but ROM doesn't currently do that.
In no case that I can find is the sn that's passed to the function every different from the result of skill_lookup( "foo" ), where foo is the name of the function (IE: spell_foo). *shrug*
11 Nov, 2008, David Haley wrote in the 250th comment:
Wait – you mean that you look up the poison skill function by number, and then you pass that number to the function you got from the number? And that's the only time you call that function? In that case I'd misunderstood earlier… that's a pretty funky thing to do.
Still, having an sn passed it could be useful if you want to parametrize a skill/spell by the effect it adds. It could be that this is an improvement that just doesn't make any sense in the context of the rest of the code, though.
Still, having an sn passed it could be useful if you want to parametrize a skill/spell by the effect it adds. It could be that this is an improvement that just doesn't make any sense in the context of the rest of the code, though.
12 Nov, 2008, Davion wrote in the 251st comment:
quixadhal said:
Ok, here's a puzzle…. One of the special procs calls spell_poison() to poison the victim. It passes gsn_poison, which is the same as skill_lookup("poison").. namely, the array index of the skill/spell.
Why?
If the spell adds an affect, like this one does, can't the spell code itself do this lookup? Setting aside for a moment, my dislike for "magic numbers", it seems stupid to pass in a number that will never change. Is there ever a case where you want to lie and pass in some other skill number? I can see that for spells that do different things, but ROM doesn't have any.
Why?
If the spell adds an affect, like this one does, can't the spell code itself do this lookup? Setting aside for a moment, my dislike for "magic numbers", it seems stupid to pass in a number that will never change. Is there ever a case where you want to lie and pass in some other skill number? I can see that for spells that do different things, but ROM doesn't have any.
spell_poison() doesn't do any special lookups to get the spot in the table. I think this is just a simple way to call a spell_fun outside of the cast function. 99% of the time a spell will not have a gsn. This is most likely a special case -just- to make this call.
12 Nov, 2008, Sandi wrote in the 252nd comment:
Quix is talking about:
(*skill_table[sn].spell_fun) (sn, level, ch, vo, target);
In the spell_fun if there's an affect we have to set the "type" to the sn, so why look it up again? Granted, this is 16 bit thinking, and if someone ever goes through the area files and replaces all the numbers, 'sn' will quickly be history.
Also, now, don't snicker… using 'sn' makes it easier to make new spell functions using cut and paste. :wink:
(*skill_table[sn].spell_fun) (sn, level, ch, vo, target);
In the spell_fun if there's an affect we have to set the "type" to the sn, so why look it up again? Granted, this is 16 bit thinking, and if someone ever goes through the area files and replaces all the numbers, 'sn' will quickly be history.
Also, now, don't snicker… using 'sn' makes it easier to make new spell functions using cut and paste. :wink:
12 Nov, 2008, Davion wrote in the 253rd comment:
Sandi said:
Quix is talking about:
(*skill_table[sn].spell_fun) (sn, level, ch, vo, target);
(*skill_table[sn].spell_fun) (sn, level, ch, vo, target);
He said special proc so I assumed he meant
bool spec_poison( CHAR_DATA *ch )
{
CHAR_DATA *victim = NULL;
if ( ch->position != POS_FIGHTING
|| ( victim = ch->fighting ) == NULL || number_percent( ) > 2 * ch->level )
return false;
act( "You bite $N!", ch, NULL, victim, TO_CHAR );
act( "$n bites $N!", ch, NULL, victim, TO_NOTVICT );
act( "$n bites you!", ch, NULL, victim, TO_VICT );
spell_poison( gsn_poison, ch->level, ch, victim, TARGET_CHAR );
return true;
}
I think he's wondering why they pass a semi constant number (gsn_poison) when it could be grabbed elsewhere. I think this is another case of old technology. They're trying to cut down on lookup time by storing constants. As to why they don't use skill_lookup("Somespell"), I think there'd be a couple reasons. One would be the one I just meantioned, to cut down on cpu cycles. The other would be to catch typo's. It'd suck to pain yourself trying to find a bug when you put in skill_lookup("pioson"). Good way to catch typos/errors at compile time, rather than mysteriously finding them via runtime bugs.
12 Nov, 2008, quixadhal wrote in the 254th comment:
Well, I got my gsn-less version to work…. slight bug in the binary search code had me wondering for a while. Yes, that's right… no more grade-school linear searches for a spell/skill name. The skill table is sorted at boot time and with 134 skills it should take at most 8 steps to find (or fail to find) what you're looking for. I'll check that in after a I do a bit more cleanup.
I did find another bug that I must have introduced somewhere – I somehow don't think this is quite right….
I did find another bug that I must have introduced somewhere – I somehow don't think this is quite right….
Quote
You are Quixadhal the %s, level 1, 17 years old (0 hours).
Race: dwarf Sex: male Class: warrior
You have 20/20 hit, 100/100 mana, 100/100 movement.
You have 5 practices and 3 training sessions.
You are carrying 5/40 items with weight 14/182 pounds.
Str: 17(17) Int: 12(12) Wis: 14(14) Dex: 10(10) Con: 15(15)
You have scored 1400 exp, and have 0 gold and 0 silver coins.
You need 1400 exp to level.
Wimpy set to 0 hit points.
You are standing.
You are defenseless against piercing.
You are defenseless against bashing.
You are defenseless against slashing.
You are defenseless against magic.
You are demonic.
<20hp 100m 100mv> equip
You are using:
<used as light> a map of the city of Midgaard(Glowing) a war banner
<worn on torso> a map of the city of Midgaard(Glowing) a war bannera sub issue vest
<worn as shield> a map of the city of Midgaard(Glowing) a war bannera sub issue vesta sub issue shield
<wielded> a map of the city of Midgaard(Glowing) a war bannera sub issue vesta sub issue shielda sub issue sword
Race: dwarf Sex: male Class: warrior
You have 20/20 hit, 100/100 mana, 100/100 movement.
You have 5 practices and 3 training sessions.
You are carrying 5/40 items with weight 14/182 pounds.
Str: 17(17) Int: 12(12) Wis: 14(14) Dex: 10(10) Con: 15(15)
You have scored 1400 exp, and have 0 gold and 0 silver coins.
You need 1400 exp to level.
Wimpy set to 0 hit points.
You are standing.
You are defenseless against piercing.
You are defenseless against bashing.
You are defenseless against slashing.
You are defenseless against magic.
You are demonic.
<20hp 100m 100mv> equip
You are using:
<used as light> a map of the city of Midgaard(Glowing) a war banner
<worn on torso> a map of the city of Midgaard(Glowing) a war bannera sub issue vest
<worn as shield> a map of the city of Midgaard(Glowing) a war bannera sub issue vesta sub issue shield
<wielded> a map of the city of Midgaard(Glowing) a war bannera sub issue vesta sub issue shielda sub issue sword
12 Nov, 2008, David Haley wrote in the 255th comment:
You might want to take a look at what I did for SmaugFUSS w.r.t. the binary search stuff. There's a subtlety you have to watch out for, which is adding skills at runtime: you still need a linear section at the end to stick new skills in (because too many places refer directly to the array position of the skill) – actually, maybe if you've got it so that gsn's don't appear anywhere, you don't care about the array indices…
Did you group skills, spells, etc. into a single sorted list, or maintain one list per category?
Did you group skills, spells, etc. into a single sorted list, or maintain one list per category?
12 Nov, 2008, quixadhal wrote in the 256th comment:
Right now, spells and skill are in the same list. They were to begin with, they just had the extra gsn_ nonsense to partially hide the fact. I wouldn't mind splitting them off, but I think it would require gutting some of how the system for attatching affects works. That's a task for another day. :)
Oh, and I found ONE of the bugs I added…. on my optimization pass, I removed one buffer clearing line that I shouldn't have. The buffer is static, so it doesn't clear itself on function entry…. oops.
Oh, and I found ONE of the bugs I added…. on my optimization pass, I removed one buffer clearing line that I shouldn't have. The buffer is static, so it doesn't clear itself on function entry…. oops.
12 Nov, 2008, David Haley wrote in the 257th comment:
Actually I don't see the point of splitting them, unless you have skills and spells with the same name.
And static buffers annoy me: it reveals yet another of the many advantages of using something like std::string (or an output stream).
And static buffers annoy me: it reveals yet another of the many advantages of using something like std::string (or an output stream).
12 Nov, 2008, quixadhal wrote in the 258th comment:
Interesting! I think I found a bug that's probably due to the recycling system….
I make a character, verified that the equipment list bug is fixed and that the lack of a title is still there (Quixadhal the %s)…. quit, deleted the character file, and was able to log back in!
I'm assuming the ressurection of the (uncleared!) data structure did that. It might even have been uncleared buffers used by login.
One point against data structure recycling….
I make a character, verified that the equipment list bug is fixed and that the lack of a title is still there (Quixadhal the %s)…. quit, deleted the character file, and was able to log back in!
I'm assuming the ressurection of the (uncleared!) data structure did that. It might even have been uncleared buffers used by login.
One point against data structure recycling….
12 Nov, 2008, David Haley wrote in the 259th comment:
That's the problem when people do pretty complicated things without really knowing what they're doing: they end up making mistakes like that…
Yet again I think it is made clear that you shouldn't go crazy micro-optimizing unless (a) you know what you're doing and (b) you have very good reason to believe that it'll actually help.
Yet again I think it is made clear that you shouldn't go crazy micro-optimizing unless (a) you know what you're doing and (b) you have very good reason to believe that it'll actually help.
12 Nov, 2008, quixadhal wrote in the 260th comment:
DavidHaley said:
You might want to take a look at what I did for SmaugFUSS w.r.t. the binary search stuff. There's a subtlety you have to watch out for, which is adding skills at runtime: you still need a linear section at the end to stick new skills in (because too many places refer directly to the array position of the skill) – actually, maybe if you've got it so that gsn's don't appear anywhere, you don't care about the array indices…
Hmmmm, well, not yet an issue since we haven't put OLC in at this point. But yes, I guess it will depend on how many places squirrel away the index, rather than using a lookup function.
So, a few years down the road we need an if(new_new_old_new_format)? :)
Question: How many of you have EVER (EVER!!!) used an area file with a MOBOLD, or OBJOLD section in it? How many of you have ever used - in your area list to load from stdin?
I'm thinking the right way to do this is to have a #VERSION header and just have load_foo( fpArea, version ).