Added a comment to nit 2.

As for testing, create a long Latin savefile name, then grab the long Arabic 
savefile name from the bug. all file- and mapnames should be properly aligned 
in the Load Game and New Game screens both when you pick a Latin or an Arabic 
language.

I know that navigating through the UI in a language yo can't read can be a bit 
scary, but Arabic is only partially translated, and your brain should have some 
muscle memory as to where the buttons are.

Diff comments:

> 
> === modified file 'src/ui_basic/table.cc'
> --- src/ui_basic/table.cc     2015-12-31 08:36:41 +0000
> +++ src/ui_basic/table.cc     2016-01-06 12:13:29 +0000
> @@ -365,8 +374,25 @@
>                               text_width = text_width + m_scrollbar->get_w();
>                       }
>                       UI::correct_for_align(alignment, text_width, 
> entry_text_im->height(), &point);
> -                     // Crop to column width
> -                     dst.blitrect(point, entry_text_im, Rect(0, 0, curw - 
> picw, lineheight));
> +
> +                     // Crop to column width while blitting
> +                     if (((static_cast<int32_t>(curw) + picw) < text_width)) 
> {

The type mismatch we have here is that coordinates are int because they can 
become negative, and width/height are uint. But I agree that we shoudln't run 
into any overflow issues if I change the type.

> +                             // Fix positioning for BiDi languages.
> +                             if (UI::g_fh1->fontset().is_rtl()) {
> +                                     point.x = alignment & Align_Right ? 
> curx : curx + picw;
> +                             }
> +                             // We want this always on, e.g. for mixed 
> language savegame filenames
> +                             if 
> (i18n::has_rtl_character(entry_string.c_str(), 20)) { // Restrict check for 
> efficiency
> +                                     dst.blitrect(point,
> +                                                                      
> entry_text_im,
> +                                                                      
> Rect(text_width - curw + picw, 0, text_width, lineheight));
> +                             }
> +                             else {
> +                                     dst.blitrect(point, entry_text_im, 
> Rect(0, 0, curw - picw, lineheight));
> +                             }
> +                     } else {
> +                             dst.blitrect(point, entry_text_im, Rect(0, 0, 
> curw - picw, lineheight));
> +                     }
>                       curx += curw;
>               }
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/table_align/+merge/279685
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/table_align.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to