簡易ぷよぷよを作ってみました

フォーラム(掲示板)ルール
フォーラム(掲示板)ルールはこちら　　※コードを貼り付ける場合は [code] と [/code] で囲って下さい。詳しくはこちら
閉鎖
elle
記事: 39
登録日時: 10年前

簡易ぷよぷよを作ってみました

#1

投稿記事 by elle » 9年前

練習として、ぷよぷよ風ゲームを作ってみました。
（音もエフェクトも何もないですが…）
ぷよぷよ風ゲーム.zip
(1.26 MiB) ダウンロード数: 186 回
気になった点や、「ここが改善できる」という点をご指摘ください。
設計の話から、push_backをemplace_backにするというような点まで何でも構いません。
個人的にはGame::BlockDrop()関数の前半の処理がやや醜く感じるのと、Draw()で現在のぷよとボードのぷよのインデックスの指定がややこしくなってしまっているのが気がかりです。

ソースは添付ファイルにも同梱しましたが、こちらにも載せます。
► スポイラーを表示
ページトップ
アバター
nullptr
記事: 239
登録日時: 12年前

Re: 簡易ぷよぷよを作ってみました

#2

投稿記事 by nullptr » 9年前

実行してませんのであしからず。ダウンロードすらしてません。それらをしないとわからない改善点は私は今は述べません。

ソースコードを流し見して。
別に、この程度のゲームであれば、このままでもいいと思ったのは事実です。
ただし。実際はもっと処理を抽象化して分散できる。Gameというシングルトンの枠に全て詰め込むのであれば、Cで書くのと何も変わらない。
  • なぜファイルを分割しないのか。
  • なぜ入力もゲームもシングルトンなのか。

  • コード: 

    enum{
        KEYMAX = 256,
    };
    はstatic const uint KEYMAX = 256ではだめなのか。
    C++11を使うならenumハックは不要なはず。

  • コード: 

        enum{
        INTERVAL_FRAME = 400,
    };
    これに至ってはenumを使う必要性がわからない。

  • コード: 

    enum class GameFlag{
        Normal,
        BlockDelete,
        FieldDrop,
    };
 
    enum BoardState{
        Board_Wall = -1,
        Board_Empty = 0,
    };
    なぜ前者はenum classで後者はenumなのか

  • コード: 

    enum{
    PIECEX = 32,
    PIECEY = 32,
    FIELDX = 8, // 実際のボード
    FIELDY = 14,
    BOARDX = 6, // 目に見えるボード
    BOARDY = 12, 
};

    コード: 

    enum{
        DXLIB_ERROR = -1,         // DXライブラリでエラーが発生したときの戻り値
        APPEAR_X = 3,             // ブロックが降りてくるX座標
        BLOCK_DOWN_FRAME = 30,    // 無操作でブロックが下りてくる間隔
    };
    名前空間を汚染するenumを使うのは避けるべきである。enum classを知っているのに使わないのは何故か。

  • コード: 

    //---------------------------------
// 隣接したものを消去する
void Game::EraseAdjoin(const int x, const int y)
{
    this->CountAdjoinImpl(mField, x, y, nullptr);
}
    this->を使うのか使わないのか統一すべき。
  • 同様に、ブロック文を使う時と使わない時がある。一行の時だけ省略するのかと思ったら、一行でもつけているところがある。統一すべき。さらに言えば一行でも改行を入れるのであればかえって読みづらい。ブロック文を書くのと書かないのでは挙動が根本的に違う。個人的には必ず書くことを奨める。

  • コード: 

    #define SHOULD_NOT_REACH_HERE assert(false && "should not reach here")
#define DISALLOW_COPY_AND_MOVE(className) \
    className(const className&) = delete; \
    className(const className&&) = delete; \
    className& operator=(const className&) = delete; \
    className& operator=(const className&&) = delete;
    マクロは使うべきではない。リフレクションの代替に目を瞑るとしても、これらは頂けない。更に、使うなら使うで使い終わったらundefすべき。例えそこで今回のコードが終わるとしても、必ず書く癖をつけるべき。それと、例外を使うのにassertをそこで使う必要はあるのか。
詳しくは処理を読んでないですけど、目についたのはこの辺。根本的に設計を変えるべきかは私は判断しないですが、C++で書く必要はないと思った。
　
　
✜ で Ｃ ご ✜
： す ＋ 注 ：
￤ か ＋ 文 ￤
： ? Is the は ：
✜ order C++?　✜
　　　　　糸冬　　　
　　――――――――
　　制作・著作 ＮＨＫ
　
　
ページトップ
elle
記事: 39
登録日時: 10年前

