MetalES / Project-Zelda

Project Zelda, made using Solarus
12 stars 1 forks source link

Dialog Box: Wrong position of colored / variable parsing text #46

Open MetalES opened 7 years ago

MetalES commented 7 years ago

This does the same thing on Wrightmat's project.

This might be due to the surface. the surface is recreated to color the text, and the X position might be wrong. This is fixable with small fonts, that's not a problem, but using a Japanese / Chinese font, or a clearer font, this could happen.

There is no space between the color application

This is the Japanese text as it appear in the text file.

あなたは$[red]ダンジョン地図を入手します$[white]!$0
今より良いあなたを識別するために使用し
ます!

This is how it is rendered. sans titre 4

The same thing happen if you parse a variable from the script (aka recreating a surface to apply a text).

This is the Japanese text in the text file

$(item_1)を押すと、画面の右下
に現在のビューの部屋を持っています。

Rendering sans titre 6

Using the same font, for the French Text, this happen sans titre 9

Using the correct font used for the French language, the text placement is correct sans titre 11

But this is the text in the text file, notice the spaces used to render it correctly (3 spaces between "la" and "Carte")

Vous obtenez la  $[red] Carte du Donjon$[white]   !$0
Elle vous servira afin de mieu vous reperer dans 
ce donjon.

I suspect this function to be the issue since it is needed to create, place and maintain the color Especially the code after local current_line nb_space to be exact.

function dialog_box:create_surface(name)
    -- Create the new surface if it does not exist.
    if not self.line_surfaces[name] then
      self.line_surfaces[name] = {}
      for i = 1, nb_visible_lines do
        self.line_surfaces[name][i] = sol.text_surface.create(dialog_box.text_properties)
      end
    end
    -- Fill with spaces in the current line of the new surface until the current position. 
    local current_line = self.current_line_surface[self.line_index]
    local new_line = self.line_surfaces[name][self.line_index]
    local nb_spaces = #current_line:get_text() - #new_line:get_text() + 1
    if nb_spaces > 0 then
      for i = 1, nb_spaces do new_line:set_text(new_line:get_text() .. " ") end 
    end
    -- Change the current surface to the new surface.
    self.current_line_surface = self.line_surfaces[name]
  end

Deleting the +1 in this case local nb_spaces = #current_line:get_text() - #new_line:get_text()

Remove the space if a special surfac has been created at the start of the text

christopho commented 7 years ago

Also note that #current_line:get_text() gives the number of bytes of the text. If some characters are multi-byte, which is possible in UTF-8 in French and Japanese, the result will be wrong. You can make a small utility function that counts one character for two bytes when the a byte is >= 192 and < 224. See for example how I handle this here: https://github.com/solarus-games/zelda-mercuris-chest/blob/dev/data/scripts/menus/dialog_box.lua#L401

MetalES commented 7 years ago

@christopho The utility is already in the script (in fact, the script is similar to MoS DX), the only difference is that it supports color, and this is how colors are handled (creating a new text surface with the color applied to it)

The same thing happen in @wrightmat 's project, it might do the same on @Diarandor 's one since code for color handling is nearly identical.

I might investigate this deeper tonight

https://github.com/MetalES/Project-Zelda/blob/master/data/scripts/loader/dialog_box.lua#L452

MetalES commented 7 years ago

Hmmmm, weird thing

$[red]- ヒーローズのアーク -$[white] TEST adding a print(nb_space) reveal that the space needed to draw the new white surface is 32 in japanese (it should be 14), which does this (btw Japanese letters are beautiful, I love how it renders)

sans titre 8

At the same time, a text like this, with correct spacing in the text file show this

$[red]- Arc du Heros - Feu -$[white] Embrasez vos cibles avec les $[red]Fleches de Feu$[white]! Attention a votre compteur de magie !

sans titre 10

Which makes me wonder what is wrong, the spacing is so random, sometime it overlaps the previous text, sometimes it goes off charts (see the OP)

I have no idea at all what cause this ...

@Diarandor: Does this happen in your project too ? I see that the code for color is nearly identical

MetalES commented 7 years ago

