Page MenuHomeWildfire Games

Use CStrIntern type for the GUI object font setting
Needs ReviewPublic

Authored by elexis on Sep 28 2019, 3:19 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Whenever (without exception) the font is used, it is converted from CStrW to CStrIntern.
So the character of the setting is CStrIntern and CStrW is just the intermediary.
Hence it would be cleaner to use the target type CStrIntern directly.

Test Plan

Make sure there is no UTF8 failure introduced.
Notice that the code other than the FromJSVal / ToJSVal / ParseString conversion is reduced in complexity (removed conversion lines).
Notice the FromJSVal / ToJSVal / ParseString conversions should be supported with or without this particular diff, if we want those functions to become more versatile.

Event Timeline

elexis created this revision.Sep 28 2019, 3:19 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/860/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/344/display/redirect

I wonder if it wouldn't be better to use the CFont as the class and then pass that on, rather than reconstructing the CFont class all the time.

g_Renderer.GetFontManager().LoadFont(font);

That will convert CStrIntern to CStr:

const VfsPath fntName(fontName.string() + ".fnt")

Then there's

	Path(const char* p)
		: path((const unsigned char*)p, (const unsigned char*)p+strlen(p))
		// interpret bytes as unsigned; makes no difference for ASCII,
		// and ensures OsPath on Unix will only contain values 0 <= c < 0x100

and this good old thing:

	/**
	 * Return a UTF-8 version of the path, in a human-readable but potentially
	 * lossy form. It is *not* safe to take this string and construct a new
	 * Path object from it (it may fail for some non-ASCII paths) - it should
	 * only be used for displaying paths to users.
	 */
	std::string string8() const
	{
		Status err;
#if !OS_WIN
		// On Unix, assume paths consisting of 8-bit charactes saved in this wide string.
		std::string spath(path.begin(), path.end());

		// Return it if it's valid UTF-8
		wstring_from_utf8(spath, &err);
		if(err == INFO::OK)
			return spath;

		// Otherwise assume ISO-8859-1 and let utf8_from_wstring treat each character as a Unicode code point.
#endif
		// On Windows, paths are UTF-16 strings. We don't support non-BMP characters so we can assume it's simply a wstring.
		return utf8_from_wstring(path, &err);
	}

which provides an opportunity to give up.

source/scriptinterface/ScriptConversions.cpp
191

It seems wrong to assume that UTF8 would be a format that this function should be concerned about.

However if we use FromJSVal<CStr> here, then it will fail if the fontname contains an ä specified in JS.