Regression in text display in Whispers of a Machine

[criezy]

Describe the bug
The text in Whispers of a machine is not displayed correctly in some places. This was discussed in PR #1705.

AGS Version
Current code from master (commit c31e55c).

Game
Whispers of a Machine
https://store.steampowered.com/app/631570/Whispers_of_a_Machine/

To Reproduce

1. Start the game
2. Watch the intro
3. See error

Expected behavior
Correct text display (see images below).

Screenshots

Correct:

Incorrect:

Desktop (please complete the following information):

• OS: macOS
• Version: 11.6

Additional context
The screenshot above are from ScummVM where the text used to be displayed correctly. A git bisect points to scummvm/scummvm@df29806.

This was a backport of commit d9c4a03
Engine: replaced the remaining wgettextheight() with get_font_height()

The AGSSpriteFont plugin was not working for WOAM before PR #1705 was merged, so I did not try to check in AGS if it indeed worked before that commit (it probably wasn't). But since the behaviour we get is the same in AGS and in ScummVM I am guessing the issue in AGS has the same root cause as in ScummVM and is related to the change to the font height.

[ivan-mogilko]

I just have an idea; this change in d9c4a03 makes engine use a precomputed font height instead of calling Renderer each time. It might be that either it does not compute correctly for the Renderers provided by plugin, or it does not recompute when the plugin replaces the font's renderer following a script command.

[criezy]

I am guessing that the issue is actually due to the font height no longer depending on the text. The Clifftop Games version of AGSSpriteFont plugin does some interesting things to compute the font height, and it depends on the text 😉

ags/Plugins/AGSSpriteFont/AGSSpriteFont/VariableWidthSpriteFontClifftopGames.cpp

Lines 16 to 37 in c31e55c

	int VariableWidthSpriteFontRendererClifftopGames::GetTextHeight(const char *text, int fontNumber)
	{
	VariableWidthFont *font = getFontFor(fontNumber);
	if(strcmp("<LINE_SPACING>", text) == 0)
	return font->LineSpacingOverride;
	for(int i = 0; i < (int)strlen(text); i++)
	{
	if (font->characters.count(text[i]) > 0)
	{
	int height = font->characters[text[i]].Height;
	if(strcmp("ZHwypgfjqhkilIK", text) == 0 || strcmp("ZhypjIHQFb", text) == 0 || strcmp("YpyjIHgMNWQ", text) == 0 || strcmp("BigyjTEXT", text) == 0)
	height += font->LineSpacingAdjust;
	else
	height += font->LineHeightAdjust;
	return height;
	}
	}
	return 0;
	}

[ivan-mogilko]

Hm, I see, so the plugin was abusing a secret knowledge of which strings the older engine pass to learn the font height in very specific circumstances... And it also seem to have a new special constant "<LINE_SPACING>" for this. I wonder if it came from script or the "Clifftop's" custom engine sent it to retrieve linespacing parameter.

[ivan-mogilko]

Need to consider the options here, but I'd rather find a way to not rely on hacks, which might mean - carefully expanding AGSFontRenderer interface to let engine find out more about what it does: such as get custom linespacing, and so forth.

EDIT: on another hand, this may not be solveable like that, if the plugin lets change font parameters on the fly from script... unless it also will have a way to notify the engine that the font has changed.

For instance, with this plugin, when the font is replaced in the engine, the characters are not loaded yet, so it cannot report a proper height.

[ivan-mogilko]

@criezy opened an experimental pr: #1744