HomeWildfire Games

Sort GUI Object types, GUI Object bases and GUI Setting value types into three…

Description

Sort GUI Object types, GUI Object bases and GUI Setting value types into three new folders.

Differential Revision: https://code.wildfiregames.com/D2345
Tested on: gcc 9.1.0
Comments By: Vladislav

Event Timeline

I think Scripting has a wrong case, it's out of repo CC. Object* and SettingTypes are not so "out of rules", but I'd prefer underscore to fit into all other folder names.

I think Scripting has a wrong case, it's out of repo CC. Object* and SettingTypes are not so "out of rules", but I'd prefer underscore to fit into all other folder names.

Citation of the CC or you cant claim its in the CC. It actually says:

Primarily, try to match the style of the functions that you're editing (assuming it's at least self-consistent and not too bizarre), in order to avoid making it less self-consistent.
Secondly, try to match the style of the files that you're editing.
Then, try to match the style of the other code in the subdirectory you're editing.
Finally, try to match the global guidelines discussed on this page.

Well its not like I didnt mentioned the folder names or the reasoning several times in the discussion we had http://irclogs.wildfiregames.com/2019-10/2019-10-01-QuakeNet-%230ad-dev.log

In particular:

19:32 < elexis> actually I see 17 folders with underscore in the public mod, so perhaps object_bases object_types is good
19:32 < elexis> there is also setting_types
19:32 < elexis> (settings would be wrong, since the settings are the CGUISetting instances)
19:35 < elexis> or SettingTypes ObjectTypes ObjectBases
19:36 < elexis> similar to ./ps/GameSetup ./tools/lobbybots/XpartaMuPP  ./tools/atlas/AtlasUI/ScenarioEditor/Sections/Object

Or

19:49 < elexis> I will implement ObjectTypes ObjectBases SettingTypes if there is no objection 
19:50 < Vladislav> Why not just Objects?
19:50 < elexis> because Objects are the instances
19:50 < elexis> equally SettingTypes because Settings are the instances
19:51 < elexis> also allows distinction of ObjectBases from ObjectTypes
19:51 < elexis> also ObjectType is more strict than Object, because someone might add things that relate to Obejcts but are not ObjectTypes

and

20:02 < Vladislav> I agree with most, I just can't call it object_base.
20:02 < elexis> ObjectBase
20:02 < elexis> s

Yes, the scripting folders in other places are called scripting, however in this folder the folders are capitalized and there is no binding contract (like there is with test folder) that forces us to use lowercase for scripting.
If the other folders and all filenames are starting with uppercase, then it is more consistent within this folder if this one does too.

Citation of the CC or you cant claim its in the CC. It actually says:

Well, before the change we didn't have such name in the folders, but we have tests and scripting. So you broke the rules from CC.

Well its not like I didnt mentioned the folder names or the reasoning several times in the discussion we had http://irclogs.wildfiregames.com/2019-10/2019-10-01-QuakeNet-%230ad-dev.log

20:02 < Vladislav> I agree with most, I just can't call it object_base.

It was in context of your patch https://code.wildfiregames.com/P183, and it contains only underscores.

Yes, the scripting folders in other places are called scripting, however in this folder the folders are capitalized and there is no binding contract (like there is with test folder) that forces us to use lowercase for scripting.
If the other folders and all filenames are starting with uppercase, then it is more consistent within this folder if this one does too.

First of all for a new folder we use other folder names style not file names.

See also P183 (which I started with)

18:34 < WildfireBot> e​l​e​x​i​s created P183 (source/gui/ revised directory) https://code.wildfiregames.com/P183

If you do a find in source/gui/ you will see that its more consistent if that folder uses the same style as the filenames.

Compare this:

./CGUI.cpp
./CGUI.h
./CGUIScrollBarVertical.cpp
./CGUIScrollBarVertical.h
./CGUISetting.cpp
./CGUISetting.h
./CGUIText.cpp
./CGUIText.h
./GUIManager.cpp
./GUIManager.h
./GUIMatrix.cpp
./GUIMatrix.h
./GUIRenderer.cpp
./GUIRenderer.h
./GUIStringConversions.cpp
./GUITooltip.cpp
./GUITooltip.h
./IGUIScrollBar.cpp
./IGUIScrollBar.h
./object_bases/IGUIButtonBehavior.cpp
./object_bases/IGUIButtonBehavior.h
./object_bases/IGUIObject.cpp
./object_bases/IGUIObject.h
./object_bases/IGUIScrollBarOwner.cpp
./object_bases/IGUIScrollBarOwner.h
./object_bases/IGUITextOwner.cpp
./object_bases/IGUITextOwner.h
./object_types/CButton.cpp
./object_types/CButton.h
./object_types/CChart.cpp
./object_types/CChart.h
./object_types/CCheckBox.cpp
./object_types/CCheckBox.h
./object_types/CDropDown.cpp
./object_types/CDropDown.h
./object_types/CGUIDummyObject.h
./object_types/CImage.cpp
./object_types/CImage.h
./object_types/CInput.cpp
./object_types/CInput.h
./object_types/CList.cpp
./object_types/CList.h
./object_types/CMiniMap.cpp
./object_types/CMiniMap.h
./object_types/COList.cpp
./object_types/COList.h
./object_types/CProgressBar.cpp
./object_types/CProgressBar.h
./object_types/CRadioButton.cpp
./object_types/CRadioButton.h
./object_types/CSlider.cpp
./object_types/CSlider.h
./object_types/CText.cpp
./object_types/CText.h
./object_types/CTooltip.cpp
./object_types/CTooltip.h
./scripting/GuiScriptConversions.cpp
./scripting/JSInterface_GUIManager.cpp
./scripting/JSInterface_GUIManager.h
./scripting/JSInterface_GUISize.cpp
./scripting/JSInterface_GUISize.h
./scripting/JSInterface_IGUIObject.cpp
./scripting/JSInterface_IGUIObject.h
./scripting/ScriptFunctions.cpp
./scripting/ScriptFunctions.h
./setting_types/CGUIColor.cpp
./setting_types/CGUIColor.h
./setting_types/CGUIList.h
./setting_types/CGUISeries.h
./setting_types/CGUISize.cpp
./setting_types/CGUISize.h
./setting_types/CGUISprite.cpp
./setting_types/CGUISprite.h
./setting_types/CGUIString.cpp
./setting_types/CGUIString.h
./setting_types/EAlign.h
./setting_types/GUISettingTypes.h
./SGUIIcon.h
./SGUIMessage.h
./SGUIStyle.h
./tests/test_GuiManager.cpp
./tests/test_GuiManager.h
./tests/test_ParseString.cpp
./tests/test_ParseString.h

to this:

