Skip to content

allow stop halt for each player#540

Open
simupoppo wants to merge 5 commits into
teamhimeh:OTRP-KUTAv6from
simupoppo:OTRP-KUTA-RC_add_stop_player
Open

allow stop halt for each player#540
simupoppo wants to merge 5 commits into
teamhimeh:OTRP-KUTAv6from
simupoppo:OTRP-KUTA-RC_add_stop_player

Conversation

@simupoppo

Copy link
Copy Markdown
Collaborator

駅への他社乗り入れ許可について、各社ごとに個別に設定可能にします。
https://forum.simutrans.com/index.php/topic,23915.0.html の移植になります。

  • 既存の他社乗り入れ許可は「全社乗り入れ許可」となり、自動的に他社・新規会社の乗り入れを許可します。
  • 乗り入れを許可された会社は駅の待機人数の近くにラインカラーが表示されます。

@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 Antigravity.

This review was generated by analyzing the diff in the context of the full codebase.

Comment thread simhalt.cc Outdated
}
}

if( file->is_saving() && get_permissions()!=0 && file->get_OTRP_version()<56 ) {

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.

古いフォーマットでセーブする際、if( file->is_saving() && get_permissions()!=0 && file->get_OTRP_version()<56 ) の条件では、ほぼすべての私有駅に HS_ALLOW_OTHER_PLAYER_CONNECTION が設定されてしまいます。

set_permissions では所有者のビットが必ずセットされるため、所有されている駅の get_permissions() は0になりません。他プレイヤーへの許可があるか確認するためには、例えば (get_permissions() & ~(1 << owner->get_player_nr())) != 0 のように判定する必要があります。

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

Comment thread simhalt.cc
Comment thread simhalt.cc
Comment thread simhalt.cc
}
}

// Track permission changes for dirty-rect management

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.

描画の不具合(グラフィカルグリッチ)が発生するリスクがあります。last_player_countpermissions != last_permissions の時のみ更新されます。しかし、公共の駅(permissions == 0xFFFF)の場合、プレイヤーが削除されたり倒産したりしても permissions は変わらないのに対し、player_count は減少します。

プレイヤーが離脱すると welt->get_player(i) が null になり、player_count が変わりますが、公共駅の permissions は変わらないため players_dirty が false のままとなり、古い描画領域が dirty としてマークされず、last_player_count も更新されないため、描画の残像(ゴースト)が残る可能性があります。ここでは if( permissions != last_permissions || player_count != last_player_count ) のように判定するべきです。

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

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.

↑これはやらなくてもいいです

@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 Antigravity.

Comment thread simhalt.cc
set_permissions(owner ? (1 << owner->get_player_nr()) : 0xFFFF);
}
}
else if( file->is_saving() ) {

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.

セーブ中に実行中のゲーム状態を変更してしまっています。else if (file->is_saving()) の中で set_permissions(...) を呼び出すと、駅のアクセス権が変更され、スケジュール登録の再構築がトリガーされてしまいます。

セーブ処理(file->is_saving())はゲーム状態に対して読み取り専用であるべきです。ここで permissions を変更し rebuilt_schedule_registration() を呼び出すと、古いバージョンにセーブしたというだけで駅が公共駅になったり私有駅になったりと、実行中のゲーム状態を破壊してしまいます。このブロックは削除するべきです。

(Antigravity による自動レビュー・再投稿)

@simupoppo

Copy link
Copy Markdown
Collaborator Author

rdwrを修正しました

@teamhimeh

teamhimeh commented Jun 1, 2026

Copy link
Copy Markdown
Owner

コードの変更内容を分析し、変更を網羅するための手動テストケースを作成しました。
予期せぬバグや互換性問題が潜んでいないか確認するため、以下のテストの実施を推奨します。

提案する手動テストケース

  • テストケース1:各プレイヤーごとの乗り入れ許可の基本動作確認

    1. プレイヤーAで駅を建設する。
    2. プレイヤーBの車両をその駅に停車させるスケジュールを組もうとした場合、エラーになるか経路が見つからないことを確認する。
    3. プレイヤーAの駅詳細ウィンドウを開き、プレイヤーBの乗り入れを許可する。
    4. プレイヤーBの車両がその駅に停車し、乗降できることを確認する。
  • テストケース2:乗り入れ許可の取り消し時の動作確認

    1. プレイヤーBがプレイヤーAの駅を利用している状態で、プレイヤーAがプレイヤーBの乗り入れ許可を取り消す。
    2. プレイヤーBのスケジュールから該当駅が外れるか、経路なしエラーになることを確認する。
  • テストケース3:全社乗り入れ許可の動作確認

    1. プレイヤーAの駅で「全社乗り入れ許可」をオンにする。
    2. 既存のプレイヤーBだけでなく、新規に設立したプレイヤーCもその駅を利用できることを確認する。
  • テストケース4:許可された会社のプレイヤーカラー表示確認

    1. プレイヤーAの駅で、プレイヤーBとプレイヤーCの乗り入れを許可する。
    2. マップ上の該当駅の待機人数の近くに、プレイヤーA、B、Cのプレイヤーカラーが帯として表示されることを確認する。
    3. プレイヤーCを削除(倒産)させた後、プレイヤーCの色が消え、描画の乱れ(ゴースト)が残らないことを確認する。
  • テストケース5:セーブ・ロードの互換性確認

    1. 古いバージョン(本PR適用前)のセーブデータを読み込む。
    2. 以前「他社乗り入れ許可」がオンになっていた駅が、全社乗り入れ許可(全プレイヤーに許可)として扱われていることを確認する。
    3. 新しいバージョンで一部のプレイヤーのみ許可した状態をセーブし、ロード後に正しく復元されることを確認する。
  • テストケース6:会社の合併時の動作確認

    1. プレイヤーAの駅でプレイヤーBの乗り入れを許可する(プレイヤーCは許可しない)。
    2. プレイヤーBがプレイヤーCに合併される操作を行う。
    3. 合併後、プレイヤーCがプレイヤーAの駅への乗り入れ許可を引き継いでいることを確認する。

(Automated PR test case generation by AI agent)

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