第４回、「シューティングゲーム (1)」

課題の説明

  1. c_shooting.tar.gz をコンパイルし、実行できることを確認せよ
  2. 本を参考にし、作りたい機能を自由に実装してみよ
  3. 作成したプログラムをコンパイルできる最小構成で圧縮し、メール添付で提出せよ

課題の説明

特になし。強いて挙げるとすれば「プログラミングしてみる」ってこと。
なので、採点しつつプログラミングのスタイルについてコメントしていきます。

提出された課題へのコメント

気になった箇所のみ、抜粋して説明していきます。

Aさんの場合

A_img.jpg

障害物をよける(自機の背景が黒いとよかったかも)

shots_ctrl.c より抜粋 
  shots[index].x =ship->x/*船のいるところから弾が来る*/;
  shots[index].y = 0;
  shots[index].xx =0 ;
  shots[index].yy = +1;
  shots[index].active = TRUE;

コメントは大変良いです。欲を言えば、他のソースコードでの記述は「自機」なので、「自機のいる X 座標に弾を配置」の方がよいでしょう。
ただ、ステートメントとセミコロンの間にコメントを入れるのは、明らかにダメです。次にやったら、おしおきです。

あと、ソースコードのスタイルを保つ、というのは非常に大切です。今回のように、元のプログラムが x = 0; の様に演算しの間にスペースを入れるスタイルであるならば、それを守るようにしましょう。ソースコード中に違和感が生じてはいけません。

shots_ctrl.c より抜粋 
/* 弾の移動 */
void move_shots(shot_info_t *shots, int max_size, const ship_info_t *ship) {
  int i;

  for (i = 0; i < max_size; ++i) {

    // !!! 弾の移動ルーチンに基づいた処理
    // !!!

    int x, y;
    x = shots[i].x + shots[i].xx;
    y = shots[i].y + shots[i].yy;
    printf("x=%d y=%d sx=%d sy=%d\n",x,y,ship->x,ship->y);
    /*当たり判定*/
    if(abs(x - ship->x) <=3 && abs(ship->y - y)<=3 ){
      printf("gameover\n");
      exit(0);
    }

    if ((x < 0) || (x > WIN_W-1)) {
      shots[i].active = FALSE;
    }
    if ((y < 0) || (y > WIN_H-1)) {
      shots[i].active = FALSE;
    }
  }
}

これは、スタイルの指摘ではなく、バグの指摘です。
shots 変数は以下のループで初期化されています。

/* 弾情報の初期化 */
void init_shots(shot_info_t* shots, int max_size) {
  int i;

  for (i = 0; i < max_size; ++i) {
    shots[i].active = FALSE;
  }
}

上記ループでは、active 変数に FALSE が代入されるようにしています。逆に言えば、active 以外のメンバ変数は、何が入っているか分からない状態です。なので、プログラムの実行環境によっては、以下のように衝突していないのに当たり判定が発生する可能性があります。

...
x=-1208399967 y=-1210973109 sx=0 sy=35
x=0 y=32 sx=0 sy=35
gameove

