search

sash2104 / carcassonne

An AI for carcassonne
0 stars 0 forks source link

棋譜読み込みとバリデーションの実装 #11

Closed mikarru closed 6 years ago

mikarru commented 6 years ago

以下の実装を行いました。

sash2104 commented 6 years ago

プルリクありがとうございます。今年中には確認します。

sash2104 commented 6 years ago

コンパイル時に、

segment.cpp: In member function 'bool Segment::isAdjacentTo(int) const':
segment.cpp:55:10: warning: enumeration value 'INVALID' not handled in switch [-Wswitch]
   switch (type_) {
          ^

のwarningがでるようになっちゃったから、switchの最後に

default:
 assert(false);

みたいなの入れるなりでwarningの解消をお願いします。

sash2104 commented 6 years ago

すぐに対応できそう and/or した方がよさそう な部分だけコメントしました。 全体としては、new/deleteがいろんなところに出てきていてdeleteし忘れとかが起きやすそう (実際に、valgrindというメモリリークチェックツールにかけた結果を見るとdeleteし忘れがありそう) なので、いずれはスマートポインタ使うなどでメモリリークが起きにくい設計にしたいなーと思った。

処理としては問題なく動いてそうなので、最低限、一つ目のコメントの修正だけしてもらえればマージしちゃっていいと思う。

mikarru commented 6 years ago

レビューありがとう。 明日ぐらいには修正します。

あと、masterの更新分を取り込んだらエラーがでるようになったので、それを修正しました。 多分、unique_ptrが中でdeleteを呼び出していてTileの定義が必要なのに、それがなかったので、怒られたのだと思います。 https://stackoverflow.com/questions/9954518/stdunique-ptr-with-an-incomplete-type-wont-compile (向こうのPRでヘッダーからtile.hppのインクルードを外していて、その変更を取り込んだから、エラーが出るようになったのだと思います。)

mikarru commented 6 years ago

すぐに対応できそうなところは対応しました。

全体としては、new/deleteがいろんなところに出てきていてdeleteし忘れとかが起きやすそう (実際に、valgrindというメモリリークチェックツールにかけた結果を見るとdeleteし忘れがありそう) なので、いずれはスマートポインタ使うなどでメモリリークが起きにくい設計にしたいなーと思った。

これはその通り。 僕のC++力が足りないので放置してます。(佐野くんが直してくれても全然いいよー。)

sash2104 commented 6 years ago

対応ありがとう。

僕のC++力が足りないので放置してます。(佐野くんが直してくれても全然いいよー。)

自分の、GUI作る方があんま進んでないからGUIが落ち着いたらこっちにも何かしらの貢献ができればと思ってます。今は俵君に任せっきりで申し訳ない。

ひとまず、このプルリクに関してはマージします。

mikarru commented 6 years ago

早くGUIでAIと対戦できるようになりたいよねー。 けっこう楽しんでやってるので大丈夫。