スマホゲームでのガチャの排出に関するトラブルなんてもう耳タコレベルの話題だとは思うんだけど，バンダイナムコゲームスの『ドラゴンボールZ ドッカンバトル』 で詫びソースコードが出てくる斬新な対応が目を引いたね。
（詫び石は配布済みみたいだよ）
当該ゲームには全く手を出してないんだけど，ソースコードの量がチラッと見てすぐに終わる感じなのでチラッと見て気になったことを記事にしてみた。
「ドラゴンボールZ ドッカンバトル」一部ガシャの「出現キャラ一覧」及び「出現キャラ提供割合」表示に関する不具合につきまして
というわけで今回はひたすら IT 系。
そもそもの問題
ソースコードをプレスリリースに出すとか
想定読者だれなんだよ，と。
そもそもプレスリリースにソースコードを出してくるってのが異常だよね。
プログラマー晒しかよって思うし *1，C++ のソースコードなんてゲームのユーザーに見せて何の説明した気になってるんだよと。
公開範囲が足りん
一部だけ晒すとか意味ないから全部 github の公開リポジトリにすればいいんでないかというね。
版権リソースだけ submodule に出して社内管理にしとけばいいっしょ。
上のリンク先，"「出現キャラ一覧」及び「出現キャラ提供割合」の表示に関する不具合の詳細" （以下『詳細プレス』とするよ） では，
ストレージから取得したカードIDリストは順序が保たれておらず
ってあるんだけど，どうせソース出すならストレージに順不同で保存しているロジックも曝さないとねえ。
とくに _cardDataCache.fetch() と _cardDataCache.store()．
このメソッドに対するユニットテストとカード ID リストの CRUD に関するテストも曝さないとねえ。
片手落ちでしょう。
詫びソースコード （前掲の記事から引用）
CardDatas CardModel::getMasterCardDatasByIds( const std::vector<uint32_t>& masterCardIds ) const { vector<CardDataPtr> results; results.resize(masterCardIds.size()); size_t exists = 0; for (int i = 0; i < masterCardIds.size(); i++) { auto p = results[i] = _cardDataCache.fetch(masterCardIds[i]); if (p != nullptr) { exists++; } } if (masterCardIds.size() == exists) { return results; } string sql = form( "SELECT * FROM cache.cards where id IN (%s);", join(masterCardIds, ",").c_str() ); int i = 0; DatabaseManager::getInstance()->query( sql, [this, &results, &i](sqlite3_stmt* stmt) { if (results[i] == nullptr) { results[i] = _cardDataCache.store(make_shared<CardData>(stmt)); } i++; } ); return results; }
横スクロール出ると読みにくいから改行入れたよ。
ダメなところ
基本的に全部個人的な意見や思い入れや思い込みだよ。
と保身しておきつつ。
まーコメント無い以外は致命的にダメなところないかなー。
コメントを付けるべし
型や引数やローカル変数が比較的まともな命名規約だから，このレベルならわたしもコメントあまり入れないけど，メソッドコメントは少なくとも欲しいな。
開発環境わからないけど Visual Studio なら XML ドキュメントで，それ以外なら JavaDoc か何かで。
命名規約を詰めるべし
CardDatas 型
Datas はあちこちで突っこまれてたね。
でも，そもそもプログラムで扱うのは全部データなんだから 型名に Data を入れるのは嫌い （わたしは）。
わたしなら CardDatas 型の命名は，
Cards
か，他の Card 系の型名との絡みがあって判りにくいなら
CardPtrsCardsArrayCardsVectorCardPtrsArrayCardPtrsVector
のどれかかなあ。
複数形にした時点でコレクション型って読み取れるよね。
Vector 付けると std::vector やめて std::list とかに換えた時こまるね。
getMasterCardDatasByIds メソッド
get〜 は汎用的過ぎて良くないよねー。ぶっちゃけ何でも get って付けられるし。
この場合は CardModel::fetchMasterCardDatasByIds() でいいんでないかなあ。
exists 変数名
あと
size_t exists = 0;
の exists は，この名前だと boolean 型と思ってしまう （わたしは）。
逆にこの人，「存在する／しないフラグ」 にはどんな変数名付けるんだろう。is_exist とかやっちゃうのかな。
わたしならどうすっかなー。
size_t cardInstancesCountOnCache = 0;
かなー。英語的に正しいかはともかくとして。
typedef したならそれ使うべし
たぶん CardDatas 形は vector<CardDataPtr> の typedef だと思うんだけど，なぜ
vector<CardDataPtr> results;
は
CardDatas results;
でないのか。
typedef すべし
std::vector<uint32_t> は CardIds で typedef してありそう。
名前空間を付けるべし
std:: を付けるのか付けないのか，どっちでもいいから統一しなよ。
わたしは
using namespace std;
を書かない派なので std::vector とか std::make_shared と書くね。
たいした話じゃない
vector<CardDataPtr> results; results.resize(masterCardIds.size());
って results 用にメモリを確保してると思うんだけど，
わたしなら
CardDatas results(masterCardIds.size());
にするかなー。
いいところ
- メソッドが短い
- const 付けてる
気になること
バグの原因は _cardDataCache.store() 見ないとなんとも言えないよねー。
vector 舐めて nullptr 見つけて上書きするってのが期待された実装で，実際は push しちゃってるとか？