Re: 簡易ぷよぷよを作ってみました

#3

投稿記事 by elle » 9年前

>新ゝ月さん
返信ありがとうございます。
ここまで詳しく意見をいただけるとは思っていませんでした。
それぞれについての私の考えは以下です。
新ゝ月 さんが書きました: なぜファイルを分割しないのか。
これはすいません、追加追加でそのままになっていて、分割するのを見過ごしていました。
新ゝ月 さんが書きました: なぜ入力もゲームもシングルトンなのか。
どちらもゲーム内で必ずただ一つ存在するものという意図でしましたが…ゲームはシングルトンでない方がいいでしょうか？
新ゝ月 さんが書きました:

コード: 

enum{
        KEYMAX = 256,
    };
はstatic const uint KEYMAX = 256ではだめなのか。
C++11を使うならenumハックは不要なはず。

コード: 

    enum{
        INTERVAL_FRAME = 400,
    };
これに至ってはenumを使う必要性がわからない。
どちらもタイプの手間を減らし、インデントされることで定数である意図をはっきりさせたつもりですが、たしかに逆に意図が不明瞭になってしまったかもしれません。
新ゝ月 さんが書きました:

コード: 

enum class GameFlag{
        Normal,
        BlockDelete,
        FieldDrop,
    };
 
    enum BoardState{
        Board_Wall = -1,
        Board_Empty = 0,
    };
なぜ前者はenum classで後者はenumなのか
一応当初の意図としては、後者は比較で直接数値として扱いたいのでenumにしていましたが、
よく見ると if(~~) が if(~~ != Board::Empty)になるだけでほとんど問題ないですね。
if文で条件式を書かないやり方(if(a)のような)にこだわりすぎていました。
新ゝ月 さんが書きました:

コード: 

enum{
    PIECEX = 32,
    PIECEY = 32,
    FIELDX = 8, // 実際のボード
    FIELDY = 14,
    BOARDX = 6, // 目に見えるボード
    BOARDY = 12, 
};

コード: 

enum{
        DXLIB_ERROR = -1,         // DXライブラリでエラーが発生したときの戻り値
        APPEAR_X = 3,             // ブロックが降りてくるX座標
        BLOCK_DOWN_FRAME = 30,    // 無操作でブロックが下りてくる間隔
    };
名前空間を汚染するenumを使うのは避けるべきである。enum classを知っているのに使わないのは何故か。
これも、数値として使用するためでしたが、上の方は無名名前空間で囲うべきでした（というより今回の場合WinMain関数と#include以外囲うべきですね）。
下の方は、こちらも数値として比較に使うのでenumとして持っていました。enum classにするとキャストが必要になり冗長になると思っていましたが、それでもenum classにするべきでしょうか？
（static const int としてもスコープは同じクラスなので。一応、enumが不自然であればstatic const int で統一しようと思います）
新ゝ月 さんが書きました:

コード: 

//---------------------------------
// 隣接したものを消去する
void Game::EraseAdjoin(const int x, const int y)
{
    this->CountAdjoinImpl(mField, x, y, nullptr);
}
this->を使うのか使わないのか統一すべき。
同様に、ブロック文を使う時と使わない時がある。一行の時だけ省略するのかと思ったら、一行でもつけているところがある。統一すべき。さらに言えば一行でも改行を入れるのであればかえって読みづらい。ブロック文を書くのと書かないのでは挙動が根本的に違う。個人的には必ず書くことを奨める。
this->は関数は使用して変数は使用しないことで統一しました。
ブロック文は、基本的には1行の時のみ省略し、for文の中のif文は書くようにしていましたが…ルールが複雑ですね。書くようにしたいと思います。
新ゝ月 さんが書きました:

コード: 

#define SHOULD_NOT_REACH_HERE assert(false && "should not reach here")
#define DISALLOW_COPY_AND_MOVE(className) \
    className(const className&) = delete; \
    className(const className&&) = delete; \
    className& operator=(const className&) = delete; \
    className& operator=(const className&&) = delete;
