clash with font renderer and server messages?

[Klaus Halfmann]

I was playing bzr8770[trunk] and when conecting to the metaserver I get:

Error rendering richtext: Syntax error at 1:385: expected an allowed tag, got '/p'. String continues with: '</p><p font-face=Widelands/Widelands font-color=00FF00 font-size=22>
Welcome on the Widelands Metase'. Text is:
<rt><p><font color=33ff33 size=9>[07:28] </font><font size=14 face=serif color=999999 bold=1>*** Server time offset is 0 seconds.</font><br></p><p><font color=33ff33 size=9>[07:28] </font><font size=14 face=serif color=999999 bold=1>*** Users marked with IRC will possibly not react to messages.</font><br></p><p><font color=33ff33 size=9>[07:28] </font><font size=14 face=serif color=999999 bold=1>*** </p><p font-face=Widelands/Widelands font-color=00FF00 font-size=22>
Welcome on the Widelands Metaserver
<br>
</p><p font-face=FreeSerif font-color=CCCCFF font-size=14>
* Our forums can be found at http://wl.widelands.org/forums
<br>
* Please report bugs at https://launchpad.net/widelands</font><br></p><p><font color=33ff33 size=9>[07:28] </font><font size=14 face=serif color=999999 bold=1>*** Connection was closed by the client normally.</font><br></p></rt>

This text is also visible on the lobby screen.

Some other branch bzr8757[bug-1619402-port-work-area-on-expedition] crashes.
R19 sems unaffected.

* I assume the server messages are encoded for the old font renderer (which we need for R19 and even r18 for quite a while)
* OTHO this message looks strange to a normal player.

I assume we should implement different welcome messages on the server now:
* <= r18 plain text: "your version is outdated, it may still work ..."
* >= r20 new font renderer
* else (== r19) keep current message

[GunChleoc (gunchleoc)]

Your analysis is correct - it's encoded for the old renderer. The new renderer needs to look like this:

<p><font color=CCCCFF size=14>text text </font></p>

We should definitely lose the font face, because it will mess up translations if we ever decide to translate these messages.

Maybe even lose the font tags completely, because that should be defined by the UI and not by the server? And a bigger header is not really needed. Both renderers can handle a simple <p>text text</p> without parameters.

[Notabilis (notabilis27)]

It seems as if the following string is the current "message of the day" sent by the metaserver:

</p><p font-face=Widelands/Widelands font-color=00FF00 font-size=22>
Welcome on the Widelands Metaserver
<br>
</p><p font-face=FreeSerif font-color=CCCCFF font-size=14>
* Our forums can be found at http://wl.widelands.org/forums
<br>
* Please report bugs at https://launchpad.net/widelands.system

Unfortunately, I am currently unable to look on the metaserver-server where this message is coming from, it does not seem to be set by the code. Maybe there is some magic inside the metaserver startup script.
GunChleoc, you have superuser rights in the lobby (green bar next to username), right? In that case you should be able to fix the string by entering in the chat line:

/motd </p><p>New fixed string similar to the one above

I am not quite sure whether this will work and whether the change will survive a restart of the metaserver, but it is worth a try.

[Notabilis (notabilis27)]

When I think about it, the initial "</p><p ...>" should probably be "</font><font ...>" now. So that could mean that the fixed string might be:

</font><font color=00FF00 size=22>
Welcome on the Widelands Metaserver
<br>
</font><font color=CCCCFF size=14>
* Our forums can be found at http://wl.widelands.org/forums
<br>
* Please report bugs at https://launchpad.net/widelands.system

I am not quite sure, you are the expert for text. (If required: The code for formatting chat messages is in src/wui/chat_msg_layout.cc . Basically it embeds the given string into <p><font>...</font></p> so we have to exit the <font> first before we can add an own one for the MotD.)

Also, using

/motd %myfile

should be possible to use as well and should set the MotD to the contents of your local file "myfile" (relative to ./widelands, I guess). If that works it might be easier than typing the richtext into the chat line.