Okay, so this is faulty and it is confirmed that this is the culprit. Now, for fixing that I don't really know what to do ... Because I don't know how Japaese is parsed because nb_spaces returns 32 instead of 14 (see the post above) But that don't solve the spacing issue at all ... I really don't know how that works to be honnest ... I need some help here

    local current_line = self.current_line_surface[self.line_index]
    local new_line = self.line_surfaces[name][self.line_index]
    local nb_spaces = #current_line:get_text() - #new_line:get_text() + 1

    if nb_spaces > 0 then 
      for i = 1, nb_spaces do new_line:set_text(new_line:get_text() .. " ") end 
    end
Diarandor commented 7 years ago

I did not have that problem, although I have only made a few tests with my script. Also, I don't know if this script is different from mine.

One of the requirements for my script to work is to use a monospaced font, which I guess is not the case with your problematic fonts. I don't know how to fix this now... you should somehow modify the code to find, approximately, how many spaces you really need in the new surface. That would make the script compatible with non-monospaced fonts. But the easiest solution is just to replace the font with a monospaced one that should work nicely.

MetalES commented 7 years ago

@diarandor This might not be because of that, because.

When recreated, the surface parse the old text to determine the space number. It's not a position in a given X axis. Same thing happen with monospaced characters

Take a look at this. sans titre 4

You see that there's only 4 Japanese characters before the red text but printing self.current_char when a new surface is created (here, when the surface for the red color is created), the value for self.current_char is 19 while it should be 6, for the exclamation mark, it is value 66, out of the textbox (and screen).

It might be the main issue for both Japanese and internationnal non-monospaced characters. But a Japanese letter means a lot of things, perharps a letter is parsed as many letters ... I don't know

Diarandor commented 7 years ago

I think Christopho is right. The computation for the number of spaces (when changing surface) of our scripts is wrong. The problem is in the line: local nb_spaces = #current_line:get_text() - #new_line:get_text() We are not taking into account that there may be multibyte characters, that is probably the problem in your dialogs; if there are multibyte characters there will be too many spaces. This can be easily fixed (for monospaced fonts) by changing that line; but still this will never work with non-monospaced fonts, unless you add the necesssary additional spaces.

MetalES commented 7 years ago

Okay, i found a rather good monospaced font, now it's time to see what's wrong with the spacing.

sans titre 4

Added a 4th line for text if needed.

With monospaced fonts, there is only some space, but as @Diarandor suggested, the variable doesn't count multibytes so this might be wrong.

Take in consideration that adding a single -2 in the variable wouldn't be enough, Japanese (or other languages that use multiple characters for assembling a word) will still be wrong, even if the font is a monospaced one.

I don't know if the default font for Asian language provided with Mystery of Solarus DX is monospaced ...

christopho commented 7 years ago

There should also be a solution with variable width fonts, using text_surface:get_size() to know the actual width.

MetalES commented 7 years ago

@christopho the error is something else.

When creating a color surface, another text surface is created, overlapping the old one so the color will be right. The problem happen in line where there are special characters, like accents in french, and I do believe that it is the cause of spacing in other languages

I've succeded to fix the spacing issue, with a monospaced font as @Diarandor suggested.

Here, everything is nicely displayed, notice that there are no special characters the line before the text surface, the issue here is solved. Except in "Forêt" where there's an extra space ... sans titre 16

But, adding special characters, like french accents, this happen. This would also happen with other language's accents, the spacing is normal in the last line. sans titre 6

using self.char_index doesn't help that much

Diarandor commented 7 years ago