./CGUI.cpp
./CGUI.cpp.orig
./CGUI.h
./CGUIScrollBarVertical.cpp
./CGUIScrollBarVertical.h
./CGUISetting.cpp
./CGUISetting.h
./CGUISize.h.svnpatch.rej
./CGUISprite.cpp
./CGUISprite.h
./CGUIText.cpp
./CGUIText.h
./GUIManager.cpp
./GUIManager.h
./GUIMatrix.cpp
./GUIMatrix.h
./GUIRenderer.cpp
./GUIRenderer.h
./GUISettingTypes.h
./GUIStringConversions.cpp
./GUITooltip.cpp
./GUITooltip.h
./IGUIObject.cpp.svnpatch.rej
./IGUIObject.h.svnpatch.rej
./IGUIScrollBar.cpp
./IGUIScrollBar.h
./ObjectBases
./ObjectBases/IGUIButtonBehavior.cpp
./ObjectBases/IGUIButtonBehavior.h
./ObjectBases/IGUIObject.cpp
./ObjectBases/IGUIObject.h
./ObjectBases/IGUIScrollBarOwner.cpp
./ObjectBases/IGUIScrollBarOwner.h
./ObjectBases/IGUITextOwner.cpp
./ObjectBases/IGUITextOwner.h
./ObjectTypes
./ObjectTypes/CButton.cpp
./ObjectTypes/CButton.h
./ObjectTypes/CChart.cpp
./ObjectTypes/CChart.h
./ObjectTypes/CCheckBox.cpp
./ObjectTypes/CCheckBox.h
./ObjectTypes/CDropDown.cpp
./ObjectTypes/CDropDown.h
./ObjectTypes/CGUIDummyObject.h
./ObjectTypes/CImage.cpp
./ObjectTypes/CImage.h
./ObjectTypes/CInput.cpp
./ObjectTypes/CInput.h
./ObjectTypes/CList.cpp
./ObjectTypes/CList.h
./ObjectTypes/CMiniMap.cpp
./ObjectTypes/CMiniMap.h
./ObjectTypes/COList.cpp
./ObjectTypes/COList.h
./ObjectTypes/CProgressBar.cpp
./ObjectTypes/CProgressBar.h
./ObjectTypes/CRadioButton.cpp
./ObjectTypes/CRadioButton.h
./ObjectTypes/CSlider.cpp
./ObjectTypes/CSlider.h
./ObjectTypes/CText.cpp
./ObjectTypes/CText.h
./ObjectTypes/CTooltip.cpp
./ObjectTypes/CTooltip.h
./Scripting
./Scripting/GuiScriptConversions.cpp
./Scripting/JSInterface_GUIManager.cpp
./Scripting/JSInterface_GUIManager.h
./Scripting/JSInterface_GUISize.cpp
./Scripting/JSInterface_GUISize.h
./Scripting/JSInterface_IGUIObject.cpp
./Scripting/JSInterface_IGUIObject.h
./Scripting/ScriptFunctions.cpp
./Scripting/ScriptFunctions.h
./SettingTypes
./SettingTypes/CGUIColor.cpp
./SettingTypes/CGUIColor.h
./SettingTypes/CGUIList.h
./SettingTypes/CGUISeries.h
./SettingTypes/CGUISize.cpp
./SettingTypes/CGUISize.h
./SettingTypes/CGUIString.cpp
./SettingTypes/CGUIString.h
./SettingTypes/EAlign.h
./SGUIIcon.h
./SGUIMessage.h
./SGUIStyle.h
./tests
./tests/test_GuiManager.cpp
./tests/test_GuiManager.h
./tests/test_ParseString.cpp
./tests/test_ParseString.h
vladislavbelov added a comment.EditedOct 2 2019, 12:13 PM

If you do a find in source/gui/ you will see that its more consistent if that folder uses the same style as the filenames.

See my comment above. And it doesn't look consistent, because it has tests folder.

vladislavbelov added a comment.EditedOct 2 2019, 12:19 PM

Before your change there were 100 folders with lower case name underscore and 2 with upper case in whole repo excluding third_party and tools. After your change there are 99 folders with lower case name underscore and 6 with upper case

See my comments above too. :P See also the comments in the CC:

Citation of the CC or you cant claim its in the CC. It actually says:

Well, before the change we didn't have such name in the folders, but we have tests and scripting. So you broke the rules from CC.

I claim this to be a lie unless you cite a rule from the CC that I broke https://trac.wildfiregames.com/wiki/Coding_Conventions
If you refer to the consistency argument, the consistency goes from bottom to top, first there is consistency in this folder, as I quoted above.

Well its not like I didnt mentioned the folder names or the reasoning several times in the discussion we had http://irclogs.wildfiregames.com/2019-10/2019-10-01-QuakeNet-%230ad-dev.log

20:02 < Vladislav> I agree with most, I just can't call it object_base.

It was in context of your patch https://code.wildfiregames.com/P183, and it contains only underscores.

And I stated immediately that I will call it ObjectBases not object_bases, like I did in the cited statements before, the cited statements after, and the differential revision proposal

You even proposed a non lowercase name yourself:

19:49 < elexis> I will implement ObjectTypes ObjectBases SettingTypes if there is no objection
19:50 < Vladislav> Why not just Objects?

Yes, the scripting folders in other places are called scripting, however in this folder the folders are capitalized and there is no binding contract (like there is with test folder) that forces us to use lowercase for scripting.
If the other folders and all filenames are starting with uppercase, then it is more consistent within this folder if this one does too.

First of all for a new folder we use other folder names style not file names.

And Ive given you several examples where uppercase foldernames already are used.

