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

Sprite Font gracious fallback #14199

Merged
merged 1 commit into from Nov 1, 2017

Conversation

Projects
None yet
5 participants
@AndriiYukhymchak
Contributor

AndriiYukhymchak commented Oct 17, 2017

Should fix #14176 . Can't test if this fixes issue for Linux, but at least it can graciously fail with bogus glyphs

@AndriiYukhymchak

This comment has been minimized.

Show comment
Hide comment
@AndriiYukhymchak

AndriiYukhymchak Oct 17, 2017

Contributor

Whoops. I'll get back to it tomorrow and fix the merge screwups

Contributor

AndriiYukhymchak commented Oct 17, 2017

Whoops. I'll get back to it tomorrow and fix the merge screwups

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Oct 17, 2017

Member

Just in case you need it, here's a cheat sheet for fixing the merge issues:

git fetch upstream
git checkout FontCrashFix
git rebase -i upstream/bleed

This will pop up an editor with a list of commits - make sure it looks sane, then change the "pick" before the "moved logic to DrawText" to "squash", which tells git to squash the two commits together, then save and exit the editor to continue

git push origin +FontCrashFix
Member

pchote commented Oct 17, 2017

Just in case you need it, here's a cheat sheet for fixing the merge issues:

git fetch upstream
git checkout FontCrashFix
git rebase -i upstream/bleed

This will pop up an editor with a list of commits - make sure it looks sane, then change the "pick" before the "moved logic to DrawText" to "squash", which tells git to squash the two commits together, then save and exit the editor to continue

git push origin +FontCrashFix
@RoosterDragon

This comment has been minimized.

Show comment
Hide comment
@RoosterDragon

RoosterDragon Oct 18, 2017

Member

Discussion point: Should we attempt to use a fallback character of some kind, e.g. a '?' or the replacement glyph?

Member

RoosterDragon commented Oct 18, 2017

Discussion point: Should we attempt to use a fallback character of some kind, e.g. a '?' or the replacement glyph?

@AndriiYukhymchak

This comment has been minimized.

Show comment
Hide comment
@AndriiYukhymchak

AndriiYukhymchak Oct 18, 2017

Contributor

@pchote Fixup done, sorry for the hussle

Contributor

AndriiYukhymchak commented Oct 18, 2017

@pchote Fixup done, sorry for the hussle

@AndriiYukhymchak

This comment has been minimized.

Show comment
Hide comment
@AndriiYukhymchak

AndriiYukhymchak Oct 18, 2017

Contributor

@RoosterDragon don't know honestly, let maints decide. This can be easily done tho, unless someone picks character that for some reason is not included in one of the glyph libs (or whatever is doing this)

Contributor

AndriiYukhymchak commented Oct 18, 2017

@RoosterDragon don't know honestly, let maints decide. This can be easily done tho, unless someone picks character that for some reason is not included in one of the glyph libs (or whatever is doing this)

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Oct 18, 2017

Member

Discussion point: Should we attempt to use a fallback character of some kind, e.g. a '?' or the replacement glyph?

I don't have a strong opinion either way.

Member

pchote commented Oct 18, 2017

Discussion point: Should we attempt to use a fallback character of some kind, e.g. a '?' or the replacement glyph?

I don't have a strong opinion either way.

@pchote

pchote approved these changes Oct 18, 2017

Code looks fine, and works as advertised (tested using a dummy exception) 👍

@abcdefg30 abcdefg30 merged commit 636a9a7 into OpenRA:bleed Nov 1, 2017

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.

Show comment
Hide comment
@abcdefg30
Member

abcdefg30 commented Nov 1, 2017

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