[GunChleoc (gunchleoc)]

I'd prefer not to have any hard-coded font statements at all, maybe just use <br> and not have a colored and bigger header?

I want to control font sizes and colors etc via Lua in the future, like this: https://bazaar.launchpad.net/~widelands-dev/widelands/font_size-lua/view/head:/data/templates/default/init.lua

[Notabilis (notabilis27)]

Would be fine by me. So maybe fix it for now with the /motd command and then start working on moving this (and probably also other) messages from the metaserver code into the client code? Since we have the MotD set to this value for quite some time already, we could hard-code the text in the client as well. This would also allow us to translate the message, which isn't possible at the moment.

[GunChleoc (gunchleoc)]

#6 Sounds good to me - the message could also come from a lua File, to make it easier to edit.

[GunChleoc (gunchleoc)]

#2 happens because the old markup raises an exception in the richtext parser, so richtext_escape is called on the whole lot. Maybe we should change the chat implementation too to do the escaping on individual chat entries if needed to prevent Multilinetextarea from escaping the whole lot?

[kaputtnik (franku)]

Don't know if it is related: The chat does also not autoscroll https://wl.widelands.org/forum/topic/4303

[Notabilis (notabilis27)]

So... after looking into the code, I am pretty much lost.

1) The error is currently gone, so I guess GunChleoc has used the motd command? Thanks!

2) I have no idea where the current motd command is saved on the server. Obviously it survives the restart of the (meta)server, so there should be some file containing it. But I wasn't able to find this file or find out how the variable within the metaserver is read/written. Hm. :-/

3) "Escaping" the chat entries is already done on a per-message basis. However, the function seems to be quite lenient and it crashes later on when trying to render the complete text. So I guess one way to fix this would be to provide a global "escape text" method that can be used by all code, independent of the chat. Unfortunately I fear that such a method would be quite computation intensive since it would have to parse the whole text (again). Maybe we can modify the render function to not throw an exception on release builds but silently drop the offending tag? Or print only the offending tag as text and interpret the rest of the richtext string? Not sure which is the best approach to fix this problem.

4) The broken autoscroll functionality didn't had anything to do with this bug, but is fixed now.

5) Currently, the "Welcome on metaserver" message is completely gone. Do you want me to re-implement it as a local message within the Widelands client for newer builds? Should the metaserver send the message as normal text (not as motd) for build 19 clients on connect? Both should be easy to do.
For trunk, I wouldn't read it from a lua file for now, though. If I am not mistaken, all UI strings are currently hardcoded so I would do that here as well until we start moving all of them to lua files.

[GunChleoc (gunchleoc)]

1) The error is currently gone, so I guess GunChleoc has used the motd command? Thanks!

No, I haven't.

3)

There already is a "richtext_escape" function in Widelands - we just need to make sure to call it correctly for this usecase.

5)

Sounds good to me.

Overall, I think we don't need a special "Message of the Day" - I can't remember that it was ever changed. So, I'd be fine with getting rid of the /motd command entirely.

[Notabilis (notabilis27)]

If I see it right, richtext_escape() simply replaces all &<> with their respective HTML-codes. So you want to disable all richtext formatting for chat and system messages?

[GunChleoc (gunchleoc)]

Exactly, no user-created formatting. It's just safer that way.

Or we could pre-render each message as a test, and it the rendering fails, escape it then. This is how the Multilinetextarea does it:

   try {
    rendered_text_ = UI::g_fh->render(text_, get_eff_w() - 2 * RICHTEXT_MARGIN);
   } catch (const std::exception& e) {
    log("Error rendering richtext: %s. Text is:\n%s\n", e.what(), text_.c_str());
    text_ = make_richtext();
    rendered_text_ = UI::g_fh->render(text_, get_eff_w() - 2 * RICHTEXT_MARGIN);
   }

[GunChleoc (gunchleoc)]

Fixed in build20-rc1