19:32 < elexis> actually I see 17 folders with underscore in the public mod, so perhaps object_bases object_types is good
19:32 < elexis> there is also setting_types
19:32 < elexis> (settings would be wrong, since the settings are the CGUISetting instances)
19:35 < elexis> or SettingTypes ObjectTypes ObjectBases
19:36 < elexis> similar to ./ps/GameSetup ./tools/lobbybots/XpartaMuPP ./tools/atlas/AtlasUI/ScenarioEditor/Sections/Object

So if you say global consistency supersedes local consistency then thats the opposite of what the CC say, and then you would actually argue against underscore, not for underscore, judging only by what is most commonly present.

After the first time I asked you about this:

2019-09/2019-09-23-QuakeNet-#0ad-dev.log:11:11 < elexis> Vladislav: would you agree to move classes that inherit IGUIObject to source/gui/objects/, and the data types they use (like CGUISeries.h) to source/gui/datatypes/, or source/gui/objects/types/?

I've precisely waited for you to come online again before writing this patch and discuss this with you to avoid you complaining afterwards.

2019-10-01-QuakeNet-%230ad-dev.log
19:14 < elexis> Vladislav: no objections?
19:49 < elexis> I will implement ObjectTypes ObjectBases SettingTypes if there is no objection
19:50 < Vladislav> Why not just Objects?
20:37 < Vladislav> Ok, then. I don't have strong objections for such movements, only tastes.

Before your change there were 100 folders with lower case name underscore and 2 with upper case in whole repo excluding third_party and tools. After your change there are 99 folders with lower case name underscore and 6 with upper case

And I never dispute that.

Look also at these new inconsitencies introduced, some code even uses the class keyword when everything else is procedural style.

./aiconfig/aiconfig.js
./civinfo/civinfo.js
./common/colorFades.js
./common/color.js
./common/functions_global_object.js
./common/functions_utility.js
./common/gamedescription.js
./common/music.js
./common/network.js
./common/settings.js
./common/tab_buttons.js
./common/timer.js
./common/tooltips.js
./credits/credits.js
./credits/texts/art.json
./credits/texts/donators.json
./credits/texts/misc.json
./credits/texts/programming.json
./credits/texts/special.json
./credits/texts/translators.json
./gamesetup/gamesetup.js
./gamesetup_mp/gamesetup_mp.js
./loadgame/SavegameDeleter.js
./loadgame/SavegameDetails.js
./loadgame/SavegameList.js
./loadgame/SavegameLoader.js
./loadgame/SavegamePage.js
./loadgame/SavegameWriter.js
./loading/loading.js
./lobby/lobby.js
./locale_advanced/locale_advanced.js
./locale/locale.js
./options/options.js
./options/options.json
./pregame/BackgroundHandler.js
./pregame/backgrounds/carthage.js
./pregame/backgrounds/hellenes.js
./pregame/backgrounds/kush.js
./pregame/backgrounds/seleucid.js
./pregame/MainMenuItemHandler.js
./pregame/MainMenuItems.js
./pregame/mainmenu.js
./pregame/ProjectInformation.js
./pregame/SplashscreenHandler.js
./pregame/userreport/userreport.js
./prelobby/common/credentials/credentials.js
./prelobby/common/feedback/feedback.js
./prelobby/common/terms/termslobby.js
./prelobby/entrance/entrance.js
./prelobby/login/login.js
./prelobby/register/register.js
./reference/common/core.js
./reference/common/draw.js
./reference/common/helper.js
./reference/common/load.js
./reference/structree/draw.js
./reference/structree/structree.js
./reference/viewer/viewer.js
./replaymenu/replay_actions.js
./replaymenu/replay_filters.js
./replaymenu/replay_menu.js
./session/developer_overlay.js
./session/input.js
./session/menu.js
./session/messages.js
./session/placement.js
./session/selection_details.js
./session/selection.js
./session/selection_panels_helpers.js
./session/selection_panels.js
./session/session.js
./session/unit_actions.js
./session/unit_commands.js
./splashscreen/splashscreen.js
./summary/counters.js
./summary/layout.js
./summary/summary.js

I claim this to be a lie unless you cite a rule from the CC that I broke https://trac.wildfiregames.com/wiki/Coding_Conventions
If you refer to the consistency argument, the consistency goes from bottom to top, first there is consistency in this folder, as I quoted above.