正しくは、active が FALSE かどうかの判定を加えて、こうです。

  for (i = 0; i < max_size; ++i) {
    if (shots[i].active == FALSE) {
      continue;
    }
  ...

って、渡したソースの方も間違ってじゃん！
今となっては、課題のつもりで未実装なのか、忘れてて未実装なのかは不明...。

気を取り直して次、shooting.c より抜粋

    /* 自機の移動 */
    get_input(&input);
    move_ship(&ship, &input);

    /* 球の移動 */
    create_shot(shots, MAX_SHOTS, &ship);
    move_shots(shots, MAX_SHOTS, &ship);



    /* 画面の更新 */
    now = get_ticks();
    if ((now - prev) > FPS) {
      delay((now - prev) - FPS);
    }

意味のない空行はダメ！ ぱっと見て分かるためにも、ぱっと見て違和感が生じるような書き方をしないようにしましょう。

くらい？
ソースの変更が少ないにも関わらず、ゲーム性が出ていてよかったと思います。


Bさんの場合

B_img.jpg

弾が撃てる

struct_defines.h より抜粋 
  /* 入力イベントの管理用 */
typedef struct {
  int quit;                     /* 終了イベント */
  int is_left;                  /* 左移動の指令中 */
  int is_right;                 /* 右移動の指令中 */
  int is_up;                    /* 上移動の指令中 */
  int is_down;                  /* 下移動の指令中 */
  int is_shot;                  // ショット
} input_t;

それまでのコメントと同じ形式にしましょう。それくらい。

/* 自機の情報 */
typedef struct {
  int x;   //自機の座標
  int y;
  int xx;  //移動量
  int yy;
} ship_info_t;

追記したコメントのようですが、いい感じのコメントです。

shooting.c より抜粋 
    /* 自機の移動 */
    move_ship(&ship, &input);

    /* 球の生成 */
      create_shot(shots, MAX_SHOTS, &ship, &input);

    /* 球の移動 */
    move_shots(shots, MAX_SHOTS, &ship);

インデントが変な感じ。あと、「球」なのは渡したソースからそうだった。「弾」の間違いです。すみません。

shots_ctrl.c より抜粋 
/* 弾の生成 */
void create_shot(shot_info_t *shots, int max_size, const ship_info_t *ship, const input_t *input) {
  static int cycle = 1000;
  int index;
  int i;

  if (++cycle < ShotCreateCycle || input->is_shot!=1) {
    return;
  }
  cycle = 0;

is_shot を数値と直接比較してはダメ。書き直したものは以下の通り。

/* 弾の生成 */
void create_shot(shot_info_t *shots, int max_size, const ship_info_t *ship, const input_t *input) {
  static int cycle = 1000;      /* 開始直後は弾を打てない */
  int index;
  int i;

  if ((++cycle < ShotCreateCycle) || (input->is_shot == FALSE)) {
    return;
  }

これだと、「弾を撃つ状態でなかったら」とすんなり読める。人によっては

  if (! input->is_shot) {
    return;
  }

を主張する人がいるかもしれないが、否定は読みづらいので、FALSE と == で比較するのでよいと思う。まぁ、どっちでもいい。

とにかく、読みやすく、曖昧性のないソースコードを書くことを目指しましょう。
以上でした。いい感じです。


Cさんの場合

C_img.jpg

降ってくる弾を避ける

まず、文字コードが UTF8 でした...。EUC か SJIS でお願いします。 あと、正直言って今までの人と同じ問題が多いので、ここではコメントすることは特にないかも。

では、せっかくなので。
Linux なら、初期化忘れの変数の検出や、メモリの解放忘れの検出に、valgrind というソフトを使ってみましょう。以下、使用例 
% make && valgrind ./shooting
make: `all' に対して行うべき事はありません。
==3127== Memcheck, a memory error detector.
==3127== Copyright (C) 2002-2006, and GNU GPL'd, by Julian Seward et al.
==3127== Using LibVEX rev 1606, a library for dynamic binary translation.
==3127== Copyright (C) 2004-2006, and GNU GPL'd, by OpenWorks LLP.
==3127== Using valgrind-3.2.0, a dynamic binary instrumentation framework.
==3127== Copyright (C) 2000-2006, and GNU GPL'd, by Julian Seward et al.
==3127== For more details, rerun with: -v
==3127==
--3127-- DWARF2 CFI reader: unhandled CFI instruction 0:50
--3127-- DWARF2 CFI reader: unhandled CFI instruction 0:50
--3127-- DWARF2 CFI reader: unhandled CFI instruction 0:50
--3127-- DWARF2 CFI reader: unhandled CFI instruction 0:50
==3127== Conditional jump or move depends on uninitialised value(s)
==3127==    at 0x80491BB: move_shots (shots_ctrl.c:92)
==3127==    by 0x804899A: main (shooting.c:53)
==3127==
==3127== Conditional jump or move depends on uninitialised value(s)
==3127==    at 0x80491C1: move_shots (shots_ctrl.c:92)
%

この結果から shots_ctrl.c の 92 行目において、初期化されてない変数が使われたことが分かります。ちなみに、92 行目付近は、こんな感じです。

    int x, y;
    x = shots[i].x + shots[i].xx;
    y = shots[i].y + shots[i].yy;
    if ((x < 0) || (x > WIN_W-1)) {
      shots[i].active = FALSE;
    }

はい。これはここまでにコメントしましたよ。
何が原因でしょうか？ ちょっと考えてみて下さい。

問題への対処して再実行すると、valgrind のエラーメッセージがなくなります。

今回は、以上です。

「第５回、「シューティングゲーム (2)」 へ」
Generated on Mon Apr 13 22:52:06 2009 by  doxygen 1.5.7.1