Function is being called four times per loop when there are only two calls to it

Question

I'm very new to programming in C, and have pretty rusty programming skills overall. In order to learn C and re-orient myself with programming in general, I'm challenging myself to try and make a simple rougelike using ncurses.

I've set up a "log", which I should be able to push messages to - the most recent 10 message should be displayed. In order to test this, I've made it so each time either the player or the very simple randomly-moving mob takes a step, a log message is pushed saying "step [direction]". However, even though they each only take one step, for some reason, four messages are pushed to the log. The second-to-most-recent one is always the actual direction the character moved, and I presume one of the other two is the mob moving, but I don't know the origin of the other two. Does anyone spot anything glaring in my code that might be causing this issue? All help is appreciated, thanks!

(I believe the only major relevant sections to look at should be the main() function, pushToLog(), printLog(), and moveCreature(). That said, there is a chance the problem might be somewhere else. I'm not sure.)

#include <stdlib.h>
#include <stdio.h>
#include <ncurses.h>
#include <unistd.h>
#include <string.h>

#define up 65
#define down 66
#define right 67
#define left 68
#define quit 113

struct creature {
  int x;
  int y;
  int hp;
  int maxhp;
};

void setupMap();
struct creature setupCreature();
void moveCreature();
void pushToLog();
void printLog();

int breakFlag = FALSE;

char mapShape[15][15];
char mapFeatures[15][15];
char outputLog[10][60];

int main(int argc, char *argv[]){
  struct creature player = setupCreature(4, 4, 100, 100);
  struct creature mob = setupCreature(5, 7, 100, 100);
  setupMap();
  initscr();
  noecho();
  curs_set(FALSE);

  while(1){
    for (int i = 0; i < 15; i++){
      for (int c = 0; c < 15; c++){
        mvprintw(c, i, "%c", mapShape[i][c]);
      }
    }
    mvprintw(player.y, player.x, "%c", '@');
    mvprintw(mob.y, mob.x, "%c", 'd');
    printLog();

    int input = getch();
    moveCreature(input, &player);

    int mobDir = rand() % (68 + 1 - 65) + 65;
    moveCreature(mobDir, &mob);

    refresh();
    usleep(300);

    if (breakFlag == TRUE){
      break;
    }
  }

  endwin();
  return 0;
}


void moveCreature(int dir, struct creature *subject){
  int next;

  if (dir == up){
    next = (subject->y - 1);
    if (mapShape[subject->x][next] != '#'){
      subject->y = next;
      pushToLog("step up     ");
    }
  }
  else if (dir == down){
    next = (subject->y + 1);
    if (mapShape[subject->x][next] != '#'){
     subject->y = next;
     pushToLog("step down     ");
    }
  }
  else if (dir == right){
    next =  (subject->x + 1);
    if (mapShape[next][subject->y] != '#'){
      subject->x = next;
      pushToLog("step right     ");
    }
  }
  else if (dir == left){
    next =  (subject->x - 1);
    if (mapShape[next][subject->y] != '#'){
      subject->x = next;
      pushToLog("step left     ");
    }
  }

  else if (dir == quit){
    breakFlag = TRUE;
  }

}

void pushToLog(char string[]){
  for (int i = 10; i > 0; i--){
    strcpy(outputLog[i], outputLog[i-1]);
  }
  strcpy(outputLog[0], string);
}

void printLog(){
  for (int i = 0; i < 10; i++){
    mvprintw(28-i, 0, outputLog[i]);
  }
}


struct creature setupCreature(int x,int y,int hp,int maxhp){

  struct creature frankenstien;

  frankenstien.x = x;
  frankenstien.y = y;
  frankenstien.hp = hp;
  frankenstien.maxhp = maxhp;

  return frankenstien;
}


void setupMap(){
  for (int i = 0; i < 15; i++){
    for (int c = 0; c < 15; c++){
      mapShape[i][c] = '.';
    }
  }
  for (int i = 0; i < 15; i++){
    mapShape[0][i] = '#';
    mapShape[14][i] = '#';
    mapShape[i][0] = '#';
    mapShape[i][14] = '#';
  }
}

If you could, remove all of the code that's not directly relevant. See [mcve]. We don't need to see your full game. All we need is a short example program that doesn't work the way you expect it to. This is not only a guideline for posting to SO; narrowing down the problem area is also a good way to debug your code. — John Kugelman, Dec 07 '16 at 14:58
Also, please post the output you expect and the output you're actually getting. — John Kugelman, Dec 07 '16 at 14:59
Cursory inspection reveals to me that your printLog() is suspect. It prints the full contents of the queue without taking into consideration if a line was printed already or not. One way of dealing with this is to clear the queue at the end of printLog(). (There are, of course, many other ways of dealing with this.) — work.bin, Dec 07 '16 at 15:09

score 3 · Accepted Answer · answered Dec 07 '16 at 15:13

3

Your problem is at the input stage. You expect directional commands via the arrow keys, but those generate multiple bytes per keypress. All but one are invalid as commands.

As a secondary problem, you do not reject invalid commands. You go ahead and move the mob after each command character read, whether that command was valid or not.

The overall upshot is that when you press an arrow key, the program zips through three iterations of the main loop, one right after the other, producing log messages for one valid player move, no log messages for two invalid player moves, and log messages for each of three mob moves.

You could have detected this by logging invalid commands, or by running your program in a debugger.

answered Dec 07 '16 at 15:13

John Bollinger

160,171
8
81
157

so, in effect, each keypress generates several values, several of which are rejected, and one of which is my actual keypress, and I only move on my actual keypress. However, the mob still moves even when the program is looping through with those rejected values, leading to the extra movement that we see above - seems to make sense. Do you have any recommendations or tips for how to reject the invalid commands? – Hstuart Dec 07 '16 at 15:19
@Hstuart, yes, you've got it. As for validating, I'd consider using a function separate from `moveCreature()` to handle each command. It might *call* `moveCreature()` when a valid movement command is entered, or do whatever is appropriate for other commands. Its return value could convey information back to `main()`, including whether the mob gets to move. – John Bollinger Dec 07 '16 at 15:38
You can get curses to handle the keyboard escape sequences for you. – Ian Abbott Dec 07 '16 at 15:40
Note also that if you want to handle arrow keys in ncurses, then you probably ought to turn on ncurses' `keypad` option via the like-named function. That should make `getch()` return only one value for each arrow-key keypress, but those will be *different* values from the ones you get now. See `getch()`'s docs for more information. – John Bollinger Dec 07 '16 at 15:40

Function is being called four times per loop when there are only two calls to it

1 Answers1