Currently Online

Latest Posts

Topic: When to namespace UI {...}?

jmoerschbach
Avatar
Joined: 2019-09-26, 21:40
Posts: 24
Ranking
Pry about Widelands
Posted at: 2020-10-27, 18:12

Just wondering when to put a class into the UI namespace. First, I thought all basic widgets (ui_basics) should the only ones being in UI namespace, but then I found helpwindow.cc (ui_fsmenu), wordwrap.cc (graphic), helpwindow (wui), suggested_teams_box (wui) and several others... Is there a rule I miss? Should for example all menu classes (ui_fsmenu) also in UI namespace?


Top Quote
Nordfriese
Avatar
Joined: 2017-01-17, 18:07
Posts: 1089
Ranking
One Elder of Players
Location: 0x55555d3a34c0
Posted at: 2020-10-27, 19:44

I guess it would be better style if everything was in a named namespace. So +1 for moving all UI-related classes to the UI namespace.

Perhaps it might be cleaner though to refactor this into three namespaces: Wui for in-game/in-editor, FsMenu for the main menu UI, and UI for basic widgets and shared code. And to let the directory structure reflect this, currently the shared code is cluttered together with the pure Wui stuff or in one case part of the editor code. This is probably worth a cleanup, not sure…


Top Quote
GunChleoc
Avatar
Joined: 2013-10-07, 15:56
Posts: 3320
Ranking
One Elder of Players
Location: RenderedRect
Posted at: 2020-10-28, 10:33

Wui for in-game/in-editor, FsMenu for the main menu UI, and UI for basic widgets and shared code.

Definitely worth a cleanup.

You can leave the wordwrap alone, we want to get rid of it. I have a branch open, but we were extremely short-handed on code reviews at the time, so it's sort of on hold until I get around to working on it again https://github.com/gunchleoc/widelands/tree/refactor-wordwrap


Busy indexing nil values

Top Quote
jmoerschbach
Avatar
Joined: 2019-09-26, 21:40
Posts: 24
Ranking
Pry about Widelands
Posted at: 2020-11-01, 22:01

should it be a nested namespace like namespace UI { namespace FsMenu { ... } } or just new flat namespace FsMenu for the menus?


Top Quote
Nordfriese
Avatar
Joined: 2017-01-17, 18:07
Posts: 1089
Ranking
One Elder of Players
Location: 0x55555d3a34c0
Posted at: 2020-11-01, 22:09

No nested namespaces please, this is just extra typing for no gain. Plain namespace FsMenu would be preferable IMHO.


Top Quote
GunChleoc
Avatar
Joined: 2013-10-07, 15:56
Posts: 3320
Ranking
One Elder of Players
Location: RenderedRect
Posted at: 2020-11-03, 10:59

Plain namespace FsMenu would be preferable IMHO.

+1


Busy indexing nil values

Top Quote