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

Add support for multiple (External)Capturable types. #14230

Merged
merged 3 commits into from Nov 4, 2017

Conversation

Projects
None yet
4 participants
@pchote
Member

pchote commented Oct 21, 2017

Also replaces the ally/neutral/enemy flags with a Stance enum.

@penev92

👍

if (allowNeutral != null)
{
if (FieldLoader.GetValue<bool>("AllowNeutral", allowNeutral.Value.Value))
stance |= Stance.Neutral;

This comment has been minimized.

@abcdefg30

abcdefg30 Oct 23, 2017

Member

Does this even make sense, as you initialized stance with Stance.Neutral | Stance.Enemy (same for "AllowEnemies")?

@abcdefg30

abcdefg30 Oct 23, 2017

Member

Does this even make sense, as you initialized stance with Stance.Neutral | Stance.Enemy (same for "AllowEnemies")?

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 3, 2017

Contributor

Looks good to me, but I'd like to see @abcdefg30's question at least answered before we merge this.

Contributor

reaperrr commented Nov 3, 2017

Looks good to me, but I'd like to see @abcdefg30's question at least answered before we merge this.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 4, 2017

Contributor

[15:43] <+pchote> reaperrr: it doesn't matter either way
[15:44] <+pchote> the behaviour in both cases is the same, its just a tradeoff of having easier to read but slightly redundant code vs more efficient but less obvious

Contributor

reaperrr commented Nov 4, 2017

[15:43] <+pchote> reaperrr: it doesn't matter either way
[15:44] <+pchote> the behaviour in both cases is the same, its just a tradeoff of having easier to read but slightly redundant code vs more efficient but less obvious

@reaperrr reaperrr merged commit faf2634 into OpenRA:bleed Nov 4, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pchote pchote deleted the pchote:multiple-capturable-types branch Apr 28, 2018

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