マクロは使うべきではない。リフレクションの代替に目を瞑るとしても、これらは頂けない。更に、使うなら使うで使い終わったらundefすべき。例えそこで今回のコードが終わるとしても、必ず書く癖をつけるべき。それと、例外を使うのにassertをそこで使う必要はあるのか。
この2つのマクロは、ゲーム関連の大規模なプロジェクトで見かけて便利だと思ったので使ってみましたが…そうですね、少なくともundefはするようにしたいと思います。
例外とassertの混在は、例外の方はデバッグ時もリリース時も原因を通知してプログラムを終了させ、assertはデバッグ中は終了させるがリリース時は無視するという風にしたかったからです。
画像が見つからないときにabortの画面でfileやlineが表示されてもユーザーは困るだろうと思ったので…。

一通りリファクタリングはしたつもりでしたが、やはり1人で書いていると結構抜けがありますね…。ご指摘感謝です。

1点お聞きしたいのですが、
新ゝ月 さんが書きました: ただし。実際はもっと処理を抽象化して分散できる。Gameというシングルトンの枠に全て詰め込むのであれば、Cで書くのと何も変わらない。
について、自分でも考えてみましたが、GameFlagの設定をセッターとして関数化するくらいしか思いつきませんでした。
どのような個所が抽象化できるでしょうか。
ページトップ
アバター
nullptr
記事: 239
登録日時: 12年前

Re: 簡易ぷよぷよを作ってみました

#4

投稿記事 by nullptr » 9年前

勉強熱心で素晴らしいですね。見習いたいものです。
これはすいません、追加追加でそのままになっていて、分割するのを見過ごしていました。
気持ちはわかりますが、このコードの再利用性は0です。メンテナンスも大変になります。この程度のプロジェクトでも、分ける癖をつけましょう。
どちらもゲーム内で必ずただ一つ存在するものという意図でしましたが…ゲームはシングルトンでない方がいいでしょうか？
確かに、シングルトンと言えなくもないように感じます。しかし、実際はシングルトンは基本使ってはいけない。なぜかといえば、再利用性も拡張性も無だから。シングルトンを適用すべきは、OSレベル等自分のプログラムの更に下の階層にあるシステムが｢ひとつでなければならない｣｢いちどだけしかしてはならない｣などとしている場合だけだと思うべきです。

少しむずかしい話になりますが、キーボードそのものが本当にひとつだけである必要はありますか？
例えばGameの中で1つ作成して使えばよいのではないですか？もし複数のキーボード入力を受け付けるゲームに変えたらどうなりますか？
キーボードという概念自体は世界で唯一ではありません。もしあなたがキーボードを1つに固定するのなら、キーボードのインスタンスをゲームで1つ作り、それにアクセスすればいい話です。1つしか創れないという制約を設ける必要はないはずです。

ゲームも同じです。例えば、一度ゲームを終了して再起動する処理を実装しようとしたら、あなたはどうしますか？また、ゲームに少し機能を追加しただけのものを作る時、シングルトンでは継承もできません。シングルトンは設計として例外中の例外です。

どちらもタイプの手間を減らし、インデントされることで定数である意図をはっきりさせたつもりですが、たしかに逆に意図が不明瞭になってしまったかもしれません。

コード: 

enum{
        KEYMAX = 256,
    };
static const int KEYMAX = 256;
タイプの手間が減りますか？そうとは思えませんが。
列挙子を使うのは、複数の定数に同じ型を与えるためです。KEYMAXなどの定数に使う必要はありません。というか、enumは使うべきではありません。

数値と比較するためと言いましたね。キャストが手間だとも。その考えは直ちに捨てて下さい。もしそれでいいと想っているなら一生Cを使う事をおすすめします。
何の為に型があり、何のために数値と比較できないようになっているのか。それはバグを生みそれを治す手間を省くためです。

列挙子を使うなとは言いません。むしろボードの状態を列挙子で扱うのは自然な考えです。問題なのはそれと｢数値を比較する状況が生まれること｣なのです。
列挙子は定数の列挙として使うのではありません。複数の定数に同じ型を与えるために使うのです。
そもそも、同じ型同士でしか比較する状況が生まれないのが正しい使い方をした場合です。
（static const int としてもスコープは同じクラスなので。一応、enumが不自然であればstatic const int で統一しようと思います）
いいえ、スコープは同じではありません。無名の名前空間も列挙子も、内部では一意な名前を持っています。列挙子は型も違います。全く別物です。

