Skip to content

train can run on different branches of a 4-way crossing simultaneously#584

Open
simupoppo wants to merge 6 commits into
teamhimeh:OTRP-KUTAv6from
simupoppo:OTRP-KUTA-RC_crossing_update
Open

train can run on different branches of a 4-way crossing simultaneously#584
simupoppo wants to merge 6 commits into
teamhimeh:OTRP-KUTAv6from
simupoppo:OTRP-KUTA-RC_crossing_update

Conversation

@simupoppo

Copy link
Copy Markdown
Collaborator

4方向分岐において、北<->西と東<->南など、対角の辺を同時に走行可能にします。
これにより、斜めタイルにおいて複線間隔を狭めて運行可能になります

@simupoppo

Copy link
Copy Markdown
Collaborator Author

テストケースとして、単線、3方向分岐、4方向分岐各種における分岐の進入制限・進入許可のデバッグ用自動テストを追加済みです

@teamhimeh teamhimeh left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review by Codex.

このレビューは、PR差分をコードベース全体の文脈で分析し、代表テストを変更前・変更後の両バイナリで検証して生成しました。

Comment thread boden/wege/schiene.cc
{
if(can_reserve(c)) {
reserved = c;
if(c == reserved) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同一 convoy の再予約で reserved_dir を更新しないため、ロード時に reserve(self, ribi_t::none) された占有タイルは finish_rd() 後も方向が none のままです。また convoi_t::reserve_route() は旧形式の ribi_type(prev,next) を渡しており、corner_set に復元できません。ロード後の予約再構築でも正しい方向を設定できるようにしてください。

(Codex による自動レビュー)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応しました

Comment thread vehicle/simvehicle.cc
// unreserve(vehicle_t*) always cleared the primary slot regardless of which
// convoy this vehicle belongs to — causing spurious promotion/clearing bugs.
const convoihandle_t self_cnv = cnv ? cnv->self : convoihandle_t();
sch0->unreserve(self_cnv);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この箇所だけ convoy handle に変更しても、schiene_t::unreserve(vehicle_t*) は引数を無視して primary reservation を解除します。simconvoi.ccsimtool.cc に同 overload の呼び出しが残っているため、secondary convoy の処理が primary convoy の予約を消し得ます。overload 自体を convoy-aware にするか、全呼び出しを handle 版へ移行してください。

(Codex による自動レビュー)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応しました

Comment thread boden/wege/schiene.h
* true, if this rail can be reserved
*/
bool can_reserve(convoihandle_t c) const { return !reserved.is_bound() || c==reserved; }
bool can_reserve(convoihandle_t c) const { return !reserved.is_bound() || c==reserved || c==reserved2; }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can_reserve() は secondary reservation も自 convoy と認識しますが、get_reserved_convoi() は primary しか返しません。そのため is_next_tile_already_reserved() が secondary convoy 自身の予約を見落とし、stop-before-check signal 等で不要な停止・再予約を起こします。予約所有者の判定 API も両スロットを扱うようにしてください。

(Codex による自動レビュー)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応しました

_cr_remove_infra(pl)
RESET_ALL_PLAYER_FUNDS()

ASSERT_TRUE(reached_a)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この permitted test は両 convoy が最終的に到達したことしか検証しておらず、同時予約または同時交差を観測していません。従来の排他的予約でも順番に到達して通るため、co-reservation が無効でも回帰を検出できません。実際、このテストは変更前の OTRP-KUTAv6 バイナリでも成功しました。両 convoy が crossing を同時に予約・通過できたことを直接確認する assertion を追加してください。

(Codex による自動レビュー)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

テスト線の路線長を変更し、同時進入と待機後進入を進入したフラグの通過順序で判定するようにしました。テスト済みです

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants