Skip to content

Update removeFromSquad to match in-game logic #5527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ab9rf merged 1 commit into from
Aug 15, 2025
Merged

Update removeFromSquad to match in-game logic #5527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Template for new versions:
## Documentation

## API
- Adjusted the logic inside ``Military::removeFromSquad`` to more closely match the game's own behavior

## Lua

Expand Down
59 changes: 35 additions & 24 deletions library/modules/Military.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,42 +477,53 @@ bool Military::addToSquad(int32_t unit_id, int32_t squad_id, int32_t squad_pos)

bool Military::removeFromSquad(int32_t unit_id)
{
// based on unitst::remove_squad_info
df::unit *unit = df::unit::find(unit_id);
if (unit == nullptr || unit->military.squad_id == -1 || unit->military.squad_position == -1)
return false;

int32_t squad_id = unit->military.squad_id;
df::squad* squad = df::squad::find(squad_id);
if (squad == nullptr)
return false;
// abort individual training
if (unit->individual_drills.size())
unit->flags3.bits.verify_personal_training = true;

int32_t squad_id = unit->military.squad_id;
int32_t squad_pos = unit->military.squad_position;
df::squad_position* pos = vector_get(squad->positions, squad_pos);
if (pos == nullptr)
return false;

df::historical_figure* hf = df::historical_figure::find(unit->hist_figure_id);
if (hf == nullptr)
return false;
df::squad* squad = df::squad::find(squad_id);

if (squad)
{
df::squad_position* pos = vector_get(squad->positions, squad_pos);
if (pos)
{
// based on unitst::remove_squad_activity_info
FOR_ENUM_ITEMS(squad_event_type, i)
{
auto activity_entry = df::activity_entry::find(pos->activities[i]);
if (activity_entry)
{
auto activity_event = binsearch_in_vector(activity_entry->events, &df::activity_event::event_id, pos->events[i]);
if (activity_event)
{
activity_event->removeParticipant(unit->hist_figure_id, unit->id, false);
pos->activities[i] = -1;
pos->events[i] = -1;
}
}
}

// remove from squad information
pos->occupant = -1;
// remove from squad position
pos->occupant = -1;
}
}
// remove from unit information
unit->military.squad_id = -1;
unit->military.squad_position = -1;
Comment on lines 520 to 521
Copy link
Member

@chdoc chdoc Jul 28, 2025

Choose a reason for hiding this comment

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

How about moving this before the if (squad) and turning the test into an early return if the squad is not defined, reducing the nesting level by one.


// abort individual training
if (unit->individual_drills.size()) {
unit->flags3.bits.verify_personal_training = true;
}

// remove unit from squad activities
auto activity_entry = binsearch_in_vector(df::global::world->activities.all,&df::activity_entry::id,squad->activity);
if (activity_entry) {
for (auto const activity_event : activity_entry->events) {
activity_event->removeParticipant(unit->hist_figure_id, unit->id, false);
}
}
// remove entity squad link
df::historical_figure* hf = df::historical_figure::find(unit->hist_figure_id);
if (hf == nullptr || squad == nullptr)
Copy link
Member

@chdoc chdoc Jul 28, 2025

Choose a reason for hiding this comment

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

Suggested change
if (hf == nullptr || squad == nullptr)
if (hf == nullptr)

assuming the test for the squad is turned into an early return

Copy link
Member Author

@quietust quietust Jul 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Because the in-game logic will still clear user->military.squad_id + user->military.squad_position even if the squad doesn't actually exist anymore.

I suppose the case could be made for clearing those variables earlier, but I'm also replicating the order in which the game's own logic does it.

Copy link
Member

@chdoc chdoc Jul 29, 2025

Choose a reason for hiding this comment

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

Yes, I wasn't suggesting to change the behavior. As it comes to the order of things, I don't see any volatile variables. So the real order is whatever the compiler makes of it, both for our code and the game code. I would prefer to see less nesting and not fetching the historical_figure if there is no squad. But then, this is all well and truly in the eye of the beholder. What I truly care about is that autotraining works, which it seems to do with his implementation.

return false;

if (squad_pos == 0) // is unit a commander?
remove_officer_entity_link(hf, squad);
Expand Down
Loading