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

Refactored WaterPaletteRotation into RotationPaletteEffect #9103

Merged
merged 5 commits into from Sep 6, 2015

Conversation

Projects
None yet
8 participants
@reaperrr
Contributor

reaperrr commented Aug 18, 2015

Moves rotation base index from tileset to palette effect.
Unhardcodes some other aspects.

Fixes #8872.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Aug 18, 2015

Member

Nice. This has been on my extended todo list for ages.

Member

pchote commented Aug 18, 2015

Nice. This has been on my extended todo list for ages.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Aug 19, 2015

Contributor

Updated.
Also added 'water' to @ keys to make it more clear that the effect is for water in RA and TD (as the trait name itself is now generic).

Contributor

reaperrr commented Aug 19, 2015

Updated.
Also added 'water' to @ keys to make it more clear that the effect is for water in RA and TD (as the trait name itself is now generic).

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 19, 2015

Member

Nice cleanup and fix. 👍

Member

Mailaender commented Aug 19, 2015

Nice cleanup and fix. 👍

@GraionDilach

This comment has been minimized.

Show comment
Hide comment
@GraionDilach

GraionDilach Aug 19, 2015

Contributor

TBH, I'd be happier if this would have a positive logic instead of the negative logic it has right now in the long run, but I guess I'm a minority here.

Contributor

GraionDilach commented Aug 19, 2015

TBH, I'd be happier if this would have a positive logic instead of the negative logic it has right now in the long run, but I guess I'm a minority here.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Aug 23, 2015

Contributor

I've tried to add a positive logic, but somehow it doesn't work as it should (likely something simple and silly). If someone helps me debugging and fixing it, I'll include it, otherwise I'd rather leave that for later when RA2 or some third-party mod needs it.

Contributor

reaperrr commented Aug 23, 2015

I've tried to add a positive logic, but somehow it doesn't work as it should (likely something simple and silly). If someone helps me debugging and fixing it, I'll include it, otherwise I'd rather leave that for later when RA2 or some third-party mod needs it.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 23, 2015

Member

You probably also have to set the Exclude* arrays to null and check against that to make it work. As all palette effects currently use negative filters this is just fine and consistent for the moment.

Member

Mailaender commented Aug 23, 2015

You probably also have to set the Exclude* arrays to null and check against that to make it work. As all palette effects currently use negative filters this is just fine and consistent for the moment.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Aug 24, 2015

Contributor

Updated, now supports both excluding and explicitly loading palettes/tilesets.

Contributor

reaperrr commented Aug 24, 2015

Updated, now supports both excluding and explicitly loading palettes/tilesets.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 29, 2015

Member

Needs a rebase.

Member

Mailaender commented Aug 29, 2015

Needs a rebase.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Aug 29, 2015

Contributor

Rebased.

Contributor

reaperrr commented Aug 29, 2015

Rebased.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 3, 2015

Contributor

Rebased.

Contributor

reaperrr commented Sep 3, 2015

Rebased.

@Unit158

This comment has been minimized.

Show comment
Hide comment
@Unit158

Unit158 Sep 4, 2015

Contributor

Simple enough :+0.5: I have yet to test it.

Contributor

Unit158 commented Sep 4, 2015

Simple enough :+0.5: I have yet to test it.

@RoosterDragon

This comment has been minimized.

Show comment
Hide comment
@RoosterDragon

RoosterDragon Sep 4, 2015

Member

👍 after addressing the minor refactorings.

Member

RoosterDragon commented Sep 4, 2015

👍 after addressing the minor refactorings.

reaperrr added some commits Aug 18, 2015

Refactored WaterPaletteRotation
Moved RotationBase from tileset definition to WaterPaletteRotation effect.
Unhardcoded rotation range and rotation rate.
Added possibility to exclude Tilesets from effect.

Fixed RA water palette rotation for actors on desert maps (#8872).
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 6, 2015

Contributor

Updated and rebased.

Contributor

reaperrr commented Sep 6, 2015

Updated and rebased.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 6, 2015

Contributor

Updated.

Contributor

reaperrr commented Sep 6, 2015

Updated.

@RoosterDragon

This comment has been minimized.

Show comment
Hide comment
@RoosterDragon
Member

RoosterDragon commented Sep 6, 2015

👍

RoosterDragon added a commit that referenced this pull request Sep 6, 2015

Merge pull request #9103 from reaperrr/water-rot
Refactored WaterPaletteRotation into RotationPaletteEffect

@RoosterDragon RoosterDragon merged commit 662077a into OpenRA:bleed Sep 6, 2015

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@Mailaender
Member

Mailaender commented Sep 6, 2015

@reaperrr reaperrr deleted the reaperrr:water-rot branch Sep 23, 2015

Name: player
ShadowIndex: 3,4

This comment has been minimized.

@Mailaender

Mailaender Nov 14, 2015

Member

I think this change is responsible for #9989. Can you elaborate why this was necessary?

@Mailaender

Mailaender Nov 14, 2015

Member

I think this change is responsible for #9989. Can you elaborate why this was necessary?

This comment has been minimized.

@reaperrr

reaperrr Nov 14, 2015

Contributor

I just didn't know this served a purpose, since only index 4 is used for shadows in the original.

@reaperrr

reaperrr Nov 14, 2015

Contributor

I just didn't know this served a purpose, since only index 4 is used for shadows in the original.

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