Nice to see that at least part of the problem is already solved; I hope we can find a solution for this. I think we should define our own character count function (unless Lua already has a decent one, which I don't know). Maybe an iteration for the characters of the string, and using some function of the Lua "string" library could do it, but I am not sure.

MetalES commented 7 years ago

I am more afraid of Asian font, it work differently, there is still this spacing issue within Asian letters. A word in japanese for example is just some letter fused together, I don't know how that work but i'm sure that it increase the spacing ... I didn't found that much of documentation about it in google.

There is still this issue using normal font, and judging from the screenshots, it is related to multibyte characters, so there is 2 major issues to deal with

The first one being the multibyte characters, even with a variable that count how many special characters there is on a single line and then add it in the nb_space algorythm still show some problems, in spacing (See pictures above and bellow) The spacing when the surface is created seems right, just for this font

sans titre 22

The second one being the incrementation of the text when using Asian words which leads to the same problem, spacing issue

The font I'm using is inconsolata, still it is small, and read-able, and is almost the same size as the old font I used

MetalES commented 7 years ago

English works great, the issue never happen because there's no special characters in most of english words (except déjà-vu) ...

sans titre 3

Spanish use a lot of special characters (might be because your dialogs are in English), and it might happen, my workaround has to grab all special characters in a variable that is incremented at each special characters but there are some issues, still on spacing ...

Diarandor commented 7 years ago

I got a very rudimentary, easy and simple idea, that should work, of how this could be fixed in a future release of solarus, but not yet.

The idea is to use a function "text_surface:get_pixels()", which does not exist yet in solarus 1.5, and then check the pixels of the surface; we could then get the last pixel (the farthest to the right) that has color different from the background color. We then can compute the exact number of spaces that we need to create for the secondary surfaces, when changing from one surface to another. (We will only need to know how many pixels needs a space symbol in the corresponding font, which does not depend on the language, and which is the X-coordinate of the first character on each line.)

MetalES commented 7 years ago

I figured out what cause this, special characters are badly parsed, and thus an algorythm is needed in order to precisely get the space number required - the special characters, it's hard to figure out the precision of this because it's been a while I'm testing this, yet there are still small issues to deal with. Pretty sure it has nothing to do with pixel, the font is monospaced, and there is no problem displaying coloured text in english, it occurs when using special characters

Diarandor commented 7 years ago

Yes, I agree that the problem is produced by special characters. But I am not sure if such an algorithm can be done without a casewise function that checks all problematic cases, and there are a looot of cases if you consider other languages for translations... and even if it is possible to do so, that might not work for all and any type of font... :'( However this possible solution I gave would be pixel-precise and may work for any type of weird font and any language, if I am not wrong. The problem is that solarus 1.6 may be delayed quite a bit (maybe for next year) since Christopho is working more on games these days, so your solution is the best workaround until then, yes.

MetalES commented 7 years ago

Well yeah, I agree, surface:get_pixel() in this case might help.

Yet, I am not sure if there is a spacing between a character, like so, I oon't know how texts surface are handled sans titre 26 ,

christopho commented 7 years ago

Again, just replace #string by a function that takes multi-byte characters. It will solve the spacing issue for monospaced fonts.

if byte >= 192 and byte < 224 then

See the example here: https://github.com/solarus-games/zelda-mercuris-chest/blob/dev/data/scripts/menus/dialog_box.lua#L401

MetalES commented 7 years ago

Hmmm, i'm gonna try Diarandor's fix and see if that corrected the issue

Diarandor commented 7 years ago

Well, I actually never fixed this. Do what @christopho says: define that function and then use it instead of the cardinal operator "#".

Diarandor commented 7 years ago

@christopho: I have read that most of Japanese and Chinese characters need 3 bytes to be encoded in utf-8, so maybe there will be another condition to check if he wants to include Japanese translations.

Diarandor commented 7 years ago

@MetalES: in this website they explain how you can define the function to count the number of characters in utf-8: http://stackoverflow.com/questions/43125333/lua-string-length-cyrillic-in-utf8 I think it is something like this: str_len = #(put_your_string_var_here):gsub('[\128-\191]', '')

They also say there that there is a function utf8.len in Lua 5.3+ that you could use instead, but I don't know which version of Lua is the one that Solarus uses, so maybe we cannot use that function.

Diarandor commented 7 years ago

I have tested the function and it really works! local character_count = function(my_string) return #(my_string):gsub('[\128-\191]', '') end

So try to write in my script local nb_spaces = character_count( current_line:get_text() ) - character_count( new_line:get_text() ) instead of local nb_spaces = #current_line:get_text() - #new_line:get_text()

MetalES commented 7 years ago

It does works now, perfect, thanks Diarandor, I now can focus peacefully on BoM's translation

MetalES commented 7 years ago

Still, I guess there is still one thing left for asian type language, as i have some knowledges in japanese since i've started to learn it, i will also try to translate BoM in Japanese. I have found something that explain how that works, http://lua-users.org/lists/lua-l/2012-12/msg00123.html