I refer to this line: "Then, try to match the style of the other code in the subdirectory you're editing.".

You even proposed a non lowercase name yourself:

19:49 < elexis> I will implement ObjectTypes ObjectBases SettingTypes if there is no objection
19:50 < Vladislav> Why not just Objects?

I claim this to be a lie unless you cite my proposition instead of question xD

And Ive given you several examples where uppercase foldernames already are used.

19:32 < elexis> actually I see 17 folders with underscore in the public mod, so perhaps object_bases object_types is good
19:32 < elexis> there is also setting_types
19:32 < elexis> (settings would be wrong, since the settings are the CGUISetting instances)
19:35 < elexis> or SettingTypes ObjectTypes ObjectBases
19:36 < elexis> similar to ./ps/GameSetup ./tools/lobbybots/XpartaMuPP ./tools/atlas/AtlasUI/ScenarioEditor/Sections/Object

As I said above we have smaller number of upper case folders.

So if you say global consistency supersedes local consistency then thats the opposite of what the CC say, and then you would actually argue against underscore, not for underscore, judging only by what is most commonly present.

I think it’s wrong, because number of underscores is bigger.

After the first time I asked you about this:

2019-09/2019-09-23-QuakeNet-#0ad-dev.log:11:11 < elexis> Vladislav: would you agree to move classes that inherit IGUIObject to source/gui/objects/, and the data types they use (like CGUISeries.h) to source/gui/datatypes/, or source/gui/objects/types/?

I've precisely waited for you to come online again before writing this patch and discuss this with you to avoid you complaining afterwards.

2019-10-01-QuakeNet-%230ad-dev.log
19:14 < elexis> Vladislav: no objections?

I didn’t have objections about the mentioned patch.

elexis added a comment.Oct 2 2019, 1:10 PM

There are find source/gui/ -type f | wc -l local reasons that motivate using title case to use the style that I have specifically and explicitly asked you to confirm before starting to write this patch.
The coding conventions prioritize local consistency over global consistency. I have posted here the comparison https://code.wildfiregames.com/rP23028#38612
I have also mentioned on IRC the precedences for that case.
If global consistency has the benefit that it is less irritating if there are differences, but this equally applies to the differences between file case and folder case.
An author who creates a patch to source/gui/ will spend less time being irritated by opposing styles for folder and filenames if they are not opposing but consistent.

When you spoke about object_bases I have immediately corrected you that it should be ObjectBases.

At last, if the objective of a diff is to improve code quality, then in some cases it may be contrary to the status quo represented in either global consistency or coding conventions, and then the latter should be updated.
Just because something has been done like this all the time doesn't mean that it can't be done better.
Which is why I had to break global consistency for the JS GUI as well #5387 in preference of gaining a better (less fragmented) codebase.

As I posted above, we already have source/GameSetup, is that bad so that it should be renamed because its not globally consistent?
Is ./tools/lobbybots/XpartaMuPP bad?
Is ./tools/atlas bad?

There are actually very few source folders.

