Skip to content
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

Add Frames support to sequence Combine #17723

Merged
merged 1 commit into from Feb 27, 2020
Merged

Conversation

@reaperrr
Copy link
Contributor

reaperrr commented Feb 23, 2020

Interim solution for #10469 until the alternative proposed in #10469 (comment) gets implemented.

Copy link
Member

pchote left a comment

LGTM. Just one simplification:

OpenRA.Mods.Common/Graphics/DefaultSpriteSequence.cs Outdated Show resolved Hide resolved
@reaperrr reaperrr force-pushed the reaperrr:combineFrames branch from cf902ba to 73c5de7 Feb 23, 2020
@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Feb 23, 2020

Updated.

@pchote pchote added the PR: Needs +2 label Feb 23, 2020
@reaperrr reaperrr force-pushed the reaperrr:combineFrames branch from 73c5de7 to 3c6c881 Feb 23, 2020
@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Feb 23, 2020

Removed testcase, since both myself and @pchote confirmed that it works.

If the next reviewer still needs one, just add
Frames: 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0
to any of the flame- or chem- definitions in the TD infantry sequences and let the infantry fire in that direction, you'll see a reversed flame animation.

For regression testing, the only other cases in our mods that use Combine are the TD Guard Tower make anim and TS large visceroid attack, both to skip certain frames.

@pchote pchote added this to the Next Release milestone Feb 23, 2020
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Feb 23, 2020

This is a simple and low risk PR that naturally fits with the other Frames fixes, so adding to the milestone.

@Mailaender Mailaender merged commit eb007fc into OpenRA:bleed Feb 27, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Mailaender

This comment has been minimized.

Copy link
Member

Mailaender commented Feb 27, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.