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

Added tunnel top rendering #9203

Merged
merged 1 commit into from Sep 14, 2015

Conversation

Projects
None yet
5 participants
@Mailaender
Member

Mailaender commented Aug 30, 2015

Before:
image

After:
image

Only polishes the rendering. Does not add support for http://modenc.renegadeprojects.com/Tubes.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 4, 2015

Member

The viewport rendering problem of actors spanning multiple cells on isometric terrain is unrelated to this.

Member

Mailaender commented Sep 4, 2015

The viewport rendering problem of actors spanning multiple cells on isometric terrain is unrelated to this.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Sep 4, 2015

Member

Regular buildings ingame do not show this problem, so 👎 to removing the label and merging a broken feature.

Member

pchote commented Sep 4, 2015

Regular buildings ingame do not show this problem, so 👎 to removing the label and merging a broken feature.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 5, 2015

Member

I probably need to rethink this then. Might need an TerrainAnimationLayer and do it like the original.

Member

Mailaender commented Sep 5, 2015

I probably need to rethink this then. Might need an TerrainAnimationLayer and do it like the original.

@Mailaender Mailaender closed this Sep 5, 2015

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Sep 5, 2015

Member

There is a simpler fix: Add the CustomSelectionSize trait with appropriate bounds to the tunnel actors ( AutoSelectionSize doesn't generate a large enough rectangle, so there is still flickering).

The viewport rendering problem of actors spanning multiple cells on isometric terrain is unrelated to this.

It sounds like your other recent decoration PRs have also missed these important traits.

Member

pchote commented Sep 5, 2015

There is a simpler fix: Add the CustomSelectionSize trait with appropriate bounds to the tunnel actors ( AutoSelectionSize doesn't generate a large enough rectangle, so there is still flickering).

The viewport rendering problem of actors spanning multiple cells on isometric terrain is unrelated to this.

It sounds like your other recent decoration PRs have also missed these important traits.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 5, 2015

Member

When we last tested #9130 with the moving viewport the BodyOrientation and sprite rendering refactor that backfired a lot wasn't in so things may have changed again.

Member

Mailaender commented Sep 5, 2015

When we last tested #9130 with the moving viewport the BodyOrientation and sprite rendering refactor that backfired a lot wasn't in so things may have changed again.

@Mailaender Mailaender reopened this Sep 5, 2015

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 5, 2015

Member

Tweaked the tunnel tops disappearing too early by introducing CustomSelectionSize bounds.

Member

Mailaender commented Sep 5, 2015

Tweaked the tunnel tops disappearing too early by introducing CustomSelectionSize bounds.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 12, 2015

Member

Rebased.

Member

Mailaender commented Sep 12, 2015

Rebased.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Sep 12, 2015

Member

Looks good to me. 👍 /

Member

abcdefg30 commented Sep 12, 2015

Looks good to me. 👍 /

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 14, 2015

Contributor

👍

Contributor

reaperrr commented Sep 14, 2015

👍

reaperrr added a commit that referenced this pull request Sep 14, 2015

@reaperrr reaperrr merged commit 605ec81 into OpenRA:bleed Sep 14, 2015

2 checks passed

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

@Mailaender Mailaender deleted the Mailaender:tunnel-rendering branch Sep 14, 2015

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