まだギリギリ3週間に1回のペースの須藤です。このシリーズのファンができました。
リーダブルなコードを目指して：コードへのコメント（3）の続きです。前回はユーザーからの入力処理のところを読んでコメントしました。
リポジトリー： https://github.com/yu-chan/Mario
今回のコメントに関するやりとりをするissue： https://github.com/yu-chan/Mario/issues/4
フレームレート
今回はメインループ中のフレームレート関連の処理を見ていきましょう。
まずはメインループの中をおさらいします。
while(!InputInterface::isOn(KEY_INPUT_Q)) { //Qを押したら終了
if(ProcessMessage() != 0) {
break;
}
InputInterface::updateKey();
Framerate::instance()->update();
ClearDrawScreen();
//ゲーム開始
Sequence::Parent::instance()->update();
ScreenFlip();
Framerate::instance()->wait();
}
この中の以下の部分がフレームレート関連の処理のはずです。
Framerate::instance()->update();
Framerate::instance()->wait();
なぜかというとFramerateクラスに属しているからです。
Framerate.hは次のようになっています。
#ifndef INCLUDED_FRAMERATE_H
#define INCLUDED_FRAMERATE_H
class Framerate {
public:
static void create();
static void destroy();
static Framerate* instance();
void update();
void wait();
int cnt() const;
private:
Framerate();
~Framerate();
static Framerate* mInstance;
int mStartTime;
int mCnt;
float mFramerate;
};
#endif
以下の部分はシングルトンパターンを実現するためのコードなので今回は無視しましょう。
static void create();
static void destroy();
static Framerate* instance();
Framerate();
~Framerate();
static Framerate* mInstance;
ということで注目するのは以下の部分です。
class Framerate {
public:
void update();
void wait();
int cnt() const;
private:
int mStartTime;
int mCnt;
float mFramerate;
};
名前から想像するとそれぞれのメンバー関数が実現する機能は次の通りです。
-
update()-
環境に合わせたフレームレートを変える。
-
マシンが重かったらフレーム減らすとか。
-
-
wait()- 現在のフレームレートに合わせたフレーム数にするために速すぎたら少し休む。
-
cnt()- 単位時間内でいまのところ何フレーム表示したかを返す。
うーん、update()はメインループ中で毎回呼ばれているんですが、そこでフレームレートを毎回調整するとは思えないんですよねぇ。実装を見てみましょう。
//フレームを更新
void Framerate::update() {
if(mCnt == 0) {
mStartTime = GetNowCount();
}
if(mCnt == INTERVAL) {
int t = GetNowCount();
mFramerate = 1000.0f / ((t - mStartTime) / (float)INTERVAL);
mCnt = 0;
mStartTime = t;
}
mCnt++;
}
この中で初めて見るのはGetNowCount()とINTERVALです。
GetNowCount()はDXライブラリが提供している「Windowsが起動してからの経過時間をミリ秒単位で返す」関数でした。
INTERVALはCommon.hで次のように定義されていました。
//フレームレート
#define INTERVAL 60
#define FPS 60
これをふまえると次のことがわかります。
-
フレームレートの単位時間は「
Framerate::update()の呼び出し回数がINTERVAL（60）回」- 1秒とかではない。
-
フレームレートの単位時間がくる度に
mStartTimeの時刻をリセットしている。 -
Framerate::update()を呼び出す毎にmFramerateを更新している。-
が、ここで計算しているフレームレートの単位時間がわからない。変な計算式な感じ。
-
mFramerateを使っているところがなさそうなので、ここのコードは今は必要なさそう。
-
mFramerateは必要なさそうなので消した方がいいでしょう。必要ないコードがあると、読むときに「どうしてここにこんなコードがあるんだ。。。」と考えないといけなくなり、理解を妨げてしまいます。コードをバージョン管理していれば消したコードを戻すことができるので、1人で開発しているときでもバージョン管理しましょう。バージョン管理していれば安心してリーダブルなコードにするための変更を重ねていけます。実は、バージョン管理したほうがいいというのはリーダブルコードの解説にも書いています。
実装を読んでみた結果「フレームレートは更新していない（mFramerateがいらなそう）だし、フレームレートの更新というかフレームを1回進めるのが目的っぽいのでupdate()じゃない名前がよさそう」という気持ちになりました。なんていう名前がいいのかなぁ。tick()とかかなぁ。フレームを1つ進めます、というイメージ。Node.JSにはprocess.nextTick()というのがあるし。
ということで、こんな感じにするのはどうだろう。
//フレームを進める
void Framerate::tick() {
if((mCnt % INTERVAL) == 0) {
mStartTime = GetNowCount();
mCnt = 0;
}
mCnt++;
}
うーん、FramerateというかFrameの方がいいのかなぁ。
もやもやしたまま次に進みましょう。Framerate::update()です。
//フレームが早かったら、早いぶんだけ待つ
void Framerate::wait() {
int t = GetNowCount() - mStartTime;
int w = mCnt * 1000 / FPS - t;
if(w > 0) {
Sleep(w);
}
}
こちらは名前から予想していた通りの実装です。ただ、FPSの使い方がもやっとします。update()では単位時間は秒ではなかったのにここでは単位時間は1秒（FPSはFrame Per Secondだから）になっています。INTERVALとFPSを統合できないかしら。こんな感じ？
//フレームを進める
void Framerate::tick() {
if((mCnt % FPS) == 0) {
mStartTime = GetNowCount();
mCnt = 0;
} else {
int t = GetNowCount() - mStartTime;
int w = mCnt * 1000.0 / FPS - t;
if(w > 0) {
Sleep(w);
}
}
mCnt++;
}
で、メインループでは最後にtick()を呼ぶだけ。これで動かないかしら。
while(!InputInterface::isOn(KEY_INPUT_Q)) { //Qを押したら終了
if(ProcessMessage() != 0) {
break;
}
InputInterface::updateKey();
ClearDrawScreen();
//ゲーム開始
Sequence::Parent::instance()->update();
ScreenFlip();
Framerate::instance()->tick();
}
あと、変数名はこんな感じにしたいですね。
//フレームを進める
void Framerate::tick() {
if((mIndex % FPS) == 0) {
mStartTime = GetNowCount();
mIndex = 0;
} else {
int expectedElapsedTime = (1000.0 / FPS) * mIndex;
int elapsedTime = GetNowCount() - mStartTime;
int restTime = expectedElapsedTime - elapsedTime;
if(restTime > 0) {
Sleep(restTime);
}
}
mIndex++;
}
Framerate::cntは単にmCntを返しているだけでした。
int Framerate::cnt() const {
return mCnt;
}
そうだろうなぁという実装です。が、これを使っているコードはなさそうなので消したほうがよさそうに思いました。
まとめ
リーダブルコードの解説を読んで「自分が書いたコードにコメントして欲しい」という連絡があったのでコメントしています。今回はメインループ内で使っているFramerateを読んでコメントしました。次回はメインループの違う処理を読んでいきます。
「リーダブルなコードはどんなコードか」を一緒に考えていきたい人はぜひ一緒にコメントして考えていきましょう。なお、コメントするときは「悪いところ探しではない」、「自分お考えを押し付けることは大事ではない」点に注意しましょう。詳細はリーダブルなコードを目指して：コードへのコメント（1）を参照してください。
