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

Addition of customizability to Hovers #16261

Merged
merged 1 commit into from Mar 22, 2019

Conversation

@CDVoidwalker
Copy link
Contributor

commented Mar 3, 2019

Adds features requested by #16230

@CDVoidwalker CDVoidwalker force-pushed the CDVoidwalker:hovers branch 2 times, most recently from 4542c9e to 1a8c4a2 Mar 3, 2019

OpenRA.Mods.Common/Traits/Render/Hovers.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Render/Hovers.cs Outdated Show resolved Hide resolved

@CDVoidwalker CDVoidwalker force-pushed the CDVoidwalker:hovers branch from 03279b4 to 18def20 Mar 10, 2019

@CDVoidwalker

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2019

applied review notes

OpenRA.Mods.Common/Traits/Render/Hovers.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Render/Hovers.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Render/Hovers.cs Outdated Show resolved Hide resolved

@CDVoidwalker CDVoidwalker force-pushed the CDVoidwalker:hovers branch 2 times, most recently from 78d4d8a to 16b2850 Mar 11, 2019

@CDVoidwalker

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

Force pushed twice as I noticed tabs instead of spaces

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

This trait doesn't affect the selection box, so increasing the height makes it look quite bad:

Screenshot 2019-03-11 at 21 24 34

Can you please increase the height of the selection box so that it looks reasonable both when the unit is active, and when it's EMPed?

@CDVoidwalker

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

hovermrls
I think that looks fair, added image so it's not necessary to test it ingame.

@CDVoidwalker CDVoidwalker force-pushed the CDVoidwalker:hovers branch from d1f57ed to cb98fed Mar 21, 2019

@CDVoidwalker

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

updated to reaperr's comment

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

I think now it would be better to squash the commits (and use the 2nd commits' title, since the first commits' original purpose no longer applies).
👍 after that.

@CDVoidwalker CDVoidwalker force-pushed the CDVoidwalker:hovers branch from cb98fed to 775bdd6 Mar 21, 2019

@CDVoidwalker CDVoidwalker force-pushed the CDVoidwalker:hovers branch from 775bdd6 to 17693cb Mar 21, 2019

@CDVoidwalker

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

done the touchup

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

The changes since @abcdefg30's +1 were just polish, so lgtm.

@reaperrr reaperrr merged commit 6c9d961 into OpenRA:bleed Mar 22, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.