zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
116 stars 24 forks source link

Command/Tool system is of two minds (tool subindex is ignored) #57

Open m-7761 opened 5 years ago

m-7761 commented 5 years ago

FWIW this code hardcodes 0 as the tool subindex. It must work because there are not multipart Tools.

https://github.com/zturtleman/mm3d/blob/d1b429433394488ba4f7c04d9cb26fa36c9635dc/src/implui/viewwin.cc#L1970

This system is confusing because there is "getPath" and also the 0th subindex is also the group name. Both of these are the same from what I can see. I can't fathom which came first, or why both approaches coexist.

I suppose plugins could use "getPath" to inject tools into submenus. How many plugins are there? From a maintenance perspective I prefer the other system. I will consolidate the tools that use "getPath" to not rely on it. The code is a few lines involving no additional data structures that way.

(By comparison, below is my simplified main-window object. glut_window_id is GLUT emulated with wxWidgets. The main-window as written is a plain GLUT window, decorated by UI subwindows. It's OpenGL context is the central drawing surface. There are 3 subwindows, plus the animation window, unless I decide to run the sidebar all the way to the top. The animation window is on top, so that the status bar runs along the bottom.)

class MainWin : Model::Observer
{   
public:

    // Model::Observer method
    virtual void modelChanged(int changeBits);

    MainWin(Model*); ~MainWin();

    static bool quit();

    static bool open(utf8 file, MainWin *window=NULL);

    static bool save(Model *model, bool expdir=false);

    static void merge(Model *model, bool anims=false);

    void reshape(int x, int y);

    void frame(Model::OperationScopeE=Model::OS_Global);

    void undo(),redo();

    bool save_work();
    bool save_work_prompt();

public:

    Model *const model;

    const int glut_window_id;

    Toolbox toolbox;
    SideBar sidebar;
    ViewPanel views; 

    void open_texture_window();
    void open_transform_window();
    void open_animation_window();
    void open_projection_window();

    void perform_menu_action(int);

private:

    void _init_menu_toolbar();  

    Model *_swap_models(Model*);

    class AnimWindow *_animation_win;
    class TransformWin *_transform_win;
    class ProjectionWin *_projection_win;
    class TextureCoordWin *_texturecoord_win;

    //These menus have state.
    //NOTE: It's pointless to separate them as long
    //as the global "config" file holds their state.
    int _view_menu, _snap_menu, _rops_menu;
    int _anim_menu, _menubar;
};
m-7761 commented 5 years ago

For the record the "key binding" stuff should be removed from Tool and Command classes. It's not used. I've done this. I think I'm going to make these objects to not touch any Qt code because they are large chunks of the library that shouldn't depend on the UI component to function.

Some of them generate windows. What I've been doing is to remove the headers for window classes and put a global function in their definition files, just like how the model filter definitions work. That way the tools don't need to know anything about the window implementation, and there are fewer files to sift through.

I feel like it's probably best to remove the path system too. It can always be restored if it feels warranted. Presently it's not required to realize the existing functionality. I think it's needless complexity, and if it were to be used, it would mean there are either more than one level of submenus (which seems clunky) or not a clear division of ownership of the menus. Like if a plugin adds a menu item, it's probably better if it's clear the functionality belongs to that plugin. The path system would allow the plugin to mingle its items with built-in functionality or other plugins' items.

m-7761 commented 5 years ago

FWIW I can see better now how getPath is used.

It should be used to sort the containers instead of the generated menus, so that the IDs in the menus match the containers'. Then it's not as hard to implement.

P.S. I've replaced the toolwidget stuff by adding some methods to Tool::Parent like addInt, addBool, so that the tool system is independent of the UI. I was going to put the tool parameters into the sidebar, where they seem like a better fit to me. But they would not be bad running across the top as they are now, I think, if there were always something occupying that space, so that it doesn't disrupt the main area layout.

The only problem with that, is the animation bar sandwiches itself between the toolbar and the main area. If it disabled the use of tools, it would not be so bad, But I think it would ultimately be useful to use tools in animation mode. I don't like the animation slider on the bottom because it goes below the status bar.

What I think could work is to make the animation slider a tool, and put the animation controls in the sidebar. In that case the slider tool is just be a visual way to move the animation position. And the real memory of the position is in the sidebar.