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

#17018 Gets rid of cutoff line in chat window #17025

Merged
merged 1 commit into from Sep 6, 2019

Conversation

@PurityLake
Copy link
Contributor

commented Sep 1, 2019

Fixes #17018

The fix involves taking away the offset of a SpriteFont object to basically cut off the top part of the chat window so that lower hanging letters like 'y' are considered outside of the chat window.

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

Hi! Thanks for the PR.

One problem i noticed during review is that the number of lines shown when in chat mode and the display mode is not the same.

See this gif.

chatdisp

@PurityLake

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

@teinarss despite reverting the changes to the original code that Batlefield Control chat doesn't show up regarcdless

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

This is how it looks on bleed
chat2

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

Also I can recommend commenting out the lines in ChatDisplayWidget.Tick() when fiddling with this.

@PurityLake

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

I'm not sure if I'm doing this pull request right but I didn't use the bleed branch for this PR. Should I redo this pull request and push to bleed instead?

Also this is my chat:

Capture

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

I'm not sure if I'm doing this pull request right but I didn't use the bleed branch for this PR. Should I redo this pull request and push to bleed instead?

Also this is my chat:

Capture

Shouldn't be needed, did just check with the parent commit of this PR and it looks the same as i did for be on bleed.

@PurityLake

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

I know this sounds cliché but it seems to work on my machine. I've checked out debug messages and they seem to not be clipped either:

Capture

Capture1

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

I see now that you tested in RA and I tested it in TD (where i have the problem, in RA it works fine),

@PurityLake

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

My view from TD:

td

@teinarss
Copy link
Contributor

left a comment

You are totally correct, ill blame the lack of sleep for my stupidity.

@abcdefg30 abcdefg30 merged commit aff3bf3 into OpenRA:bleed Sep 6, 2019

2 checks passed

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

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.