./collada
./collada/tests
./graphics
./graphics/scripting
./graphics/tests
./gui
./gui/ObjectBases
./gui/ObjectTypes
./gui/Scripting
./gui/SettingTypes
./gui/tests
./i18n
./i18n/scripting
./lib
./lib/adts
./lib/allocators
./lib/allocators/tests
./lib/external_libraries
./lib/file
./lib/file/archive
./lib/file/archive/disabled_tests
./lib/file/common
./lib/file/common/tests
./lib/file/disabled_tests
./lib/file/io
./lib/file/vfs
./lib/file/vfs/tests
./lib/pch
./lib/posix
./lib/posix/tests
./lib/res
./lib/res/graphics
./lib/res/graphics/tests
./lib/sysdep
./lib/sysdep/arch
./lib/sysdep/arch/aarch64
./lib/sysdep/arch/amd64
./lib/sysdep/arch/arm
./lib/sysdep/arch/ia32
./lib/sysdep/arch/x86_x64
./lib/sysdep/arch/x86_x64/tests
./lib/sysdep/os
./lib/sysdep/os/android
./lib/sysdep/os/bsd
./lib/sysdep/os/linux
./lib/sysdep/os/osx
./lib/sysdep/os/unix
./lib/sysdep/os/unix/x
./lib/sysdep/os/win
./lib/sysdep/os/win/aken
./lib/sysdep/os/win/tests
./lib/sysdep/os/win/whrt
./lib/sysdep/os/win/wposix
./lib/sysdep/rtl
./lib/sysdep/rtl/gcc
./lib/sysdep/rtl/msc
./lib/sysdep/tests
./lib/tests
./lib/tex
./lobby
./lobby/glooxwrapper
./lobby/scripting
./maths
./maths/tests
./mocks
./network
./network/scripting
./network/tests
./pch
./pch/atlas
./pch/engine
./pch/glooxwrapper
./pch/graphics
./pch/gui
./pch/lobby
./pch/lowlevel
./pch/network
./pch/scriptinterface
./pch/simulation2
./pch/test
./pch/tinygettext
./ps
./ps/GameSetup
./ps/GameSetup/tests
./ps/scripting
./ps/tests
./ps/XML
./ps/XML/tests
./renderer
./renderer/scripting
./scriptinterface
./scriptinterface/tests
./scriptinterface/third_party
./simulation2
./simulation2/components
./simulation2/components/tests
./simulation2/docs
./simulation2/helpers
./simulation2/scripting
./simulation2/serialization
./simulation2/system
./simulation2/tests
./soundmanager
./soundmanager/data
./soundmanager/items
./soundmanager/scripting
./third_party
./third_party/cppformat
./third_party/encryption
./third_party/encryption/tests
./third_party/jsonspirit
./third_party/mikktspace
./third_party/mongoose
./third_party/tinygettext
./third_party/tinygettext/include
./third_party/tinygettext/include/tinygettext
./third_party/tinygettext/src
./third_party/tinygettext/src/win32
./tools
./tools/atlas
./tools/atlas/AtlasFrontends
./tools/atlas/AtlasObject
./tools/atlas/AtlasObject/tests
./tools/atlas/AtlasUI
./tools/atlas/AtlasUI/ActorEditor
./tools/atlas/AtlasUI/CustomControls
./tools/atlas/AtlasUI/CustomControls/Buttons
./tools/atlas/AtlasUI/CustomControls/Canvas
./tools/atlas/AtlasUI/CustomControls/ColorDialog
./tools/atlas/AtlasUI/CustomControls/DraggableListCtrl
./tools/atlas/AtlasUI/CustomControls/EditableListCtrl
./tools/atlas/AtlasUI/CustomControls/FileHistory
./tools/atlas/AtlasUI/CustomControls/HighResTimer
./tools/atlas/AtlasUI/CustomControls/MapDialog
./tools/atlas/AtlasUI/CustomControls/SnapSplitterWindow
./tools/atlas/AtlasUI/CustomControls/VirtualDirTreeCtrl
./tools/atlas/AtlasUI/CustomControls/Windows
./tools/atlas/AtlasUI/ErrorReporter
./tools/atlas/AtlasUI/General
./tools/atlas/AtlasUI/Misc
./tools/atlas/AtlasUI/Misc/Graphics
./tools/atlas/AtlasUI/ScenarioEditor
./tools/atlas/AtlasUI/ScenarioEditor/Sections
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Cinema
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Common
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Environment
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Map
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Object
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Player
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Terrain
./tools/atlas/AtlasUI/ScenarioEditor/Tools
./tools/atlas/AtlasUI/ScenarioEditor/Tools/Common
./tools/atlas/GameInterface
./tools/atlas/GameInterface/Handlers
./tools/autolog
./tools/autolog/SVNLog
./tools/autolog/SVNLog/lib
./tools/autolog/SVNLog/lib/SVNLog
./tools/autolog/SVNLog/lib/SVNLog/Controller
./tools/autolog/SVNLog/lib/SVNLog/Model
./tools/autolog/SVNLog/lib/SVNLog/Model/CDBI
./tools/autolog/SVNLog/lib/SVNLog/View
./tools/autolog/SVNLog/root
./tools/autolog/SVNLog/root/PublicMessage
./tools/autolog/SVNLog/script
./tools/autolog/SVNLog/sql
./tools/cmpgraph
./tools/dist
./tools/entdocs
./tools/entgraph
./tools/entity
./tools/fontbuilder2
./tools/i18n
./tools/i18n/extractors
./tools/lobbybots
./tools/lobbybots/EcheLOn
./tools/lobbybots/mod_ipstamp
./tools/lobbybots/mod_ipstamp/src
./tools/lobbybots/XpartaMuPP
./tools/mapcompatibility
./tools/openlogsfolder
./tools/profiler2
./tools/replayprofile
./tools/selectiontexgen
./tools/springimport
./tools/templatesanalyzer
./tools/templatesanalyzer/tablefilter
./tools/templatesanalyzer/tablefilter/style
./tools/templatesanalyzer/tablefilter/style/themes
./tools/templatesanalyzer/tablefilter/style/themes/default
./tools/templatesanalyzer/tablefilter/style/themes/default/images
./tools/templatesanalyzer/tablefilter/style/themes/mytheme
./tools/templatesanalyzer/tablefilter/style/themes/mytheme/images
./tools/templatesanalyzer/tablefilter/style/themes/skyblue
./tools/templatesanalyzer/tablefilter/style/themes/skyblue/images
./tools/templatessorter
./tools/tracelogger
./tools/webservices
./tools/webservices/userreport
./tools/webservices/userreport/templates
./tools/webservices/userreport/templates/reports
./tools/webservices/userreport/templatetags
./tools/xmlvalidator

