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

D2K - Adjust Building Selection Boxes #14405

Merged
merged 1 commit into from Dec 12, 2017

Conversation

Projects
None yet
5 participants
@MustaphaTR
Member

MustaphaTR commented Nov 22, 2017

Starport, IX Research Center and Repair Pad had smaller selection box than their actual size. This fixes that and also some other adjustments.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 26, 2017

Contributor

Note that these (except the Radar) were intentional changes so their selectable area doesn't overlap units and turrets so much when those are placed on the building's passable corner cells.
Not sure if that is still as much of an issue as back then (iirc @RoosterDragon fixed it so the actor whose center is closest to the cursor is selected), just wanted to point this out in case anyone else reviews this before me.

Contributor

reaperrr commented Nov 26, 2017

Note that these (except the Radar) were intentional changes so their selectable area doesn't overlap units and turrets so much when those are placed on the building's passable corner cells.
Not sure if that is still as much of an issue as back then (iirc @RoosterDragon fixed it so the actor whose center is closest to the cursor is selected), just wanted to point this out in case anyone else reviews this before me.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 8, 2017

Member

This conflicts with my pending selection/rendering bounds PR, so adding the dependencies label.

Member

pchote commented Dec 8, 2017

This conflicts with my pending selection/rendering bounds PR, so adding the dependencies label.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Dec 11, 2017

Contributor

Needs rebase now.

Contributor

reaperrr commented Dec 11, 2017

Needs rebase now.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 11, 2017

Member

Rebased.

Member

MustaphaTR commented Dec 11, 2017

Rebased.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 11, 2017

Member

Imho the Starport's bounds extend a bit too far down. Also what about Palace, Construction Yard, Outpost and High Tech Factory? (Not real blockers though.)

Member

abcdefg30 commented Dec 11, 2017

Imho the Starport's bounds extend a bit too far down. Also what about Palace, Construction Yard, Outpost and High Tech Factory? (Not real blockers though.)

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 12, 2017

Member

This PR is meamt to adjust them so they all match the footprint, not artwork. Most of other buildings already have them like that.

Member

MustaphaTR commented Dec 12, 2017

This PR is meamt to adjust them so they all match the footprint, not artwork. Most of other buildings already have them like that.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 12, 2017

Member

Ok. 👍 then I guess.

Member

abcdefg30 commented Dec 12, 2017

Ok. 👍 then I guess.

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Dec 12, 2017

Contributor

Couldn't find any issues, so 👍

Without PR
image
With PR
image
Contributor

ltem commented Dec 12, 2017

Couldn't find any issues, so 👍

Without PR
image
With PR
image
@ltem

ltem approved these changes Dec 12, 2017

@ltem ltem merged commit fd1aa07 into OpenRA:bleed Dec 12, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@ltem
Contributor

ltem commented Dec 12, 2017

@MustaphaTR MustaphaTR deleted the MustaphaTR:d2k-selection-boxes branch Dec 17, 2017

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