更にenumは、enum名を省略して書けてしまうため、まるで同じ空間に属しているかのように扱えます。だから汚染だと言っているのです。別のところにあるのに、同じ名前でアクセスできる識別子が存在する可能性が生まれます。それは深刻なバグになるでしょうね。

列挙子を使うならenum class、定数を宣言するだけならconst、これがいずれ大きなバグを防ぐでしょう。
this->は関数は使用して変数は使用しないことで統一しました。
ブロック文は、基本的には1行の時のみ省略し、for文の中のif文は書くようにしていましたが…ルールが複雑ですね。書くようにしたいと思います。
thisとメンバ変数の命名規則については、私は言及しません。それで良いと思います。
ブロック文は一行の時に省略する慣習はあります。しかしそれは｢一行の場合は大抵処理が短く、1つであるから｣なのです。短く1つの処理ならば、スコープは関係ないからです。
しかし実際はブロックをつける場合はスコープが生成され、そうでなければ生成されません。これは意味が違うんです。生成されないことで生まれるメリットはたった二文字を省略できることだけです。
私は必ず書くことを勧めます。

私などは、switchですらブロックを必ず書きます。

コード: 

switch(a){
    case 1: {
    }
    case 2: {
    }
    default: {
    }
}
この2つのマクロは、ゲーム関連の大規模なプロジェクトで見かけて便利だと思ったので使ってみましたが…そうですね、少なくともundefはするようにしたいと思います。
例外とassertの混在は、例外の方はデバッグ時もリリース時も原因を通知してプログラムを終了させ、assertはデバッグ中は終了させるがリリース時は無視するという風にしたかったからです。
画像が見つからないときにabortの画面でfileやlineが表示されてもユーザーは困るだろうと思ったので…。
そのゲームプロジェクトはよほど非効率に運用されたでしょうね。
そうですか。まぁそれはそれで良いと思います。私は実行時アサートは一切仕込まないのですが、それも1つの手段です。
1点お聞きしたいのですが、

新ゝ月 さんが書きました:ただし。実際はもっと処理を抽象化して分散できる。Gameというシングルトンの枠に全て詰め込むのであれば、Cで書くのと何も変わらない。


について、自分でも考えてみましたが、GameFlagの設定をセッターとして関数化するくらいしか思いつきませんでした。
どのような個所が抽象化できるでしょうか
逆に、なぜどこも分散しなかったのか謎です。
全てゲームに詰め込んでいますね。これでは1つ修正する度にゲーム全てを作り直すことになります。

そもそも

コード: 

    void LoadImg();                             // 画像読み込み
    void InitParams();                          // 変数初期化
    void NewBlock();                            // 新しいブロックを降らせる
    void KeyProcess();                          // キー入力処理
    void BlockMove(const int dir);              // ブロックを左右に動かす
    void Rotate();                              // 回転処理
    void BlockDown();                           // ブロックを落とす
    void BlockDrop();                           // ブロックを一気に落とす
    void EraseAll();                            // 4つ以上つながったブロックを全て消去する
    std::list<Block> EnumAdjoin();              // 消去すべきものの座標の一覧を返す
    int CountAdjoin(const int x, const int y);  // 消去判定
    void CountAdjoinImpl(int fieldBuf[][FIELDX], const int x, const int y, uint* countBuf); // 再帰的に隣接同色ブロックを数える
    void EraseAdjoin(const int x, const int y); // 隣接したものを消去する
    void FieldDrop();                           // 全体を落下させる
    void Draw() const;                          // 描画処理
このクラスは何のためのクラスですか？｢画像を読み込んで、キーボード確認して、ブロックを操作して、描画したりしてゲームを実行する｣でしょうか。実に具体的すぎるとおもいませんか？このクラスだけで全てが完結しています。これならCで書いたコードと変わらないというものです。Cで書いたコードをクラスに全部ブチ込んだだけですね。

例えば描画処理をちゃんと関数にわけたのはいい考えではあります。しかしそれならCでもできますね？

私はぷよぷよをプレイしたことがないのでその辺の関係性はなにかおかしいかもしれませんが、一例としてコードをあげます。

コード: 

#include <vector>

enum class Color
{
    // colors
};

struct Block
{
};

class Board
{
private:
    std::vector<Block*> data;

public:

};

class PuyoPuyo
{
private:
    const Keyboard& keyboard;
    Board board;

public:
    PuyoPuyo( const Keyboard& );
    void process();
    const Board& getBoard() const;
};