When you spoke about object_bases I have immediately corrected you that it should be ObjectBases.

I didn't talk about underscores, only about name semantic.

As I posted above, we already have source/GameSetup, is that bad so that it should be renamed because its not globally consistent?
Is ./tools/lobbybots/XpartaMuPP bad?
Is ./tools/atlas bad?

I think it's bad to have them out of common names.

elexis added a comment.Oct 2 2019, 4:03 PM

When you spoke about object_bases I have immediately corrected you that it should be ObjectBases.

I didn't talk about underscores, only about name semantic.

That is precisely the problem, because I explicitly asked for the name capitalization.

19:32 < elexis> actually I see 17 folders with underscore in the public mod, so perhaps object_bases object_types is good
19:32 < elexis> there is also setting_types
19:32 < elexis> (settings would be wrong, since the settings are the CGUISetting instances)
19:35 < elexis> or SettingTypes ObjectTypes ObjectBases
19:36 < elexis> similar to ./ps/GameSetup ./tools/lobbybots/XpartaMuPP  ./tools/atlas/AtlasUI/ScenarioEditor/Sections/Object
19:49 < elexis> I will implement ObjectTypes ObjectBases SettingTypes if there is no objection 
19:50 < Vladislav> Why not just Objects?
19:50 < elexis> because Objects are the instances
19:50 < elexis> equally SettingTypes because Settings are the instances
19:51 < elexis> also allows distinction of ObjectBases from ObjectTypes
19:51 < elexis> also ObjectType is more strict than Object, because someone might add things that relate to Obejcts but are not ObjectTypes
19:52 < Vladislav> I mean we have not so many objects to put in different folders.
19:52 < elexis> if its in that folder, it should be an IGUIObject inheriting class
19:53 < elexis> 8, 12, 29 directory items
19:57 < elexis> I'll start typing then
20:02 < Vladislav> I agree with most, I just can't call it object_base.
20:02 < elexis> ObjectBase
20:02 < elexis> s
20:06 < elexis> if a class inherits IGUIObject and can be instantiated from XML it goes into ObjectTypes, like CList
20:06 < elexis> ObjectBases are the ones who are only bases
20:08 < elexis> the ObjectTypes have to coordinate that they call all virtual functions of base classes
20:12 < elexis> having the two different folders is important, because it allows one to traverse only files from one of the two categories when working with the code
20:14 < elexis> SettingTypes/CGUISprite.h perhaps thats more than a setting type
20:33 < Vladislav> elexis: "a base class is one that is inherited", what if we want just a helper without inheritance?
20:36 < elexis> then its like IGUIScrollBar.h or GUITooltip.h and goes into source/gui/
20:37 < elexis> (and probably asks to be refactored)
20:37 < Vladislav> Ok, then. I don't have strong objections for such movements, only tastes.

