widelands-dev team mailing list archive

[Holger Rapp <HolgerRapp@xxxxxxx>]

> On 05.08.2015, at 17:12, Fòram na Gàidhlig <fios@xxxxxxxxxxxxxxxxxxx> wrote:
> 
> I am working on a new branch to handle keypresses. This let me to do
> some refactoring on the return codes, to give them all names rather than
> using plain numbers.
> 
> At the moment, I have 2 different types of named codes:
> 
> 1. Two const int values in UI::Panel as generic codes (dying_code,
> ok_code) [1]

these seem generally useful and can probably live in ui_basic. 

> 
> 2. An enum class in FullscreenMenuBase with Widelands-specific codes
> (kTutorial, kSinglePlayer etc.) [2]
> 
> I think we should have one global enum class with all the return codes
> in it, and use it as the argument/return type for Panel::run() and
> Panel::end_modal(). This way, we could get rid of all plain numbers and
> hodgepodge scattered enums permanently via type safety.

Well, only enum class is type safe. enum are implicitly casted to int’s and so you can compare any with any. Having one global enum for all cases means tighter coupling of the code to each other. I am not sure if that is a better design - but it is hard to say without seeing the full code.

One solution to having the enum outside of ui_basic, but using it inside it is templating the Panel::run method on it’s return type:

template <typename T>
T run();

then calling it:

MyCoolPanel p;
p.run<MyCoolEnum>();

or you pull out an interface that the current enums need to implement.


> 
> However, we want to keep ui_basic from being Widelands-specific. So,
> where is the best way to put them?
> 
> 
> 
> [1]
> https://bazaar.launchpad.net/~widelands-dev/widelands/bug-1480937/view/head:/src/ui_basic/panel.h#L91
> 
> [2]
> https://bazaar.launchpad.net/~widelands-dev/widelands/bug-1480937/view/head:/src/ui_fsmenu/base.h#L43
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~widelands-dev
> Post to     : widelands-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~widelands-dev
> More help   : https://help.launchpad.net/ListHelp