Skip to content

Bandit Interactions - #2296

Open
jordanbrymora wants to merge 21 commits into
developmentfrom
BanditInteractionsSync
Open

Bandit Interactions#2296
jordanbrymora wants to merge 21 commits into
developmentfrom
BanditInteractionsSync

Conversation

@jordanbrymora

@jordanbrymora jordanbrymora commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • All new classes have class-level documentation comments, if there are any at all
  • Tests for the changes have been added (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bug fix (non-breaking change that fixes an issue)

What is the current behavior?

There are a few interactions with bandits that aren't synced:

  • Taking bandits prisoners when they surrender.
  • Recruiting surrendered bandits if have player(s) have the perk for it.
  • Interaction history (hostile, neutral).

What is the new behavior?

Resolves #1795
Resolves #3032

Bot Changelog Entry

Surrendering bandits can now be taken prisoner and recruited (with the right perk).

@zzzzzzzzott

Copy link
Copy Markdown
Contributor

lgtm but should you add some unit and regression tests here?

@jordanbrymora
jordanbrymora marked this pull request as ready for review August 15, 2026 08:53

@ShoT-UPfps ShoT-UPfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cmt

Comment thread source/GameInterface/Services/Bandits/Handlers/BanditInteractionsHandler.cs Outdated

@ShoT-UPfps ShoT-UPfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cmt

Comment thread source/GameInterface/Services/Bandits/Handlers/BanditInteractionsHandler.cs Outdated

@ShoT-UPfps ShoT-UPfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

two correctness issues remain

@ShoT-UPfps ShoT-UPfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

one correctness issue remains

@ShoT-UPfps ShoT-UPfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

two follow-up comments

@jordanbrymora
jordanbrymora marked this pull request as draft August 19, 2026 01:16
@jordanbrymora
jordanbrymora marked this pull request as ready for review August 21, 2026 07:48

@ShoT-UPfps ShoT-UPfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

two correctness issues remain

}

// Do surrender on server and send message to client to update post battle screens
var surrenderMessage = new BanditsSurrenderAsPrisoners(PlayerEncounter.Battle, PlayerEncounter.Battle.PlayerSide);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

StartBattle() can still return null for Rejected or Unresolved, so this dereference throws. don't return on null for Unresolved, because that consumes the one-shot surrender before the late-created event attaches

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added guard against NRE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this still returns for Unresolved, so the NRE is fixed but the one-shot surrender is still consumed before the late-created event attaches.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added guard against null result from StartBattle() for unresolved battles

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this still consumes the one-shot surrender for Unresolved, and it now also returns when PlayerEncounter.Battle already exists because createdBattle is only assigned in the null branch. reuse the existing battle and defer surrender until a late-created event attaches.

Comment thread source/GameInterface/Services/Bandits/Handlers/BanditInteractionsHandler.cs Outdated
@jordanbrymora
jordanbrymora dismissed ShoT-UPfps’s stale review August 22, 2026 03:28

Won't let me re-request

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

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

[BANDITS] Bandit surrender skips the prisoner interaction screen and yields no captives on dedicated servers Bandit Interactions Sync

3 participants