Change Details

The LoadStyle function silently ignores if a Style specifies a Setting that doesn't exist on the IGUIObject type it is assigned to. By adding a harmless LOGWARNING, the machine can help authors to identify such entries. There is a code comment in LoadStyle stating that it is a beauty to silently ignore, but it was not specified why this is a beauty. If one applies the warning, we see a lot of warnings that arise from the "default" template, so it seems originally it was used for the default template. If exempting the default style from the LOGWARNING, we dont get any false positives, indicating that there is no real use of this in other places. So this warning identified 27 true positives and 0 false positives. The next question is whether there could be any beauty (use case) in this at all. If anything, it would be inheritance. There is inheritance for IGUIObjects already, for example COList inherits CList inherits IGUIObject. There are two ways for the inheritance: 1. We observe defining a style that works for a base class will after this patch still apply without showing warnings. So "base styles" are a use case and still supported. 2. We observe that a style that specializes for a child class will specify wrong settings for the base class. The first effect of this is that authors are invited to specify settings (most importantly via copypasta) that don't ever have an effect without them realizing that. The second observation for this case is that style authors will specify Settings that suit one child class (for example COList inheriting CList), but this style will have Settings specified that (a) don't exist in the other child class (b) exist in the other child class but not this one (c) specify a setting that should have a different value for both child classes, but the same Setting name! The default style is most exposed to 2(c), because it applies to literaly every object, but specifying default values that apply to literally everything is not really possible. So the conceivable use cases are better served by implementing style inheritance specifically if this is important. Then one could specify a style for CList and then inherit this style in the style for COList and CDropdown. (That could be called beauty perhaps.) The next question is whether virtual style inheritance is important enough to spend time on implementing. If we look at the codebase, we see that other than the default style, nothing uses this as a feature. For example Dropdowns and OLists (tables) specify their own style. It's simple to perform. The only disadvantage of not having style inheritance is more copypasta. So for this patch and end of august 2019, I would suggest to add the warning and guide people to specify one style per IGUIObject rather than guiding them to specify settings that never apply. This patch also unifies the two functions and removes one unneeded CGUI argument that is always same to the member.

The LoadStyle function silently ignores if a Style specifies a Setting that doesn't exist on the IGUIObject type it is assigned to. By adding a harmless LOGWARNING, the machine can help authors to identify such entries. There is a code comment in LoadStyle stating that it is a beauty to silently ignore, but it was not specified why this is a beauty. If one applies the warning, we see a lot of warnings that arise from the "default" template, so it seems originally it was used for the default template. If exempting the default style from the LOGWARNING, we dont get any false positives, indicating that there is no real use of this in other places. So this warning identified 27 true positives and 0 false positives. The next question is whether there could be any beauty (use case) in this at all. If anything, it would be inheritance. There is inheritance for IGUIObjects already, for example COList inherits CList inherits IGUIObject. There are two ways for the inheritance: 1. We observe defining a style that works for a base class will after this patch still apply without showing warnings. So "base styles" are a use case and still supported. 2. We observe that a style that specializes for a child class will specify wrong settings for the base class. The first effect of this is that authors are invited to specify settings (most importantly via copypasta) that don't ever have an effect without them realizing that. The second observation for this case is that style authors will specify Settings that suit one child class (for example COList inheriting CList), but this style will have Settings specified that (a) don't exist in the other child class (b) exist in the other child class but not this one (c) specify a setting that should have a different value for both child classes, but the same Setting name! The default style is most exposed to 2(c), because it applies to literaly every object, but specifying default values that apply to literally everything is not really possible. So the conceivable use cases are better served by implementing style inheritance specifically if this is important. Then one could specify a style for CList and then inherit this style in the style for COList and CDropdown. (That could be called beauty perhaps.) The next question is whether virtual style inheritance is important enough to spend time on implementing. If we look at the codebase, we see that other than the default style, nothing uses this as a feature. For example Dropdowns and OLists (tables) specify their own style. It's simple to perform. The only disadvantage of not having style inheritance is more copypasta. So for this patch and end of august 2019, I would suggest to add the warning and guide people to specify one style per IGUIObject rather than guiding them to specify settings that never apply. This patch also unifies the two functions and removes one unneeded CGUI argument that is always same to the member. The patch arose as a preparation of D2231.

The LoadStyle function silently ignores if a Style specifies a Setting that doesn't exist on the IGUIObject type it is assigned to. By adding a harmless LOGWARNING, the machine can help authors to identify such entries. There is a code comment in LoadStyle stating that it is a beauty to silently ignore, but it was not specified why this is a beauty. If one applies the warning, we see a lot of warnings that arise from the "default" template, so it seems originally it was used for the default template. If exempting the default style from the LOGWARNING, we dont get any false positives, indicating that there is no real use of this in other places. So this warning identified 27 true positives and 0 false positives. The next question is whether there could be any beauty (use case) in this at all. If anything, it would be inheritance. There is inheritance for IGUIObjects already, for example COList inherits CList inherits IGUIObject. There are two ways for the inheritance: 1. We observe defining a style that works for a base class will after this patch still apply without showing warnings. So "base styles" are a use case and still supported. 2. We observe that a style that specializes for a child class will specify wrong settings for the base class. The first effect of this is that authors are invited to specify settings (most importantly via copypasta) that don't ever have an effect without them realizing that. The second observation for this case is that style authors will specify Settings that suit one child class (for example COList inheriting CList), but this style will have Settings specified that (a) don't exist in the other child class (b) exist in the other child class but not this one (c) specify a setting that should have a different value for both child classes, but the same Setting name! The default style is most exposed to 2(c), because it applies to literaly every object, but specifying default values that apply to literally everything is not really possible. So the conceivable use cases are better served by implementing style inheritance specifically if this is important. Then one could specify a style for CList and then inherit this style in the style for COList and CDropdown. (That could be called beauty perhaps.) The next question is whether virtual style inheritance is important enough to spend time on implementing. If we look at the codebase, we see that other than the default style, nothing uses this as a feature. For example Dropdowns and OLists (tables) specify their own style. It's simple to perform. The only disadvantage of not having style inheritance is more copypasta. So for this patch and end of august 2019, I would suggest to add the warning and guide people to specify one style per IGUIObject rather than guiding them to specify settings that never apply. This patch also unifies the two functions and removes one unneeded CGUI argument that is always same to the member. The patch arose as a preparation of D2231.