This is also what I had uploaded
21:40 < WildfireBot> e​l​e​x​i​s created D2345 (Sort GUI files into ObjectTypes ObjectBases SettingTypes subdirectories) https://code.wildfiregames.com/D2345

As I posted above, we already have source/GameSetup, is that bad so that it should be renamed because its not globally consistent?
Is ./tools/lobbybots/XpartaMuPP bad?
Is ./tools/atlas bad?

I think it's bad to have them out of common names.

Literally?
So you are actually saying this is bad:

./tools/atlas
./tools/atlas/AtlasFrontends
./tools/atlas/AtlasObject
./tools/atlas/AtlasObject/tests
./tools/atlas/AtlasUI
./tools/atlas/AtlasUI/ActorEditor
./tools/atlas/AtlasUI/CustomControls
./tools/atlas/AtlasUI/CustomControls/Buttons
./tools/atlas/AtlasUI/CustomControls/Canvas
./tools/atlas/AtlasUI/CustomControls/ColorDialog
./tools/atlas/AtlasUI/CustomControls/DraggableListCtrl
./tools/atlas/AtlasUI/CustomControls/EditableListCtrl
./tools/atlas/AtlasUI/CustomControls/FileHistory
./tools/atlas/AtlasUI/CustomControls/HighResTimer
./tools/atlas/AtlasUI/CustomControls/MapDialog
./tools/atlas/AtlasUI/CustomControls/SnapSplitterWindow
./tools/atlas/AtlasUI/CustomControls/VirtualDirTreeCtrl
./tools/atlas/AtlasUI/CustomControls/Windows
./tools/atlas/AtlasUI/ErrorReporter
./tools/atlas/AtlasUI/General
./tools/atlas/AtlasUI/Misc
./tools/atlas/AtlasUI/Misc/Graphics
./tools/atlas/AtlasUI/ScenarioEditor
./tools/atlas/AtlasUI/ScenarioEditor/Sections
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Cinema
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Common
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Environment
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Map
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Object
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Player
./tools/atlas/AtlasUI/ScenarioEditor/Sections/Terrain
./tools/atlas/AtlasUI/ScenarioEditor/Tools
./tools/atlas/AtlasUI/ScenarioEditor/Tools/Common
./tools/atlas/GameInterface
./tools/atlas/GameInterface/Handlers

and should be renamed to

./tools/atlas
./tools/atlas/atlasfrontends
./tools/atlas/atlasobject
./tools/atlas/atlasobject/tests
./tools/atlas/atlasui
./tools/atlas/atlasui/actoreditor
./tools/atlas/atlasui/customcontrols
./tools/atlas/atlasui/customcontrols/buttons
./tools/atlas/atlasui/customcontrols/canvas
./tools/atlas/atlasui/customcontrols/colordialog
./tools/atlas/atlasui/customcontrols/draggablelistctrl
./tools/atlas/atlasui/customcontrols/editablelistctrl
./tools/atlas/atlasui/customcontrols/filehistory
./tools/atlas/atlasui/customcontrols/highrestimer
./tools/atlas/atlasui/customcontrols/mapdialog
./tools/atlas/atlasui/customcontrols/snapsplitterwindow
./tools/atlas/atlasui/customcontrols/virtualdirtreectrl
./tools/atlas/atlasui/customcontrols/windows
./tools/atlas/atlasui/errorreporter
./tools/atlas/atlasui/general
./tools/atlas/atlasui/misc
./tools/atlas/atlasui/misc/graphics
./tools/atlas/atlasui/scenarioeditor
./tools/atlas/atlasui/scenarioeditor/sections
./tools/atlas/atlasui/scenarioeditor/sections/cinema
./tools/atlas/atlasui/scenarioeditor/sections/common
./tools/atlas/atlasui/scenarioeditor/sections/environment
./tools/atlas/atlasui/scenarioeditor/sections/map
./tools/atlas/atlasui/scenarioeditor/sections/object
./tools/atlas/atlasui/scenarioeditor/sections/player
./tools/atlas/atlasui/scenarioeditor/sections/terrain
./tools/atlas/atlasui/scenarioeditor/tools
./tools/atlas/atlasui/scenarioeditor/tools/common

just to make it globally consistent?