Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ViewModel._update() does not support plugins with multiple views / views with PluginName_ViewName nomenclature #744

Closed
Tyronnosaurus opened this issue Oct 26, 2021 · 6 comments

Comments

@Tyronnosaurus
Copy link
Contributor

I found this possible bug trying to solve another issue. More info in this Cura forums thread.


First, some info we need to know:

  • If we create a plugin with one View, its ID is the plugin's name.
  • If we create a plugin with 2 or more Views, in order to differentiate between them we need to assign a self.name value to each. Then, their ID is PluginName_ViewName. See more info in this Cura Forums thread and in the method PluginObject.getId()
    This system ensures no two Views ever have the same name, no matter how many plugins we install.

The actual bug:

ViewModel._update() has no support for the PluginName_ViewName nomenclature:

        views = self._controller.getAllViews()
        ...
        for view_id in views:
            view_meta_data = PluginRegistry.getInstance().getMetaData(view_id).get("view", {})

If we've got a view called MyPlugin_View1, then getMetaData(view_id) will try to find a plugin called "MyPlugin_View1" instead of just "MyPlugin".

Moreover, even if it found the plugin in question, get() would return a list instead of a dict, due to the fact that in the file plugin.json you can declare either one view (a dict) or multiple views (list of dicts). We'd need to filter every dict to find the one where key 'name' is equal to MyView.name (both are set manually, so the developer must ensure they're the same).


Possible solution:

        views = self._controller.getAllViews()
        ...
        for view_id in views:
          #Views may be named as "PluginName" or "PluginName_ViewName"
          plugin_name = view_id.split('_')[0]    #Get the part before '_', or all the string if there's no '_'
          view_meta_data = PluginRegistry.getInstance().getMetaData(plugin_name).get("view", {})
          if isinstance(view_meta_data,list):      #Multiple views in the plugin
            view_name = view_id.split('_')[1]     #Get the part after '_'
            view_meta_data = list(filter(lambda view: view['name'] == view_name , view_meta_data))[0]  #Filter for specific View

Tested on Windows 10, running Cura from source on latest version.

@Tyronnosaurus
Copy link
Contributor Author

In the meantime, I'd appreciate if anyone thinks of a workaround that can be implemented through a plugin instead of waiting for a future version of Uranium&Cura.

@nallath
Copy link
Member

nallath commented Oct 26, 2021

Ah, yes, now I remember. See fb8bde4 for a related change. It seems that we only enabled this for tools, so sorry for the confusion that I generated on the forums!

I think we just never updated this to also work with views as well.

@Tyronnosaurus
Copy link
Contributor Author

No worries, I appreciate all the help I've been getting from you and the complexity of developing for Cura.

we only enabled this for tools

As in, multiple tools per plugin are supported, but multiple views were never officially supported? If so, I guess I should use only one View per plugin for now.

From my end I've only encountered this one error related to multiple views. Maybe we should consider PR'ing the solution above and adding official support for multiple views?

@nallath
Copy link
Member

nallath commented Oct 27, 2021

As in, multiple tools per plugin are supported, but multiple views were never officially supported?

Correct. We should have just enabled both while we were at it. I consider this to be an ovesight on our side.

From my end I've only encountered this one error related to multiple views. Maybe we should consider PR'ing the solution above and adding official support for multiple views?

Please do! A PR would save us time :)

@Tyronnosaurus
Copy link
Contributor Author

Sure, I'll get on it. Just one detail:

  ...
  view_meta_data = list(filter(lambda view: view['name'] == view_name , view_meta_data))[0]  #Filter for specific View

For this filtering to work, these two items...

  • 'name' in the metadata dictionary from the plugin's __init__()
  • self._name in the View's instance (this is what's used for PluginName_ViewName nomenclature in PluginObject.getId())

... must be manually set with the same string value by the developer.
I'm studying the plugin registering process, to see if I can take advantage of the order in which Views are loaded, rather than filtering by name.

@Tyronnosaurus
Copy link
Contributor Author

Solved in PR #745

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants