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

Fix Cruiser's Turret #14562

Merged
merged 1 commit into from Dec 23, 2017

Conversation

Projects
None yet
6 participants
@MustaphaTR
Member

MustaphaTR commented Dec 22, 2017

Fixes #14550.

@penev92 penev92 added this to the Next release milestone Dec 22, 2017

@GraionDilach

This comment has been minimized.

Show comment
Hide comment
@GraionDilach

GraionDilach Dec 22, 2017

Contributor

Do you really need both lines? Either would be already enough AFAIK.

Contributor

GraionDilach commented Dec 22, 2017

Do you really need both lines? Either would be already enough AFAIK.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 22, 2017

Member

Which both lines? Armament:>Name: is removed, not added.

Member

MustaphaTR commented Dec 22, 2017

Which both lines? Armament:>Name: is removed, not added.

@GraionDilach

This comment has been minimized.

Show comment
Hide comment
@GraionDilach

GraionDilach Dec 22, 2017

Contributor

I meant both lines of the diff. If you're removing the secondary Name from the Armament, why add secondary to AttackTurreted? If you're adding secondary to AttackTurreted, why revert the secondary cannon's armament name to primary?

Contributor

GraionDilach commented Dec 22, 2017

I meant both lines of the diff. If you're removing the secondary Name from the Armament, why add secondary to AttackTurreted? If you're adding secondary to AttackTurreted, why revert the secondary cannon's armament name to primary?

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 22, 2017

Member

AttackTurreted no longer checks Armament:>Name: (so it is unnecessary now), but Turreted:>Turret: and AttackTurreted:>Turret: defatults to just primary.

Member

MustaphaTR commented Dec 22, 2017

AttackTurreted no longer checks Armament:>Name: (so it is unnecessary now), but Turreted:>Turret: and AttackTurreted:>Turret: defatults to just primary.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 22, 2017

Member

AttackTurreted no longer checks Armament:>Name:

protected virtual Func<IEnumerable<Armament>> InitializeGetArmaments(Actor self)
{
var armaments = self.TraitsImplementing<Armament>()
.Where(a => Info.Armaments.Contains(a.Info.Name)).ToArray();
return () => armaments;
}

?

Member

pchote commented Dec 22, 2017

AttackTurreted no longer checks Armament:>Name:

protected virtual Func<IEnumerable<Armament>> InitializeGetArmaments(Actor self)
{
var armaments = self.TraitsImplementing<Armament>()
.Where(a => Info.Armaments.Contains(a.Info.Name)).ToArray();
return () => armaments;
}

?

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 22, 2017

Member

Ok, i'm not sure what is going on here. But this is working fine.

Is what wanted here to keep secondary Name for Armament?

Member

MustaphaTR commented Dec 22, 2017

Ok, i'm not sure what is going on here. But this is working fine.

Is what wanted here to keep secondary Name for Armament?

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 22, 2017

Member

That is the simplest approach, yeah.

Member

pchote commented Dec 22, 2017

That is the simplest approach, yeah.

@reaperrr reaperrr merged commit 7bf3d5d into OpenRA:bleed Dec 23, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@reaperrr
Contributor

reaperrr commented Dec 23, 2017

@MustaphaTR MustaphaTR deleted the MustaphaTR:ra-fix-cruiser branch Dec 23, 2017

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