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 usage of different palette for WithSpriteTurret #15525

Merged
merged 1 commit into from Sep 29, 2018

Conversation

Projects
None yet
5 participants
@IceReaper
Copy link
Contributor

IceReaper commented Aug 15, 2018

In KKnD i have a different palette per unit Spritesheet. Also a vehicle "base" is a different Spritesheet as its turret. Default behavior is to use the base Palette on the Turret. This PR allows turrets to use a different palette than its base actor.

[PaletteReference] public readonly string Palette = null;

[Desc("Custom PlayerColorPalette: BaseName")]
[PaletteReference(true)] public readonly string PlayerPalette = null;

This comment has been minimized.

@abcdefg30

abcdefg30 Aug 16, 2018

Member

I think we just expose a IsPlayerPalette boolean elsewhere. Any specific reason you deviated from this here?

This comment has been minimized.

@GraionDilach

GraionDilach Aug 16, 2018

Contributor

This is probably copied from RenderSprites tbh.

This comment has been minimized.

@IceReaper

IceReaper Aug 20, 2018

Contributor

Yup it is.

This comment has been minimized.

@GraionDilach

GraionDilach Aug 20, 2018

Contributor

Yeah, and that makes sense on RenderSprites - not over here though.

This comment has been minimized.

@pchote

pchote Sep 23, 2018

Member

I agree that this should be consistent with

[Desc("Custom palette name")]
[PaletteReference("IsPlayerPalette")] public readonly string Palette = null;
[Desc("Custom palette is a player palette BaseName")]
public readonly bool IsPlayerPalette = false;

@IceReaper can you please change this?

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Aug 16, 2018

It might be better longterm if we create a Repalettable TraitInfoInterface and longterm shift all With*Overlays/Animations/WithSprite* etc if necessary (WithSpriteBody should not be needed).

@IceReaper IceReaper force-pushed the IceReaper:WithSpriteTurret.Palette branch 3 times, most recently from 7cd9715 to 02a98f3 Sep 27, 2018

@@ -25,6 +25,12 @@ public class WithSpriteTurretInfo : ConditionalTraitInfo, IRenderActorPreviewSpr
[Desc("Sequence name to use")]
[SequenceReference] public readonly string Sequence = "turret";

[Desc("Custom palette name")]
[PaletteReference] public readonly string Palette = null;

This comment has been minimized.

@pchote

pchote Sep 28, 2018

Member

This needs to be [PaletteReference("IsPlayerPalette")] or the linter will get confused

@IceReaper IceReaper force-pushed the IceReaper:WithSpriteTurret.Palette branch from 02a98f3 to f953bf6 Sep 28, 2018

@IceReaper

This comment has been minimized.

Copy link
Contributor

IceReaper commented Sep 28, 2018

Updated! :)

@pchote pchote added the PR: Needs +2 label Sep 28, 2018

@IceReaper IceReaper force-pushed the IceReaper:WithSpriteTurret.Palette branch from f953bf6 to 8ceec6f Sep 29, 2018

@reaperrr reaperrr merged commit 28623ce into OpenRA:bleed Sep 29, 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:WithSpriteTurret.Palette branch Sep 29, 2018

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