Unable to select font Widelands.ttf

[Martin]

The selection of "Widelands" font via "Advanced Options" logs a couple of error messages like that:

> Could not find font file "/fonts/Widelands/Widelands.ttf"
> Make sure the path is given relative to Widelands font directory.
> Widelands will use standard font.

So the game defaults to the standard font.
The described problem seems to be related to the leading slash. Indeed, there is no /fonts/Widelands/Widelands.ttf, the correct path should be "fonts/Widelands/Widelands.ttf" or "./fonts/Widelands/Widelands.ttf".
The path seems to be miscreated by the g_fs->FS_CanonicalizeName(...) function. Either the cwd is not determined successfully or a leading slash is added for another erroneous reason.

[Venatrix (elisabeth)]

Yes, you’re right, I never tried it before, I guess.

[Jari Hautio (jarih)]

Sounds like the issue I fixed in a branch a while ago. Path handling in FileSystem code bugged with relative paths and trying to use current working directory on linux throwed file access to root folder. The problem was that FileSystem root was always initialized first to '/' and then later FS_CanonicalizeName function did not use current working directory because root was already set. I noticed and fixed it when I enabled unit tests for filesystem code. The issue should be fixed in lp:~jarih/widelands/fix-unit-tests branch. See http://bazaar.launchpad.net/~jarih/widelands/fix-unit-tests/revision/5808 .

[Martin (p-martin)]

I have a fix for the problem:

https://code.launchpad.net/~p-martin/widelands/font_selection

I've already proposed the branch for merging.
Jari, is this the same way like you did the fix? So maybe we could just take yours...

[Martin (p-martin)]

Yes, I see. Jari, your fix (5808) looks nearly the same like mine - it's the issue with m_root. So which one should we take now? And why has your fix not been merged into trunk yet?

[Jari Hautio (jarih)]

Martin, my branch contains more changes related to fixing unit tests and these fixes go in after build 16. So your branch is better for build16.

[Nasenbaer (nasenbaer)]

Thanks for your fixes you two.
Committed in rev 5879

[David Allwicher (aber)]

I can confirm, it is broken. The code uses pwd or getcwd do determine the working directory. I don't think this can actually work. So we don't wont to unset m_root. For me the '.' actually works. Which makes kind of sense.
I don't know Martin this is your second patch and while the first one did not check the return value, this one seems also a bit careless.
Jari could you please remove this particular chance from your branch so that we don't merge this again into trunk?

[Nasenbaer (nasenbaer)]

Actually I tested the fix before committing . David is current trunk (inlcuding the patch) broken for you?

[David Allwicher (aber)]

Sorry for not being to specific. Have a look at filesystem.cc:252:
FS_Tokenize(m_root.empty() ? getWorkingDirectory() : m_root, m_filesep,std::inserter(components, components.begin()));

getWorkingDirectory() is only a valid directory if you start widelands with './widelands'. It's basically a random directory.

So yes the current trunk is broken for me and the mac package is also broken.

And while the documentation states this should not contain the fileseparator. '/' is also the root directory on unix.

[Nasenbaer (nasenbaer)]

Sorry that I have not found time to look into it again until now. David you are of course right.
I reverted the previous patch and chnaged it along the lines you suggested. If this breaks something on any system, than only the font selection that did not work before anyways ;)

[Jari Hautio (jarih)]

I happened to check this again and the later fix is absolutely correct. Call to FS_CanonicalizeName was in incorrect place. It will be automatically called inside file system code. But I still think m_root should be initialized to empty string in linux/mac, but we can discuss it later when merging my changes in.

[SirVer (sirver)]

Released in build16-rc1