We-the-People-civ4col-mod / Mod

This is the repository where the mod resides.
91 stars 37 forks source link

Clean way to handle WidgetTypes #929

Open Nightinggale opened 1 year ago

Nightinggale commented 1 year ago

WidgetTypes can do with a cleanup. It's currently spread across the source code (both C++ and python) what the meaning of each is.

I want to make a class for each Widget type and it would look like this:

class WidgetHandler
{
    virtual void parseHelp(CvWStringBuffer &szBuffer, CvWidgetDataStruct &widgetDataStruct);
    virtual bool executeAction( CvWidgetDataStruct &widgetDataStruct);
    virtual bool executeAltAction( CvWidgetDataStruct &widgetDataStruct);
    virtual bool executeDropOn(const CvWidgetDataStruct& destinationWidgetData, const CvWidgetDataStruct& sourceWidgetData);
    virtual bool executeDoubleClick(const CvWidgetDataStruct& destinationWidgetData);
    virtual bool isLink(const CvWidgetDataStruct &widgetDataStruct);
}

Those are the functions the EXE calls in the DLL. By making a base class and letting each widget have a child class, widget specific code will no longer need to be in the code called by the exe and instead can be put in a single class. This way everything related to a widget will be in a single location. I already implemented a similar system for ButtonPopup windows (not in develop yet through).

One aspect of this design is still undecided through and that is CvWidgetDataStruct. The layout is obviously set already as it's part of the exe interface.

struct DllExport CvWidgetDataStruct
{
    int m_iData1;                                       //  The first bit of data
    int m_iData2;                                       //  The second piece of data

    bool m_bOption;                                 //  A boolean piece of data

    WidgetTypes m_eWidgetType;          //  What the 'type' of this widget is (for parsing help and executing actions)
};

How do we set iData1+2 in a way, which makes it clear what they are supposed to represent? What is bOption doing and how do we (un)set it?

My first thought was to bake the info decoding into the new widget class, but then it would only be available inside CvDLLWidgetData.cpp and as such not where the widget data is created. That won't do us any good.

Second thought was to create a struct for each widget, each mapping between itself and CvWidgetDataStruct. The side effect of that would be that each widget would now need two classes as opposed to one and in two files.

Also how do python "learn" how to make widgets using this approach? A lot of the widget placement code is in python rather than C++.

Would it make sense to (script?) generate a class for each widget in python and then use such an instance to fill out the data using type correct names. If so, then we can make methods for adding widgets, which takes a class instance instead of each variable (essentially a wrapper method) and that one can have getData1 and such to convert to the struct layout.

If this is done right, then we can unlock the power to use widgets way more than how they are used today. We do have some nice features build into widgets already and we can add more if we like. However with no overview, everybody tend to skip past widgets and create a custom solution each time a widget is needed. That's messy and will result in inferior solutions.

Nightinggale commented 1 year ago

I think I figured out how to do this. The answer isn't to push everything into one file. The answer is to write code in a single file and then script add the rest of the code.

class WidgetData<WIDGET_UNIT_SOMETHING>
{
    const UnitTypes m_eUnit;
    const YieldTypes m_YieldRequired;
public:
    WidgetData(const CvWidgetDataStruct& destinationWidgetData)
        : m_eUnit(destinationWidgetData.m_iData1)
        , m_YieldRequired(destinationWidgetData.m_iData2);

Note: typecasting has intentionally been ignored due to readability.

This will then script generate a python class, like

class WidgetContainer
    def __init__(self):
        self.m_eUnit = -1
        self.m_YieldRequired = -1

    def getWidget(self):
        return WIDGET.WIDGET_UNIT_SOMETHING

    def getData1(self):
        return self.m_eUnit

    def getData2(self):
        return self.m_YieldRequired 

Now if an instance of this class is returned from a function, which takes a widget as argument, then python can set up iData1+2 using the variable names from C++. Also rather than calling the exe directly, python can have a wrapper class, which takes an instance of this new class, hence 3 arguments will be replaced by a single one.

If each new class has the 3 get functions using those names, then the GUI wrapper class can expand that one argument into the 3 needed by the exe.

Python has some attribute magic to avoid adding new attributes to be added, hence either use the C++ variable names or throw an error.

johan-buret commented 1 year ago

https://github.com/We-the-People-civ4col-mod/Mod/wiki/Widget-creation to document widget creation