Skip to content
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.

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

Allow Stun Lance to target Mind Controlled units #1314

Merged
merged 2 commits into from
Mar 3, 2024

Conversation

copyrite
Copy link
Contributor

@copyrite copyrite commented Feb 29, 2024

Fixes #1312

@Iridar Iridar added bug-basegame ready-to-review A pull request is ready to be reviewed labels Mar 2, 2024
Copy link
Contributor

@Iridar Iridar left a comment

Choose a reason for hiding this comment

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

Not 100% comfortable changing a function that could be potentially getting used by mods to create their own versions of Stun Lance where the changed behavior may or may not be necessary. This change is on the edge between a bugfix and a BC break.

OTOH TreatMindControlledSquadmateAsHostile is a really weird flag you would expect to be true by default.


/// HL-Docs: ref:Bugfixes; issue:1312
/// Allow Stun Lance to target Mind Controlled units (currently on the same team as the Stun Lancer)
TargetPropertyCondition.TreatMindControlledSquadmateAsHostile = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add an empty line after this.

@Iridar Iridar added waiting-on-author A pull request is waiting on changes from the author and removed ready-to-review A pull request is ready to be reviewed labels Mar 2, 2024
@copyrite copyrite requested a review from Iridar March 2, 2024 09:49
@copyrite
Copy link
Contributor Author

copyrite commented Mar 2, 2024

I expect this PR to have the same merge conflict as #1313, the project file changes all landed on the same line in alphabetical order. Therefore I'd wait for that one to be merged first.

@copyrite
Copy link
Contributor Author

copyrite commented Mar 2, 2024

I'll also repeat here the consideration I brought up on Discord:

I'm not completely sure if there's a mod compatibility risk related to AI. For example, let's say there's a Sectoid and Stun Lancer in play. Sectoid Mind Controls an XCOM soldier. Before the fix, the same XCOM soldier is not a valid target for Stun Lance, but after the fix, it is.

Testing this in a vanilla game both with and without highlander, and verifying the available moves with x2allowselectall, I found that the Lancer will not attack the mind controlled target even if it has the option to do so. But if this feels like it'd cause unwanted changes downstream, I'm not bummed if this PR gets rejected. After all the situation where the player would get burnt by this interaction is very niche; take 2 MCs (one of which is a Lancer) and make them fight against each other.

@Iridar Iridar added ready-for-merge the pull request was reviewed and is ready to be merged. and removed waiting-on-author A pull request is waiting on changes from the author labels Mar 3, 2024
@Iridar Iridar merged commit 2372204 into X2CommunityCore:master Mar 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-basegame ready-for-merge the pull request was reviewed and is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stun Lancer's Stun Lance can't target MC'd or hacked units
2 participants