Approximates chatmessage's height and width valueWe so back #11947
Conversation
|
I can't do pointers, we are not on 515 yet |
|
The testing evidence is completely meaningless since it doesn't show the case that this actually affects; the case where text wraps to a new line I also have concerns about users that dont have the small fonts font and have downloaded a replacement, since different fonts would break it again. It's not a huge case that's likely to cause a ton of issues, but I think there are much more elegant solutions overall, like giving them infinite width and manually controlling the line breaks ourselves by just adding them when we see enough characters. |
|
There are also so many hard coded values that it would become extremely annoying to modify text formatting if we merge this and it wouldn't be able to be ported to codebases that have different sized for italics easier. As I said above, manually inserting the new lines when we read enough chars is a much better solution and would be a lot more generic while never having any size inaccuracies or client calls |
|
Small fonts is included in windows. |
|
|
||
| if(has_icon) | ||
| value += CHAT_MESSAGE_ICON_SIZE | ||
| return list(value, string) |
There was a problem hiding this comment.
Allocating lists in the return value is a bad idea, since its on a datum you don't need a pointer because you can just modify the message as a class scoped variable.
|
|
||
| var/i = 1 | ||
| while(i <= length(string)) | ||
| var/list/letters = SSrunechat.letters[string[i]] |
There was a problem hiding this comment.
For how many times this gets called, using an assoc lookup is going to be slow. If you have a sequence of characters you can just use an int lookup with base offset
There was a problem hiding this comment.
If you are sure converting char to int will be faster than accessing this list I can change it
There was a problem hiding this comment.
- I would like some test cases. Show us what you have tested, the point of testmerges isn't to be the first point that something is given extensive testing. Note that there should also be some testing for sentances that have 3 lines in them, since you can never underestimate with 1 line strings. This is important, because if we overestimate a string, it really doesn't matter, but if we underestimate then we ruin our text and make things unreadable.
- This changes how much spacing there is between words, meaning that when this fails, it is significantly more noticable than when it fails on the server which is relatively hard to spot.
Running through some test cases, I had 1 case that showed an improvement though I need a more varied set of testing cases when it comes to word length.
In the case of a 3 line long string I tried, this new method underpredicts the length which results in text being culled, which is worse than the current method which has a tendancy to over-predict the amount of text. Try using the string W90 4i5ut9oap tia9pe4i tpoaiwW90 4i5ut9oap tia9pe4i tpoaiw
Failure on live server:
Failure when running this branch:
|
This might have been related to how font style is defined in skin.dmf maptext |
|
Feel free to reopen if this is actually going to be worked on |
Same as #6788 but I can't reopen it
About The Pull Request
This PR removes call to MeasureText(), effectively making creating chatmessages synchronous.
This fixes a few possible runtimes and weird undefined behaviors.
CHAR_WIDTHis (width/ 1000) *font_sizeWhere width is width as calculated by FontForge.width/ 1000 is the conversion between FontForge's em-size value and 1000 em-size is just 1 em.Check the FontForge's docs for better explanation. https://fontforge.org/docs/APPROX_HEIGHTisfont_size* 1.7Where 1.7 is approximation, based on comparing results of the MeasureText() and used font.The values from FontForge are saved in static list. If something isn't on the list we assume the worst and give it width of 1000.The values from font forge were inacurate. The approx width compared to measured one differed by about 20%.
The more naive solution has been implemented - measure cache of some sort. Every client should have the same viewport so the measurements between clients should be the same. Thus, once, we calculate values for every character and keep using them.
After testing it is about ~1% inacurate (which is huge improvement over 20%).
Additionally the values are cached now per client plus every uncached character will be calculated for all the clients.I changed my mind. I precalculated values for first 1500 characters for unicode. That should cover all the characters you would ever need. Anything that isn't included gets replaced with "?".
If you ever need a new character you can modify json with the value.
Why It's Good For The Game
Less chatmessage bugs and better performance.
Testing Photographs and Procedure
Screenshots&Videos
NEW CONTENT BELLOW
(old is still relevant though)
sunglasses emoji was replaced with "?"
Changelog
🆑
refactor: refactored how chatmessage width and height is calculated
/:cl: