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 different color picker preview actors per faction. #15735

Merged
merged 1 commit into from Nov 2, 2018

Conversation

Projects
None yet
3 participants
@IceReaper
Copy link
Contributor

IceReaper commented Oct 26, 2018

Implement optional per faction color picker preview actor support.

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 26, 2018

Checking for code style violations in OpenRA.Mods.Common...
OpenRA.Mods.Common/Widgets/Logic/ColorPickerLogic.cs:L28: [SP2100] Code line must be no longer than 180 characters (now 189).
OpenRA.Mods.Common/Widgets/Logic/ColorPickerLogic.cs:L31: [SP2100] Code line must be no longer than 180 characters (now 187).
make: *** [check] Error 1

@IceReaper IceReaper force-pushed the IceReaper:ColorPickerFaction branch from 533e974 to 1fee3ee Oct 26, 2018

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Oct 26, 2018

Nice. We can use this for D2K too.

actorType = "mcv";
if (initialFaction == null || !ChromeMetrics.TryGet("ColorPickerActorType-" + initialFaction, out actorType))
if (!ChromeMetrics.TryGet("ColorPickerActorType", out actorType))
actorType = "mcv";

This comment has been minimized.

@pchote

pchote Oct 26, 2018

Member

IMO this would be a good time to remove this mod-specific default, and require all mods to list it explicitly.

This comment has been minimized.

@IceReaper

IceReaper Oct 26, 2018

Contributor

removing "mcv" makes sense, but the default "ColorPickerActorType" is required for random faction or the color picker in the settings.

This comment has been minimized.

@pchote

pchote Oct 26, 2018

Member

Yup, indeed.

@MustaphaTR
Copy link
Member

MustaphaTR left a comment

Works as described. 👍 after Pchote's comment is adressed.

@IceReaper IceReaper force-pushed the IceReaper:ColorPickerFaction branch from 1fee3ee to 1a348a5 Oct 26, 2018

@IceReaper IceReaper force-pushed the IceReaper:ColorPickerFaction branch from 1a348a5 to a25db9c Oct 30, 2018

@pchote

pchote approved these changes Nov 2, 2018

@pchote pchote merged commit 89e3b62 into OpenRA:bleed Nov 2, 2018

2 checks passed

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

@IceReaper IceReaper deleted the IceReaper:ColorPickerFaction branch Nov 6, 2018

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