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

Polish TS deployer animations #13210

Merged
merged 3 commits into from Apr 30, 2017

Conversation

Projects
None yet
4 participants
@pchote
Member

pchote commented Apr 29, 2017

This fixes the jerky size and position jumps when deploying the tick tank and mobile sensor array.

@@ -175,6 +175,8 @@ TTNK:
RequiresCondition: undeployed
RevealOnFire:
ArmamentNames: primary, deployed
RenderVoxels:
Scale: 11.5

This comment has been minimized.

@reaperrr

reaperrr Apr 29, 2017

Contributor

Have you checked how it looks with 12.0?

I've suspected for some time that our default 10.0 is a bit too small, because the voxel-internal scale value that ours probably multiplies with, defaults to 0.083333333~ for all Westwood voxels, so in theory Scale: 12.0 should give us pretty much the original sizes, disregarding potential rounding errors.

@reaperrr

reaperrr Apr 29, 2017

Contributor

Have you checked how it looks with 12.0?

I've suspected for some time that our default 10.0 is a bit too small, because the voxel-internal scale value that ours probably multiplies with, defaults to 0.083333333~ for all Westwood voxels, so in theory Scale: 12.0 should give us pretty much the original sizes, disregarding potential rounding errors.

This comment has been minimized.

@pchote

pchote Apr 29, 2017

Member

12 is slightly too big for these two at least. How about I change the default scale to 12, then keep the 11.5 here?

@pchote

pchote Apr 29, 2017

Member

12 is slightly too big for these two at least. How about I change the default scale to 12, then keep the 11.5 here?

This comment has been minimized.

@reaperrr

reaperrr Apr 29, 2017

Contributor

Fine with me, though I guess we should compare some of the other voxels via screenshots, maybe 11.5 gives the best result for them, too.

@reaperrr

reaperrr Apr 29, 2017

Contributor

Fine with me, though I guess we should compare some of the other voxels via screenshots, maybe 11.5 gives the best result for them, too.

This comment has been minimized.

@pchote

pchote Apr 29, 2017

Member

12 gives the best result for the MCV, so I have set that to the default.

@pchote

pchote Apr 29, 2017

Member

12 gives the best result for the MCV, so I have set that to the default.

This comment has been minimized.

@reaperrr

reaperrr Apr 29, 2017

Contributor

Yeah, I've come to the same conclusion with buggies and recon bikes.

@reaperrr

reaperrr Apr 29, 2017

Contributor

Yeah, I've come to the same conclusion with buggies and recon bikes.

@Phrohdoh

This comment has been minimized.

Show comment
Hide comment
@Phrohdoh

Phrohdoh Apr 29, 2017

Member

This changes default values without an upgrade rule (or without even alerting game makers).

Member

Phrohdoh commented Apr 29, 2017

This changes default values without an upgrade rule (or without even alerting game makers).

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Apr 29, 2017

Member

Indeed, but as mentioned above the old default value was a bug. Any C&C-style gen2 mod will want to be using the new value, and we don't make a point of claiming or aiming the gen2 mod features to be a stable target. IMO that argues against having an explicit upgrade rule, but we should definitely mention it in the changelog entry.

Member

pchote commented Apr 29, 2017

Indeed, but as mentioned above the old default value was a bug. Any C&C-style gen2 mod will want to be using the new value, and we don't make a point of claiming or aiming the gen2 mod features to be a stable target. IMO that argues against having an explicit upgrade rule, but we should definitely mention it in the changelog entry.

@GraionDilach

👍

@reaperrr reaperrr merged commit 1349dfb into OpenRA:bleed Apr 30, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@reaperrr
Contributor

reaperrr commented Apr 30, 2017

@pchote pchote deleted the pchote:polish-deployers branch May 7, 2017

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