Page MenuHomeWildfire Games

Defragment IGUIButtonBehavior members
ClosedPublic

Authored by elexis on Sep 22 2019, 3:37 PM.

Details

Summary

This diff

  1. Moves the AddSetting calls from the child classes to the base class. Benefits:

a) reduces the code by few lines (minimization)
b) Makes it so that AddSetting is in the same class as GetSetting (defragmentation)
c) Might allow in the future to make settings members (D2313, see discussion with Vladislav on http://irclogs.wildfiregames.com/2019-09/2019-09-20-QuakeNet-%230ad-dev.log)
d) Cleaning all AddSetting calls allows to warn about AddSetting calls redundant in derived and base class instead of silently ignoring them and misleading authors to believe that all AddSetting calls are purposeful.

  1. Moves the ChooseColor() function from IGUIButtonBehavior to CButton, since it is only used by this class following rP14476 (refs rP22952 / D2314). If the function was to be considered a part of IGUIButtonBehavior, then the AddSetting calls to textcolor, textcolor_disabled, textcolor_pressed, textcolor_over should also be in this class (see argument for 1). So also the comment // Yes, the object must possess these settings. They are standard.

    Notice that CDropdown mirrors part of the IGUIButtonBehavior and excludes other parts thereof, which makes it particularly less confusing if there are less AddSetting calls.
Test Plan

Follow the arguments in the summary. Notice that the patch doesnt change code behavior. Notice that there is more related to cleanup, but should be done in other patches to make it more failsafe and auditable.

Event Timeline

elexis created this revision.Sep 22 2019, 3:37 PM

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

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

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

Linter detected issues:
Executing section Source...

source/gui/CDropDown.h
|  46| class·CDropDown·:·public·CList
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCDropDown:' is invalid C code. Use --std or --language to configure the language.

source/gui/CButton.h
|  25| class·CButton·:·public·IGUIButtonBehavior,·public·IGUITextOwner
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCButton:' is invalid C code. Use --std or --language to configure the language.

source/gui/IGUIButtonBehavior.h
|  38| class·IGUIButtonBehavior·:·virtual·public·IGUIObject
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classIGUIButtonBehavior:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Stan added a subscriber: Stan.Sep 22 2019, 3:51 PM
Stan added inline comments.
source/gui/CButton.cpp
103

ternary ?

elexis added inline comments.Sep 22 2019, 3:56 PM
source/gui/CButton.cpp
103

Not sure if its much nicer, because theres already a condition in it.
Cant escape the early return though.

source/gui/IGUIButtonBehavior.cpp
28

uninitialized member from rP13040

elexis edited the summary of this revision. (Show Details)Sep 22 2019, 4:01 PM
elexis edited the summary of this revision. (Show Details)Sep 22 2019, 4:49 PM

This seems like a good move.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 22 2019, 4:53 PM
This revision was automatically updated to reflect the committed changes.