Page MenuHomeWildfire Games

Delete broken fallback font for broken fonts
ClosedPublic

Authored by elexis on Aug 23 2019, 4:41 AM.

Details

Summary

The L"default" font doesn't actually exist, so instead of complaining about missing "fonts/.fnt" it complains about missing "fonts/default.fnt" if it was ever to be the case.
The approach of the GUI is that it reads data from the XML file and complains if one didn't provide a mandatory value, so it seems out of place to do it differently for the fonts.
If one wanted a default font, it should be set alongside the other font values in the constructor instead.

The only cases in which this code was actually triggered is when no font was rendered.
Precisely in the CDropdown constructor called from Xeromyces_ReadObject there are some SetSetting calls that happen to send the settings-changed message which resulted in SetupText calls,
and then those calls read from the default constructed setting values. But then Xeromyces_ReadObject finishes and the font is set.

Test Plan

Consider whether the argument in the summary is approvable. One can add an ENSURE !font.empty and if empty LOGERROR to see in which cases it happens.
Notice that we don't have a default.fnt, so it won't change anything for 0ad nor mods unless the mod would happen to add a default.fnt and happen to make use of this featurebug that only C++ developers noticed.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Aug 23 2019, 4:41 AM

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

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

elexis added a comment.EditedAug 23 2019, 5:17 AM

Revision history:

rP141 introduced the "default" style
rP648 copied it to the "official" mod
rP1074 introduced the hardcoded "default" font
rP1085 introduced the

// TODO Gee: (2004-08-14) Don't define standard like this. Do it with the default style.

rP1540 adds CTooltip.cpp and copies the code
rP1903 fixes that TODO (probably unknowingly)
rP2116 adds CList and copies the TODO
rP15063 copies the default style in modern/styles.xml

So the currently committed code only worked for objects in Xeromyces_ReadObject after ConstructObject before object->LoadStyle(*this, "default"), which is only in the according GUI object constructor, or it becomes relevant later if the default style is not defined.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 23 2019, 5:28 AM
This revision was automatically updated to reflect the committed changes.