class Keyboard
{
};

class Game
{
private:
    Keyboard keyboard;
    PuyoPuyo* puyopuyo;

public:
    Game()
        : keyboard{}
        , puyopuyo{ new PuyoPuyo{ this->keyboard } }
    {
    }
    void process()
    {
    }
    void draw()
    {
        this->puyopuyo.getBoard(); // ←この情報を元に描画する これでPuyoPuyoそのものから描画という処理を分離できる
        // つまりPuyoPuyoを変更しなくても描画だけ変えることもできるようになる。ファイル分割していれば再コンパイルする量が減る。
    }
};
いちから作るのは面倒なので、とりあえず描画処理を別に切り分ける案です。一番コンパイル部分が長いゲーム本体を再コンパイルする必要もなく、描画ができます。情報からぷよを表示するクラスすら用意すべきです。もし複数タイプの描画スタイルを用意しても、ポリモーフィズムをすることで継承したクラスをそのまま扱えばいいのです。

一番単純な例を上げましたが、例えばボードのサイズを変更できるようにしたりするときに、部品化を進めておけば拡張が用意になります。
PuyoPuyoを継承して、倍速で実行するハイエンドぷよぷよクラス(class HighPuyo)を作ったとします。そうしたら

コード: 

        , puyopuyo{ new PuyoPuyo{ this->keyboard } }
を

コード: 

        , puyopuyo{ new HightPuyo{ /*args*/ } }
にすればよいだけなのです。或いは後から

コード: 

//Cキーを押したら～とかいうタイミングで
delete this->puyopuyo;
this->puyopuyo = new HightPuyo{ /*args*/ }
とすれば実行時に切り替えることもできます。

まだまだ分割できます。部品化すればこれを発展させたもっと面白いゲームが簡単に作れるかもしれませんよ。
　
　
✜ で Ｃ ご ✜
： す ＋ 注 ：
￤ か ＋ 文 ￤
： ? Is the は ：
✜ order C++?　✜
　　　　　糸冬　　　
　　――――――――
　　制作・著作 ＮＨＫ
　
　
ページトップ
アバター
へにっくす
記事: 634
登録日時: 11年前
住所: 東京都

Re: 簡易ぷよぷよを作ってみました

#5

投稿記事 by へにっくす » 9年前

新ゝ月さんがコードの指摘を行ってますので
私は実行してみた感想を。

・キーの配置について
AとDが左右なのはいいが、なんでSが下？と思いました。
個人的にはZかXかなーと。
また回転が右回転のJキーしかないのもどうかな・・・
左回転がほしいです。
Kが左回転かと思ったら自由落下なのもちょっと驚きました。
カスタマイズできるようにするとベストかもしれない。

・起動していきなり始まるのはどうか。
通常のゲームって、まず「トップ」画面があって、ユーザーが開始するものです。
起動していきなり始まってるのはよくないと思います。

・ESCキーを押すと、ゲームの途中でも抜けてしまう。
終了するときは必ず、「終了しますか？」と必ずユーザーに確認するものです。

あとは、スコアをつけることですかね。
スコアを表示できるようになったら、今度は順位を表示するとか。

以上がどんなゲームでも必ずあると思われる点です。

まずは新ゝ月さんが述べているコードの改善点を念頭に置いて、
私の述べる改善をしていってはどうでしょうか？
がんばってください。
written by へにっくす
ページトップ
elle
記事: 39
登録日時: 10年前

Re: 簡易ぷよぷよを作ってみました

#6

投稿記事 by elle » 9年前

＞新ゝ月さん
ありがとうございます。
どの点も納得しました。
特に最後のコード例のような書き方を実はほとんどしたことがなく、
クラスの機能やSTLを勉強してC++を勉強した気になっていましたが、オブジェクト指向の考え方が身についていませんでした。
まだCの考え方が染みついていたようです。
ちょっと消化不良気味なのでもう少し概念を調べてコードを書き直したいと思います。

＞へにっくすさん
ご意見ありがとうございます。
キーの配置は、一般的な、WSADが上下左右に対応する形にしました。
Xが下というのは慣れない人が多いのでは…。
逆回転、タイトル画面と終了確認については、入れてみたいと思います。


一通り疑問が解けたので解決としますが、後にコードをアップデートすると思うので、その時はよろしくお願いします。
ページトップ
閉鎖

“C言語何でも質問掲示板” へ戻る