Skip to content

Conversation

@stm2
Copy link
Contributor

@stm2 stm2 commented Jan 15, 2026

Tests und Implementierung für Schiffe und Gebäude war nicht parallel

Copilot AI review requested due to automatic review settings January 15, 2026 12:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a bug where empty units (units with number = 0) could remain as ship owners, bringing the ship ownership logic in line with the existing building ownership logic.

Changes:

  • Added a check in ship_owner() to ensure units with number <= 0 cannot be ship owners
  • Corrected a test to properly test the scenario where a unit becomes empty (rather than leaving the ship)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/kernel/ship.c Added owner->number <= 0 check to prevent empty units from being ship owners
src/kernel/ship.test.c Fixed test to use u->number = 0 instead of leave_ship(u) to match test intent
Comments suppressed due to low confidence (2)

src/kernel/ship.test.c:160

  • The test test_shipowner_goes_to_next_when_empty uses leave_ship(u) which tests a different scenario than intended. For consistency with the building tests and the test name, it should use u->number = 0 to test the case where a unit becomes empty (rather than leaves the ship). There is a separate test test_shipowner_goes_to_next_after_leave that tests the leave scenario.
    leave_ship(u);

src/kernel/ship.test.c:225

  • The test test_shipowner_goes_to_same_faction_when_empty uses leave_ship() which tests a different scenario than intended. For consistency with the building tests (see test_buildingowner_goes_to_same_faction_when_empty which uses u->number = 0 and u3->number = 0) and the test name, it should use u->number = 0 and u3->number = 0 to test the case where units become empty. There is a separate test test_shipowner_goes_to_same_faction_after_leave that tests the leave scenario.
    leave_ship(u);
    CuAssertPtrEquals(tc, u3, ship_owner(sh));
    leave_ship(u3);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ennorehling ennorehling self-assigned this Jan 19, 2026
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