Change Details

When there is a syntax error in an XML file, such as the example from D1574 which was the typo reported in https://code.wildfiregames.com/rP21847#30949, then there will be a crash as of rP22796 (trying to access a map item that doesn't exist and cant be healthily default-constructed). In the string parsing function previous to rP22796 (`IGUIObject::SetSetting(const CStr& Setting, const CStrW& Value, const bool& SkipMessage)`), it did simply not report it for most calls, which is wrong as well. For example if a translatable attribute was not found, then it should inform the XML author. For example the silent ignoring of the error in this case: ``` CStrW caption(Element.GetText().FromUTF8()); if (!caption.empty()) object->SetSettingFromString("caption", caption, false); ``` was a intentional choice in rP290: > // There is no harm if the object didn't have a "caption" However there is harm to having a caption there, since the XML author specified something that he assumes will have an effect, will maintain and optimize it when in truth it is not used. Compare also to rP22792 showing a LOGWARNING when the style specifies a value for as setting that does not exist.

When there is a syntax error in an XML file, such as the example from D1574 which was the typo reported in https://code.wildfiregames.com/rP21847#30949, then there will be a crash as of rP22796 (trying to access a map item that doesn't exist and cant be healthily default-constructed). In the string parsing function previous to rP22796 (`IGUIObject::SetSetting(const CStr& Setting, const CStrW& Value, const bool& SkipMessage)`), it did simply not report it for most calls, which is wrong as well. For example if a translatable attribute was not found, then it should inform the XML author. For example the silent ignoring of the error in this case: ``` CStrW caption(Element.GetText().FromUTF8()); if (!caption.empty()) object->SetSettingFromString("caption", caption, false); ``` was a intentional choice in rP290: > // There is no harm if the object didn't have a "caption" However there is harm to having a caption there, since the XML author specified something that he assumes will have an effect, will maintain and optimize it when in truth it is not used. Compare also to rP22792 showing a LOGWARNING when the style specifies a value for as setting that does not exist. rP14953 introduced the `elmt_translatableAttribute` code without error reporting. rP74 originally threw an error, rP1494 changed it to a return value, but most/many callers did not test for that value.

When there is a syntax error in an XML file, such as the example from D1574 which was the typo reported in https://code.wildfiregames.com/rP21847#30949, then there will be a crash as of rP22796 (trying to access a map item that doesn't exist and cant be healthily default-constructed). In the string parsing function previous to rP22796 (`IGUIObject::SetSetting(const CStr& Setting, const CStrW& Value, const bool& SkipMessage)`), it did simply not report it for most calls, which is wrong as well. For example if a translatable attribute was not found, then it should inform the XML author. For example the silent ignoring of the error in this case: ``` CStrW caption(Element.GetText().FromUTF8()); if (!caption.empty()) object->SetSettingFromString("caption", caption, false); ``` was a intentional choice in rP290: > // There is no harm if the object didn't have a "caption" However there is harm to having a caption there, since the XML author specified something that he assumes will have an effect, will maintain and optimize it when in truth it is not used. Compare also to rP22792 showing a LOGWARNING when the style specifies a value for as setting that does not exist. rP14953 introduced the `elmt_translatableAttribute` code without error reporting. rP74 originally threw an error, rP1494 changed it to a return value, but most/many callers did not test for that value.