Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[Ihlosi]

I've been looking at The Code(tm) again. And I have a question.

In several places, most prominently in creaturenames.cpp, there are huge switch/case statements that basically map one number to a string or another number. In creaturenames.cpp, a random number is mapped to a name, for example.

Is there any special reason that I missed to use a switch/case statement in these places instead of a char ** and using the random number as an index into a string of names? As in:

Code: [Select]

char *male_first_names[] = {"Aaron", "Abraham", ...};
(Don't have a compiler right now, but I think that's the syntax for an array of variable-length char strings ...)

[Gatleos]

That would certainly clean up and condense the code. I'm guessing that particular function has been there for quite a long time and nobody's bothered cleaning it up.

I don't see any reason not to change it.

[Jonathan S. Fox]

Your suggestion is almost exactly what I did for Zombie Survival Squad's name generator. The only reason for the current system of generating creature names is that it's what is already in place.

Your suggested alternate approach has several benefits. In particular, we'd be able to more easily maintain and edit the name list (keeping it sorted, deleting, and inserting are cheaper in terms of human effort), and we can have the compiler track the size of the name list without needing magic numbers:

num_names = sizeof(names) / sizeof(names[0]);

So, if someone wanted to rewrite the name generator to clean it up, that'd be cool. It's low priority for me personally, simply because it has no immediate impact on the game.

[Ihlosi]

So, if someone wanted to rewrite the name generator to clean it up, that'd be cool. It's low priority for me personally, simply because it has no immediate impact on the game.

I just installed Code::Blocks. I'll try to get the game to compile. Unfortunately, the largest part of my programming practice and experience is writing C (plus some assembly) for microcontrollers (mostly ARM, some 8051, some DSP) and dealing with the appropriate development tools. Writing for an actual computer might have the benefit of not having to deal with emulators, "hardware in development", and debugging with an oscilloscope.

I found quite a few things that would make the code more maintainable/expandable without any impact on the game. Improvements here would be investments for the future, but have not immediate benefit.

Now if I could only tear myself away from playing the game.

[Ihlosi]

num_names = sizeof(names) / sizeof(names[0]);

This only works if the array uses fixed-length strings (which may actually be advisable due to being able to use the line above, even though it leaves some memory unused), e.g.

Code: [Select]

char male_first_names[MFN_COUNT][MAX_MFN_LENGTH] = {"Aaron", "Abraham", ...}
On the other hand,

Code: [Select]

char *male_first_names[] = {"Aaron", "Abraham", ...}
does not leave any memory unused, since it is technically an array of pointers into one long string, "Aaron\0Abraham\0...".

[Jonathan S. Fox]

num_names = sizeof(names) / sizeof(names[0]);

This only works if the array uses fixed-length strings (which may actually be advisable due to being able to use the line above, even though it leaves some memory unused), e.g.

Code: [Select]

char male_first_names[MFN_COUNT][MAX_MFN_LENGTH] = {"Aaron", "Abraham", ...}

Correct me if I'm wrong, but I don't see why that would be. sizeof(names) should be the number of bytes in a pointer times the number of names; you defined it as an array of pointers, not an array of arrays.

[G-Flex]

The size of a string, at least in this context, shouldn't have anything to do with the size of its contents anyway.

[Ihlosi]

Correct me if I'm wrong, but I don't see why that would be. sizeof(names) should be the number of bytes in a pointer times the number of names; you defined it as an array of pointers, not an array of arrays.

You're right. I got some things confused. That makes this approach even more elegant than I had thought.

[Ihlosi]

So, if someone wanted to rewrite the name generator to clean it up, that'd be cool. It's low priority for me personally, simply because it has no immediate impact on the game.

I have a changed creaturenames.cpp ready to submit. Now what do I do?

Then we just need to hunt down all the other places where strings are randomly picked from a list.

[Ihlosi]

Hm, is there a source file that holds small "utility" functions that are used frequently by all parts of the code?

My approach uses a generic "pick random string function":

Code: [Select]

const char *SelectRandomString(const char **string_table, int table_size)
{
int roll;

roll = LCSrandom(table_size);

return(string_table[roll]);
}

that picks one random string from a given array, e.g.

Code: [Select]

static const char *male_first_names[] =
{
"Aaron", "Abraham", "Adam", "Adolph", ...
        };

I placed it in creaturenames.cpp, but logically it should go in a more general place.

[Jonathan S. Fox]

I have a changed creaturenames.cpp ready to submit. Now what do I do?

Cool! Well, the easiest for me is that you could PM me with your Sourceforge username and I can add you as a developer on the project. Then you can SVN commit your changes.

Hm, is there a source file that holds small "utility" functions that are used frequently by all parts of the code?

Mmm, maybe src/common/misc.cpp? It's not hugely important; the code isn't really all that organized to begin with. Toady left everything in one file tens of thousands of lines of code long, and when I mapped out the revised file structure for breaking that monolith up, I had all of one year of programming experience, and hadn't worked on anything of this size before. It's a small miracle it's as organized as it is.

[Ihlosi]

Cool! Well, the easiest for me is that you could PM me with your Sourceforge username and I can add you as a developer on the project. Then you can SVN commit your changes.

Done. I still have to get used to SVN, though. I'm using a different (proprietary) revision control system at work.

Toady left everything in one file tens of thousands of lines of code long,

*faints*

Well, we all learn (usually the hard way) from beginners mistakes. Some of mine are over ten years old and still occasionally come back to haunt me in the form of nasty assembly blobs. I've grown wiser.

[Ihlosi]

Okay, I think I managed to commit some changes using the new selectRandomString function. Let me know what you think and then I can work on all the other occurrences.

Basically, a search for "case 0:addstr(" gives possible places where the new function might be used.

[Ihlosi]

Hm. I keep running into code parts that are almost, but not quite, about picking strings from a selection. They modify some or all of the possible strings in some way, e.g. by adding object/creature names, pronouns, or by taking one or more of the current laws (most prominently: free speech) into account.

I wonder how much coding effort it would be to write a parser for strings that contain all of the information necessary to assemble the output string in a simple markup language, like <rnd:a|b|c> for a random choice or <freespeech|[rats]|crap|crap|crap|shit> for a choice depending on the current free speech law.

[Jonathan S. Fox]

Hm. I keep running into code parts that are almost, but not quite, about picking strings from a selection. They modify some or all of the possible strings in some way, e.g. by adding object/creature names, pronouns, or by taking one or more of the current laws (most prominently: free speech) into account.

I wonder how much coding effort it would be to write a parser for strings that contain all of the information necessary to assemble the output string in a simple markup language, like <rnd:a|b|c> for a random choice or <freespeech|[rats]|crap|crap|crap|shit> for a choice depending on the current free speech law.

Well, at that point I wonder if you're really gaining in clarity by avoiding a switch statement? You'll take some of the lines of code out, but it seems to me that custom markup is significantly less self-documenting than function calls.

The changes to creaturenames.cpp look good. I'm guessing you already have your IDE set to three space tabs, but you're using tab characters -- you should switch to three space indentation with spaces instead of tabs, since this is how most of the project is built. Mixing tabs and spaces won't immediately suck in your editor, as long as your tab length is the same as the number of spaces used in the project, but as soon as somebody tries to read the code with a fancy visual diff tool that uses eight space tabs, the code readability is going to go to hell due to inconsistent whitespace.