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

D2K - Subfaction FactionSuffix #14457

Merged
merged 1 commit into from Dec 11, 2017

Conversation

Projects
None yet
4 participants
@MustaphaTR
Member

MustaphaTR commented Nov 30, 2017

Before this PR making a human player one of the subfactions crashes the game on start.

I made it so using FactionSuffix- those sides use the same UI with the side they share building artwork.

Note that currently on OpenRA, Fremen are using Harkonnen structure artwork, this is wrong and i'll fix it with another PR.

Testcase enables the subfactions so you can test.

Make subfactions use UI for their main faction
So it won't crash if a human player uses them.
@ltem

I didn't find any issues while testing it (keeping the follow up PRs in mind). The implementation is similar to Red Alert (https://github.com/OpenRA/OpenRA/blob/bleed/mods/ra/metrics.yaml), which I used for comparison.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 10, 2017

Member

The announcer and unit audio doesn't work for the new factions. The smugglers starport and mercenary heavy factory build icons are also combat tanks?

Member

pchote commented Dec 10, 2017

The announcer and unit audio doesn't work for the new factions. The smugglers starport and mercenary heavy factory build icons are also combat tanks?

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Dec 10, 2017

Contributor

Audio and these building icons will be fixed in the follow up PR #14458

Contributor

ltem commented Dec 10, 2017

Audio and these building icons will be fixed in the follow up PR #14458

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 10, 2017

Member

Ok, I see. In that case 👍.

@MustaphaTR can you please drop the testcase commit now?

Member

pchote commented Dec 10, 2017

Ok, I see. In that case 👍.

@MustaphaTR can you please drop the testcase commit now?

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 11, 2017

Member

Removed the TESTCASE.

Member

MustaphaTR commented Dec 11, 2017

Removed the TESTCASE.

@ltem

ltem approved these changes Dec 11, 2017

@abcdefg30 abcdefg30 merged commit c69df4e into OpenRA:bleed Dec 11, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 11, 2017

Member

Changelog (as suggested on IRC.)

Member

abcdefg30 commented Dec 11, 2017

Changelog (as suggested on IRC.)

@MustaphaTR MustaphaTR deleted the MustaphaTR:d2k-subfaction-radar-images branch Dec 12, 2017

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