はかせのラボ

私の頭の中を書いていく雑記ブログです

落ちモノパズル 指摘を受けた部分の修正

あいさつ

どうも、はかせです。
今回は就活の中で作品を見せたときにもらった
意見を作品に反映してみたり
その他細かい点の修正を行いました。

Componentの判定の最適化

大分初期に作ったコンポーネントシステムの根っこの部分ですね。
今まではこうでした。

template<typename T>
inline T * DXGameObject::AddComponent()
{
	T* pReturn = nullptr;
	//Componentの派生クラスでなければnullを返す
	if (!typeid(T).before(typeid(Component*))) return pReturn;
	pReturn = new T();
	//追加したコンポーネントの初期化処理を呼び出す
	pReturn->Initialize(this);
	mComponentsList.push_back(pReturn);
	return pReturn;
}

もちろんこれでもうごきます。
ただこのコードの問題点として、
①Component以外を渡してもコンパイルが通ってしまう
②typeidを使ってランタイム判定しているためコストが高い

という指摘をうけました。

①はComponent以外だったとしてもnullが返るだけでエラーにならないからですね。
意図としてはComponent以外はエラーとして処理してほしいわけです。
②はランタイム情報をあつかっているためどうしてもコストが割高になってしまう問題です。
私の場合だとランタイムでやるメリットがほぼないので
コストに見合った結果が得られていないだろうということです。

私がpragmaなどをうまくつかってコストに見合った結果が得られる書き方をしているなら別ですが
そうでないならなるべくランタイム情報を使った処理は避けた方がいいと教えてもらいました。

2つの指摘を受け直したコードが下記になります。

template<typename T>
inline T * DXGameObject::AddComponent()
{
	T* pReturn = new T();
	//Componentでなければコンパイルエラーが起こる
	Component* com = pReturn;
	//追加したコンポーネントの初期化処理を呼び出す
	pReturn->Initialize(this);
	mComponentsList.push_back(pReturn);
	return pReturn;
}

なんてことはなく
ただ基底クラスにキャストしようとしているだけです。
普通だったらここに例外処理差し込んで
プログラムが止まらないようにすると思いますが、
今回は意図しないものが入ったらコンパイルエラーにしてほしいため
あえて例外処理はいれていません。

指摘を受けたが未修整の部分

DXGameObjectとComponentのインスタンス生成の問題です。
DXGameObjectもComponent現状特定クラスがファクトリのような働きをし
(DXGameObjectはScene、ComponentはDXGameObjectがファクトリに相当)
生成管理をしています。

私が使えば何も問題がありません。
ただ私以外の人が使ったときに想定外の使い方、
具体的に言えば
DXGameObjectとComponentを直接newする可能性があるということです。
意図しないとこで意図しないクラスがnewされるというのはなんとも微妙な感じです。

ただ今回修正していない理由は極めて単純です。
されたところで特に害がないからです。
意図しない使い方をされて害がないというのは
どういうことって思うかもしれません。
害がない理由はDXGameObjectもComponentも
それ単品では動けないからです。
どちらのクラスもファクトリに相当するクラスからの
メッセージを受けて動きます。
ただファクトリがメッセージを飛ばすのは
自身が生成したインスタンスだけです。
動的に存在するインスタンスを捕まえて自身の管理リストに入れるような処理は組み込んでいませんから。

なのでファクトリを経由せずインスタンスを作ったところで
何も起きません
なので極めて無害なので私はやらなくてもいいのでは?と思いましたし、
実際指摘をしてくれた方もわざわざやる必要があるのかなぁ?
と首を傾げていたので今回は放置することにしました。
(まぁこれがエンジン開発とかなったら話変わるんでしょうけどね)

まぁ実装コストとそれに見合う結果を得られなさそうという判断ですね。

あとがき

今回は作品を見せてもらった意見のフィードバックでした。
Componentの判定はある程度C++出来るようになった今だと
なんであんな実装にしたんだろうと当時の私を蹴り飛ばしたくなりましたw

意図しないとこでnew出来てしまう問題はどうしても気になったら
コンストラクタをprivateにしてfriend使って解決できるという話を聞きました。
ただ私個人の価値観ですがfriendって特定クラスだけとはいえ
privateな部分に対してのアクセスを許可することになるので
意図しないとこでnew出来てしまうのと
同じかそれ以上に気持ち悪く感じちゃうんですよね。
まぁこの問題をどうするかは色んな人に意見をもらいながら決めたいと思います。

それでは今回はこの辺でノシ

今回作ったものはgithubに上げました
github.com