Print

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

[Ihlosi]

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.

That probably depends on finding a good way to separate game mechanics from text output. I haven't had any brilliant ideas there, mostly because my usual code doesn't involve things like text output, or even any kind of interaction with an actual human. 

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.

Okay, I need to check all of the editor settings again. I unchecked the "Use TAB character" and "TAB indents" fields in code::blocks editor settings and set "TAB size in spaces" to 3, and I was pretty sure I wouldn't be littering the code with tab characters this way.

[Ihlosi]

I think I found the relevant settings in code::blocks: "Use TAB character" needs to be unchecked, but "TAB indents" needs to be checked in order to have the editor use spaces for indentations. Sounds a bit counter-intuitive.

Yet another question:

The code often assembles output strings with repeated calls to addstr():

Code: [Select]

   addstr("Choose a Liberal squad member for Place ");
   char num[20];
   itoa(spot+1,num,10);
   addstr(num);
   addstr(".");

Is there any special reason/philosophy for this instead of generating the complete formatted output string first and then printing it via addstr?

E.g.:

Code: [Select]

   char line_buf[80];
   sprintf(line_buf, "Choose a Liberal squad member for Place %i.", (spot + 1));
   addstr(line_buf);

(This could probably rolled into one function, to avoid having a separate line_buf in hundreds of functions all over the code.)

[Jonathan S. Fox]

Code: [Select]

   addstr("Choose a Liberal squad member for Place ");
   char num[20];
   itoa(spot+1,num,10);
   addstr(num);
   addstr(".");

Is there any special reason/philosophy for this instead of generating the complete formatted output string first and then printing it via addstr?

E.g.:

Code: [Select]

   char line_buf[80];
   sprintf(line_buf, "Choose a Liberal squad member for Place %i.", (spot + 1));
   addstr(line_buf);

(This could probably rolled into one function, to avoid having a separate line_buf in hundreds of functions all over the code.)

I think this is just a quirk of how Tarn was comfortable initially coding the game; string formatting would certainly be more concise.

If you end up defining a new function to wrap the temp variable (that's what I would do), take a look at the custom overloads of addstr() and mvaddstr() at the bottom of common/commondisplay.cpp. You could decline to support std::String objects since you're just doing c-string formatting, but Log support is fairly important, and is trickier, since an overload in the style we're currently doing (optional last parameter) would conflict with the variable parameter list. In any case, I'd definitely supply a mvaddstr() variant (wraps move() and addstr() into one call) for any new addstr() variant you added.

[usr_share]

Code: [Select]

   addstr("Choose a Liberal squad member for Place ");
   char num[20];
   itoa(spot+1,num,10);
   addstr(num);
   addstr(".");

Is there any special reason/philosophy for this instead of generating the complete formatted output string first and then printing it via addstr?

E.g.:

Code: [Select]

   char line_buf[80];
   sprintf(line_buf, "Choose a Liberal squad member for Place %i.", (spot + 1));
   addstr(line_buf);

(This could probably rolled into one function, to avoid having a separate line_buf in hundreds of functions all over the code.)

I think this is just a quirk of how Tarn was comfortable initially coding the game; string formatting would certainly be more concise.

If you end up defining a new function to wrap the temp variable (that's what I would do), take a look at the custom overloads of addstr() and mvaddstr() at the bottom of common/commondisplay.cpp. You could decline to support std::String objects since you're just doing c-string formatting, but Log support is fairly important, and is trickier, since an overload in the style we're currently doing (optional last parameter) would conflict with the variable parameter list. In any case, I'd definitely supply a mvaddstr() variant (wraps move() and addstr() into one call) for any new addstr() variant you added.

I always thought that "printing and adding to a log file" should have a different function from "just printing", say laddstr() or something. Like there's printf(...) and fprintf(stderr,...) for outputting to standard and error streams. Though, as I'm more of a C programmer, I don't know how acceptable would that be in C++.

But yeah, a printf-style variation on addstr would be really nice.

[Ihlosi]

But yeah, a printf-style variation on addstr would be really nice.

Hm, I think I just realized that this might not be as easy as it sounds - functions can be declared to have a variable number of arguments, but the actual number of arguments for each call (and their type, etc) must be known at compile time. This means that you can't just hand over those arguments to another function.

A workaround might be to use some clever overloading, but that limits the new function to the given set of parameter combinations, i.e. have one printf_addstr() that takes one integer, one that takes a string, one that takes two integers, etc.

Another way to avoid each function having their own 80-char buffer for sprintf might be to make this buffer global. As long as the program is nice and single-threaded, there would not be any conflicts when accessing that buffer. I'm still not sure whether that would count as "nasty hack" or "most straightforward solution", or both. 

[Jonathan S. Fox]

Hm, I think I just realized that this might not be as easy as it sounds - functions can be declared to have a variable number of arguments, but the actual number of arguments for each call (and their type, etc) must be known at compile time. This means that you can't just hand over those arguments to another function.

While this is a huge barrier for directly wrapping the printf and sprintf functions, the wise minds providing the standard libraries anticipated exactly this problem and provided alternate versions that will accept your va_list directly rather than expecting you to decompose it into separate arguments. Try using vsprintf instead of sprintf!

[Ihlosi]

While this is a huge barrier for directly wrapping the printf and sprintf functions, the wise minds providing the standard libraries anticipated exactly this problem and provided alternate versions that will accept your va_list directly rather than expecting you to decompose it into separate arguments. Try using vsprintf instead of sprintf!

Heh. Things you never learn when your code never needs to format strings ...

So a formatted version of addstr() could look like this?

Code: [Select]

void addstrf( const char * format, ... )
{
  static char buffer[256];
  va_list args;

  va_start (args, format);
  vsnprintf (buffer,256,format, args);
  va_end (args);
  addstr(buffer);
}


[Ihlosi]

If you end up defining a new function to wrap the temp variable (that's what I would do), take a look at the custom overloads of addstr() and mvaddstr() at the bottom of common/commondisplay.cpp. You could decline to support std::String objects since you're just doing c-string formatting, but Log support is fairly important, and is trickier, since an overload in the style we're currently doing (optional last parameter) would conflict with the variable parameter list. In any case, I'd definitely supply a mvaddstr() variant (wraps move() and addstr() into one call) for any new addstr() variant you added.

Hm ... addstr() wrappers that include log support would probably need to take the log object as the first argument (or one of the first arguments) instead of the last.

However, making a custom overload of a library function that adds functionality not directly related to the library functions original task seems like a recipe for future headaches. Using a macro instead would clarify that there is additional functionality and provide a simple means of deactivating the additional functionality by changing the macro.

[Carlos Gustavos]

Ihlosi, please use unix-style line endings in your commits.

[Ihlosi]

Ihlosi, please use unix-style line endings in your commits.

Changed end-of-line in code::blocks' editor setting to "LF". That should do it, I think.

[Ihlosi]

But yeah, a printf-style variation on addstr would be really nice.

I think I'm ready to add one.

However, keeping logging support in the present format (log object as the last function argument) would be a bit of a hassle due to the variable argument list of vsnprintf. Moving the log object argument to the beginning of the list (before or after any x/y coordinates for a move()?) would make things easier.

[Ihlosi]

I added printf-style versions of addstr and mvaddstr (with and without logging).

[Jonathan S. Fox]

Just wanted to mention how convenient I'm finding the formatted addstr variants. Thanks, Ihlosi.