Follow-up Comment #1, patch #1140 (project wesnoth):

Ah good, finally someone fix this old bug :-)

Now, I couldn't find a translated string (in game or in help with a hypertext
link) where the word wrapping was broken (tested Japanese and Korean). I know
that the bug is there and I suppose that I could recreate a case, but it
would be simpler if you could point me a place(+version and language) or send
me a file allowing to test your patch (because I assume you already have
that)

Assuming that it works fine (and reading the code, all seems ok). I trust you
about the CJK characters involved. Fixing the code redundancy in help should
be possible (I didn't look into that yet).

A small remark, not sure if it's an official policy, but we generally don't
sign code. Instead you must add your name in data/core/about.cfg under
"Miscellaneous contributors". I hope that you don't mind having you name
(twice) in the real credits instead of buried in code ;-).

Now, if all works fine, it could be committed, but maybe I can interest you
to do some polishing and optimize things, because functions like
no_break_before() start to be rather long:

- maybe optimize a little these long tests by using some basic range test
(like checking if it's between 0x3000 and 0x30ff) before testing against
individual values. And don't forget short-circuit evaluation.
- Sorting these individual values may also help to see these ranges or check
if a specific test is already there.
- it would be also useful to avoid all these tests for non-CJK languages. So,
if I understand unicode range, maybe check first 'if (ch>0x2000 && (...'. It
would simplify things for these languages.
- those are all long inline functions, and are all expanded near
marked-up_text.cpp:403. Plus, at the same place, it seems that some tests are
done twice (for example, if the first no_break_after was true, then the next
break_after will always be false. Same for your is_cjk_char). I believe that
could be optimized by reorganizing the 'if' there. It would be nice if you
find a way to do it.

PS: I realize that such optimizations are a bit futile. I didn't tested if it
has a sensible impact (e.g. for rendering a big tooltip) and probably doesn't.
So, work on it only if you like efficient code.

    _______________________________________________________

Reply to this item at:

  <http://gna.org/patch/?1140>

_______________________________________________
  Message sent via/by Gna!
  http://gna.org/


_______________________________________________
Wesnoth-bugs mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-bugs

Reply via email to