フリーランモードに対応 #4
Merged
フリーランモードに対応 #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthroughこの変更は、プロジェクト全体の高レベルな改善を行いました。環境変数のチェック、クラウドストレージからファイルのダウンロード、指定されたパラメータでサーバーを実行し、ログをクラウドストレージにアップロードするスクリプトに関する変更が含まれています。 Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: .coderabbit.yaml
Files selected for processing (19)
- .dockerignore (1 hunks)
- .github/pr-labeler.yml (1 hunks)
- .github/release-drafter.yml (1 hunks)
- .github/workflows/pr_label.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .gitignore (1 hunks)
- Cargo.lock (13 hunks)
- Cargo.toml (1 hunks)
- Dockerfile (1 hunks)
- ai_bridge/src/ai_loader.rs (1 hunks)
- ai_bridge/src/interface.rs (12 hunks)
- ai_bridge/tests/test_func.rs (6 hunks)
- docker-compose.yml (1 hunks)
- install.sh (1 hunks)
- loadlibrary (1 hunks)
- mahjong-app-engine (1 hunks)
- run.sh (1 hunks)
- server/Cargo.toml (1 hunks)
- server/src/main.rs (3 hunks)
Files skipped from review due to trivial changes (4)
- .dockerignore
- Cargo.toml
- loadlibrary
- mahjong-app-engine
Additional comments: 28
.gitignore (1)
- 1-3: .gitignoreファイルに変更が加えられましたが、これらの変更は適切です。
docker-compose.yml (1)
- 4-13:
docker-compose.ymlのappサービスのボリュームマッピングで、ホストの現在のディレクトリをコンテナ内の/home/app_copyにマッピングしていますが、このパスはプロジェクトの構造やDockerfileと整合性が取れているか確認が必要です。.github/workflows/pr_label.yml (1)
- 1-15:
pr_label.ymlの設定は適切に見えますが、実際のラベル名とGitHubリポジトリのラベル設定が一致しているか確認する必要があります。server/Cargo.toml (1)
- 16-17:
Cargo.tomlに追加されたclapとdotenvの依存関係は適切です。Dockerfile (1)
- 1-25: Dockerfileの変更は、基本的に適切に見えますが、
RUN cargo build -p server --release --features load-dllのコマンドでビルドされるバイナリが実際に/home/app/target/release/serverに存在するか確認する必要があります。.github/release-drafter.yml (1)
- 1-31:
release-drafter.ymlの設定は適切に見えます。特に修正する必要はありません。.github/workflows/release.yml (1)
- 1-33:
release.ymlの設定は適切に見えます。GitHub Actionsの設定に関して特に問題はありません。ai_bridge/tests/test_func.rs (1)
- 72-118: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [3-167]
test_func.rsのテストケースにおいて、state.createやstate.sutehaiの呼び出しに追加の引数が必要になっていますが、これらの変更は適切に行われています。ai_bridge/src/interface.rs (6)
- 40-46:
MJRL_AKA5の処理において、明示的なブレースの追加が行われていますが、Rustの慣習として、単一の式であればブレースは省略可能です。ただし、この変更はコードの明確性を向上させるために有用かもしれません。- 49-56:
MJRL_SHANYUの処理において、if-elseブロックに明示的なブレースが追加されています。この変更はコードの読みやすさを向上させますが、Rustの慣習としては必須ではありません。- 59-66:
MJRL_NANNYUの処理で、if-elseブロックにブレースが追加されています。この変更はコードの可読性を高めるものですが、Rustでは単一の式の場合ブレースを省略することが一般的です。- 90-90:
mjsend_message_impl関数のpstateパラメータは、可変参照として渡されています。これは、関数内でPaiStateの状態を変更するために必要です。適切な使用です。- 94-99: コメントアウトされた
println!マクロは、デバッグ目的で使用されていた可能性がありますが、本番コードには含めない方が良いでしょう。不要なコードは削除することを推奨します。- 447-455: LinuxまたはWindows以外のOSで
mjsend_message関数を使用する場合、PaiState::default()を使用してpstateを初期化しています。このアプローチは、上記のMaybeUninitを使用する方法よりも簡潔で安全です。Cargo.lock (14)
- 85-97: 新しいパッケージ
anstreamとその依存関係が追加されました。これはPRの目的に沿っています。- 99-103: 新しいパッケージ
anstyleが追加されました。これはPRの目的に沿っています。- 105-111: 新しいパッケージ
anstyle-parseとその依存関係が追加されました。これはPRの目的に沿っています。- 114-120: 新しいパッケージ
anstyle-queryとその依存関係が追加されました。これはPRの目的に沿っています。- 123-130: 新しいパッケージ
anstyle-winconとその依存関係が追加されました。これはPRの目的に沿っています。- 338-346: 新しいパッケージ
clapとその依存関係が追加されました。これはPRの目的に沿っています。- 348-357: 新しいパッケージ
clap_builderとその依存関係が追加されました。これはPRの目的に沿っています。- 361-369: 新しいパッケージ
clap_deriveとその依存関係が追加されました。これはPRの目的に沿っています。- 387-391: 新しいパッケージ
colorchoiceが追加されました。これはPRの目的に沿っています。- 463-467: 新しいパッケージ
dotenvが追加されました。これはPRの目的に沿っています。- 669-673: 新しいパッケージ
heckが追加されました。これはPRの目的に沿っています。- 1689-1692: 新しいパッケージ
strsimが追加されました。これはPRの目的に沿っています。- 1970-1973: 新しいパッケージ
utf8parseが追加されました。これはPRの目的に沿っています。- 1987-1994: 新しいパッケージ
uuid-macro-internalとその依存関係が追加されました。これはPRの目的に沿っています。
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- run.sh (1 hunks)
- server/src/main.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- run.sh
Additional comments: 2
server/src/main.rs (2)
- 148-200: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [120-197]
cmd関数では、複数のunsafeブロックを使用して外部ライブラリとのインタラクションが行われています。unsafeブロックの使用は避けられない場合がありますが、これらのブロック内でのエラーハンドリングが重要です。特に、unwrapを使用する代わりに、エラーが発生した場合に適切なエラーメッセージを提供することで、デバッグを容易にすることが推奨されます。ただし、以前のレビューコメントでunwrapの使用が意図的であることが確認されているため、この点については追加のアクションは不要です。
- 208-220:
main関数において、環境変数の読み込み、コマンドライン引数の解析、Sentryの初期化、そしてcmd関数の呼び出しが行われています。unwrapを使用している箇所がありますが、これは以前のレビューコメントでSentry通知のために意図的に使用されていることが確認されています。このため、この実装については特に問題はありません。
Comment on lines
+21
to
+30
| #[derive(Parser, Debug)] | ||
| #[command(author, about, version)] | ||
| struct Command { | ||
| #[arg(short, long)] | ||
| log_path: String, | ||
| #[arg(short, long)] | ||
| input_dll: String, | ||
| #[arg(short, long)] | ||
| paiyama_path: Option<String>, | ||
| } |
There was a problem hiding this comment.
Command構造体の定義については、コマンドライン引数を解析するための明確な方法を提供しており、適切に実装されています。ただし、フィールドに対するドキュメントコメントがないため、各フィールドの目的がコードから直接読み取りにくい可能性があります。
#[derive(Parser, Debug)]
#[command(author, about, version)]
struct Command {
+ /// ログファイルのパス
#[arg(short, long)]
log_path: String,
+ /// 入力DLLのパス
#[arg(short, long)]
input_dll: String,
+ /// パイ山ファイルのパス（オプション）
#[arg(short, long)]
paiyama_path: Option<String>,
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| #[derive(Parser, Debug)] | |
| #[command(author, about, version)] | |
| struct Command { | |
| #[arg(short, long)] | |
| log_path: String, | |
| #[arg(short, long)] | |
| input_dll: String, | |
| #[arg(short, long)] | |
| paiyama_path: Option<String>, | |
| } | |
| #[derive(Parser, Debug)] | |
| #[command(author, about, version)] | |
| struct Command { | |
| /// ログファイルのパス | |
| #[arg(short, long)] | |
| log_path: String, | |
| /// 入力DLLのパス | |
| #[arg(short, long)] | |
| input_dll: String, | |
| /// パイ山ファイルのパス（オプション） | |
| #[arg(short, long)] | |
| paiyama_path: Option<String>, | |
| } |
Comment on lines
+32
to
+118
| unsafe fn experiment(func: MJPInterfaceFuncP, inst: *mut c_void, play_log: &mut play_log::PlayLog) { | ||
| { | ||
| let state = &mut G_STATE; | ||
| state.start(play_log); | ||
| } | ||
| func(inst, MJPI_STARTKYOKU.try_into().unwrap(), 0, 0); | ||
| println!("start kyoku end"); | ||
|
|
||
| let mut is_agari = false; | ||
|
|
||
| for _i in 0..18 { | ||
| let mut tsumohai_num: usize; | ||
| { | ||
| let state = &mut G_STATE; | ||
| state.tsumo(play_log); | ||
| tsumohai_num = state.players[state.teban as usize] | ||
| .tsumohai | ||
| .pai_num | ||
| .try_into() | ||
| .unwrap(); | ||
| } | ||
|
|
||
| ensure!(args.len() >= 2, "usage: {} DLLPath", args[0]); | ||
| let ret: u32 = func(inst, MJPI_SUTEHAI.try_into().unwrap(), tsumohai_num, 0) | ||
| .try_into() | ||
| .unwrap(); | ||
| let index = ret & 0x3F; | ||
| let flag = ret & 0xFF80; | ||
| // println!("ret = {} flag = {:04x}", index, flag); | ||
|
|
||
| { | ||
| let state = &mut G_STATE; | ||
| /* | ||
| { | ||
| let player = &state.players[state.teban as usize]; | ||
| for p in &player.tehai { | ||
| print!("{}", p); | ||
| } | ||
|
|
||
| print!("{}", player.tsumohai); | ||
| let shanten = PaiState::from(&player.tehai).get_shanten(0); | ||
| println!(" シャンテン数 {}\r", shanten); | ||
| } | ||
| */ | ||
|
|
||
| if flag == MJPIR_SUTEHAI { | ||
| state.sutehai(play_log, index as usize, false); | ||
| } else if flag == MJPIR_REACH { | ||
| state.sutehai(play_log, index as usize, true); | ||
| } else if flag == MJPIR_TSUMO { | ||
| let score: [i32; 4] = [0, 0, 0, 0]; | ||
| println!("agari!!!"); | ||
| let agari_r = state.tsumo_agari(play_log); | ||
|
|
||
| match agari_r { | ||
| Ok(agari) => { | ||
| println!("{:?}", agari.yaku); | ||
| } | ||
| Err(e) => { | ||
| println!("{:?}", e); | ||
| } | ||
| } | ||
|
|
||
| let path = PathBuf::from(&args[1]); | ||
| is_agari = true; | ||
| func( | ||
| inst, | ||
| MJPI_ENDKYOKU.try_into().unwrap(), | ||
| MJEK_RYUKYOKU.try_into().unwrap(), | ||
| std::mem::transmute(score.as_ptr()), | ||
| ); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| load_ai(&path); | ||
| if !is_agari { | ||
| println!("流局(;;)"); | ||
| let score: [i32; 4] = [-3000, 0, 0, 0]; | ||
| func( | ||
| inst, | ||
| MJPI_ENDKYOKU.try_into().unwrap(), | ||
| MJEK_RYUKYOKU.try_into().unwrap(), | ||
| std::mem::transmute(score.as_ptr()), | ||
| ); | ||
| let state = &mut G_STATE; | ||
| state.nagare(play_log); | ||
| } | ||
| } |
There was a problem hiding this comment.
experiment関数において、unsafeブロック内での外部関数の呼び出しやポインタ操作が適切に行われています。ただし、println!マクロを使用したデバッグ出力が複数存在します。これらは開発中に有用ですが、本番環境のコードでは不要な出力が発生する可能性があるため、削除するか、条件付きコンパイルを使用してデバッグビルド時のみに含めることを検討してください。
- println!("start kyoku end");
+ #[cfg(debug_assertions)]
+ println!("start kyoku end");
- println!("agari!!!");
+ #[cfg(debug_assertions)]
+ println!("agari!!!");
- println!("流局(;;)");
+ #[cfg(debug_assertions)]
+ println!("流局(;;)");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| unsafe fn experiment(func: MJPInterfaceFuncP, inst: *mut c_void, play_log: &mut play_log::PlayLog) { | |
| { | |
| let state = &mut G_STATE; | |
| state.start(play_log); | |
| } | |
| func(inst, MJPI_STARTKYOKU.try_into().unwrap(), 0, 0); | |
| println!("start kyoku end"); | |
| let mut is_agari = false; | |
| for _i in 0..18 { | |
| let mut tsumohai_num: usize; | |
| { | |
| let state = &mut G_STATE; | |
| state.tsumo(play_log); | |
| tsumohai_num = state.players[state.teban as usize] | |
| .tsumohai | |
| .pai_num | |
| .try_into() | |
| .unwrap(); | |
| } | |
| ensure!(args.len() >= 2, "usage: {} DLLPath", args[0]); | |
| let ret: u32 = func(inst, MJPI_SUTEHAI.try_into().unwrap(), tsumohai_num, 0) | |
| .try_into() | |
| .unwrap(); | |
| let index = ret & 0x3F; | |
| let flag = ret & 0xFF80; | |
| // println!("ret = {} flag = {:04x}", index, flag); | |
| { | |
| let state = &mut G_STATE; | |
| /* | |
| { | |
| let player = &state.players[state.teban as usize]; | |
| for p in &player.tehai { | |
| print!("{}", p); | |
| } | |
| print!("{}", player.tsumohai); | |
| let shanten = PaiState::from(&player.tehai).get_shanten(0); | |
| println!(" シャンテン数 {}\r", shanten); | |
| } | |
| */ | |
| if flag == MJPIR_SUTEHAI { | |
| state.sutehai(play_log, index as usize, false); | |
| } else if flag == MJPIR_REACH { | |
| state.sutehai(play_log, index as usize, true); | |
| } else if flag == MJPIR_TSUMO { | |
| let score: [i32; 4] = [0, 0, 0, 0]; | |
| println!("agari!!!"); | |
| let agari_r = state.tsumo_agari(play_log); | |
| match agari_r { | |
| Ok(agari) => { | |
| println!("{:?}", agari.yaku); | |
| } | |
| Err(e) => { | |
| println!("{:?}", e); | |
| } | |
| } | |
| let path = PathBuf::from(&args[1]); | |
| is_agari = true; | |
| func( | |
| inst, | |
| MJPI_ENDKYOKU.try_into().unwrap(), | |
| MJEK_RYUKYOKU.try_into().unwrap(), | |
| std::mem::transmute(score.as_ptr()), | |
| ); | |
| break; | |
| } | |
| } | |
| } | |
| load_ai(&path); | |
| if !is_agari { | |
| println!("流局(;;)"); | |
| let score: [i32; 4] = [-3000, 0, 0, 0]; | |
| func( | |
| inst, | |
| MJPI_ENDKYOKU.try_into().unwrap(), | |
| MJEK_RYUKYOKU.try_into().unwrap(), | |
| std::mem::transmute(score.as_ptr()), | |
| ); | |
| let state = &mut G_STATE; | |
| state.nagare(play_log); | |
| } | |
| } | |
| unsafe fn experiment(func: MJPInterfaceFuncP, inst: *mut c_void, play_log: &mut play_log::PlayLog) { | |
| { | |
| let state = &mut G_STATE; | |
| state.start(play_log); | |
| } | |
| func(inst, MJPI_STARTKYOKU.try_into().unwrap(), 0, 0); | |
| #[cfg(debug_assertions)] | |
| println!("start kyoku end"); | |
| let mut is_agari = false; | |
| for _i in 0..18 { | |
| let mut tsumohai_num: usize; | |
| { | |
| let state = &mut G_STATE; | |
| state.tsumo(play_log); | |
| tsumohai_num = state.players[state.teban as usize] | |
| .tsumohai | |
| .pai_num | |
| .try_into() | |
| .unwrap(); | |
| } | |
| let ret: u32 = func(inst, MJPI_SUTEHAI.try_into().unwrap(), tsumohai_num, 0) | |
| .try_into() | |
| .unwrap(); | |
| let index = ret & 0x3F; | |
| let flag = ret & 0xFF80; | |
| // println!("ret = {} flag = {:04x}", index, flag); | |
| { | |
| let state = &mut G_STATE; | |
| /* | |
| { | |
| let player = &state.players[state.teban as usize]; | |
| for p in &player.tehai { | |
| print!("{}", p); | |
| } | |
| print!("{}", player.tsumohai); | |
| let shanten = PaiState::from(&player.tehai).get_shanten(0); | |
| println!(" シャンテン数 {}\r", shanten); | |
| } | |
| */ | |
| if flag == MJPIR_SUTEHAI { | |
| state.sutehai(play_log, index as usize, false); | |
| } else if flag == MJPIR_REACH { | |
| state.sutehai(play_log, index as usize, true); | |
| } else if flag == MJPIR_TSUMO { | |
| let score: [i32; 4] = [0, 0, 0, 0]; | |
| #[cfg(debug_assertions)] | |
| println!("agari!!!"); | |
| let agari_r = state.tsumo_agari(play_log); | |
| match agari_r { | |
| Ok(agari) => { | |
| println!("{:?}", agari.yaku); | |
| } | |
| Err(e) => { | |
| println!("{:?}", e); | |
| } | |
| } | |
| is_agari = true; | |
| func( | |
| inst, | |
| MJPI_ENDKYOKU.try_into().unwrap(), | |
| MJEK_RYUKYOKU.try_into().unwrap(), | |
| std::mem::transmute(score.as_ptr()), | |
| ); | |
| break; | |
| } | |
| } | |
| } | |
| if !is_agari { | |
| #[cfg(debug_assertions)] | |
| println!("流局(;;)"); | |
| let score: [i32; 4] = [-3000, 0, 0, 0]; | |
| func( | |
| inst, | |
| MJPI_ENDKYOKU.try_into().unwrap(), | |
| MJEK_RYUKYOKU.try_into().unwrap(), | |
| std::mem::transmute(score.as_ptr()), | |
| ); | |
| let state = &mut G_STATE; | |
| state.nagare(play_log); | |
| } | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
新機能
.gitディレクトリとlogs/ディレクトリを.dockerignoreファイルで除外するパターンを追加しました。[profile.release]セクションに追加しました。appサービスのビルド設定を更新し、ボリュームマッピングを変更しました。バグ修正
.dllファイル、targetという名前のディレクトリ、およびlogsディレクトリを無視するルールを導入しました。ドキュメント
リファクタリング
load_ai関数とget_ai_symbol関数を更新しました。PaiStateインスタンスを初期化しました。clap::Parserインポートを追加し、ai_bridgeのインポートと構造体フィールドを修正し、experiment関数を更新し、main関数をリファクタリングしました。依存関係の更新
clapバージョン 4.5.0 とdotenvバージョン 0.15.0 を追加しました。