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

Feedback plug-ins #142

Closed
mottosso opened this issue Feb 7, 2015 · 15 comments
Closed

Feedback plug-ins #142

mottosso opened this issue Feb 7, 2015 · 15 comments

Comments

@mottosso
Copy link
Member

mottosso commented Feb 7, 2015

Goal

To facilitate software agnosticism by separating actions that rely on a host from general-purpose plug-ins.

For example, consider a Validate Naming Convention plug-in. The process may look something like this.

  1. Selection gathers names from host
  2. Names are validated, some are invalid
  3. Feedback plug-ins call upon host libraries to perform an interactive selection of invalid names.

In this way, Validators can remain software-agnostic whilst still providing the user with software-specific feedback.

Implementation

Feedback plug-ins is responsible for providing a user with interactive feedback; such as selecting an invalid asset or parts of an asset, such as vertices, pixels or channels.

They would run after Validator and only during validation failure.

import pyblish.api
import maya.cmds

class FeedbackNamingConvention(pyblish.api.Feedback):
    hosts = ["maya"]
    families = ["asset"]
    order = 4
    def process_instance(self, instance):
        cmds.select(instance)
@ghost
Copy link

ghost commented Feb 7, 2015

Great Idea! 👍

@BigRoy
Copy link
Member

BigRoy commented Feb 8, 2015

The hard thing for me (to think about) is how the Feedback is a completely separate plug-in that is ordered after all Validations even though it is so tightly connected to a Validator (a Validator fails and a user requests its corresponding feedback).

I've been thinking about how one would manage 'actions' that can be performed on a Plug-in that can be expanded by plug-in developers. Plus directly work with the UI (so the action pops up there in a context menu). Consider that Feedback wouldn't be a completely separate plug-in that gets run in a specific order after Validation, but it is a PluginAction that can be added to a certain type of plug-in. This way an artist can trigger the Feedback (as an Action) of a specific Validator whenever he wants.

This would at the same time also cover how Validator repairs could work, we just implement an action for Repairing it. The UI could show small buttons or context menu on right mouse click to provide the user with all possible actions. Also Actions could be 'host' specific so some might only show up when working with a certain host application, or have a differing implementation per host type.

What do you think?

Some pseudocode:

import pyblish.api
import maya.cmds


class FeedbackAction(pyblish.api.PluginAction):
    title = 'feedback'
    annotation = 'Raise feedback for this validator'
    hosts = ["maya"]

    def process_instance(self, instance):
        pass


class RepairAction(pyblish.api.PluginAction):
    title = 'repair'
    annotation = 'Try to auto-repair invalid nodes for this validator'

    def process_instance(self, instance):
        pass


class ValidateNamingConventions(pyblish.api.Validator):
    hosts = ["maya"]
    families = ["asset"]
    order = 4

    def initActions(self):
        self.addAction(RepairAction)
        self.addAction(FeedbackAction)

    def process_instance(self, instance):
        cmds.select(instance)

This could even allow developers to add a suite of proprietary actions for their plug-ins. For example add an Action that quickly brings the artist to the document with all the naming conventions so they have the reference right there. Basically it allows you to expand the functionality for each individual plug-in. Maybe one could even add a certain action to all plug-ins of a specific class type (eg. all validators).

@mottosso
Copy link
Member Author

mottosso commented Feb 8, 2015

Thanks for the feedback @BigRoy!

First off, I like the idea of attaching things to plug-ins; it promotes separation of concerns and re-usability which is definitely something we could have a look at implementing. Having said that, I'd vouch for keeping features to a bare minimum for as long as possible and only add when necessary.

Before looking at new features, let's consider how a Feedback plug-in might look using only what we have available today.

Currently                               |           Proposal
                                        |
 ___________                            |            ___________                                
|           |                           |           |           |                               
| Selection |                           |           | Selection |                               
|___________|                           |           |___________|                               
      |                                 |                 |                                     
      |                                 |                 |                                     
 _____v______                           |            _____v______                               
|            |                          |           |            |                              
| Validation |--- failed ----           |           | Validation |--- failed ----               
|____________|               |          |           |____________|               |              
      |                      |          |                 |                      |              
      |                      |          |                 |                ______|_______       
  succeeded                  |          |             succeeded           |              |      
      |                      |          |                 |               |   Feedback   |      
      |                      |          |                 |               |______________|      
 _____v______                |          |            _____v______                |              
|            |               |          |           |            |               |              
| Extraction |               |          |           | Extraction |               |              
|____________|               |          |           |____________|               |              
      |                      |          |                 |                      |              
      |                      |          |                 |                      |              
      |                      |          |                 |                      |              
 _____v_____                 |          |            _____v_____                 |              
|           |                |          |           |           |                |              
|  Conform  |                |          |           |  Conform  |                |              
|___________|                |          |           |___________|                |              
      |                      |          |                 |                      |              
      |                      |          |                 |                      |              
      v                      |          |                 v                      |              
     exit <------------------           |                exit <------------------               
                                        |   
                                        |   
                                        |           

And here's a working example.

import pyblish.api


# Mock up Feedbacker plug-in.
pyblish.api.Feedbacker = pyblish.api.Validator


class ValidateNamingConvention(pyblish.api.Validator):
    hosts = ["python"]
    families = ["asset"]

    def process_instance(self, instance):
        if instance.name != "Correct Name":
            instance.set_data("userFeedback", "namingConvention")
            raise pyblish.api.ValidationError(
                "%s did not have the correct name" % instance.name)


class FeedbackNamingConvention(pyblish.api.Feedbacker):
    hosts = ["python"]
    families = ["asset"]

    def process_instance(self, instance):
        if instance.data("userFeedback") == "namingConvention":
            print ("Validation failed, so "
                   "I am selecting \"%s\"." % instance.name)


# Simulate selection
context = pyblish.api.Context()
instance = context.create_instance(name="Incorrect Name")
# instance = context.create_instance(name="Correct Name")  # Uncomment to test working case
instance.set_data("family", "asset")

# Simulate pyblish.main.publish()
for instance, error in ValidateNamingConvention().process(context):
    if error:
        for instance, error in FeedbackNamingConvention().process(context):
            if error:
                raise error

Results

INFO - Processing instance: "Incorrect Name"
INFO - Processing instance: "Incorrect Name"
Validation failed, so I am selecting "Incorrect Name".

Does that make sense to you? I'd love to make your suggestion into a new issue, it's something I haven't considered before and can definitely see use in.

@BigRoy
Copy link
Member

BigRoy commented Feb 8, 2015

@mottosso Thanks for the example. It could definitely work, though I still feel there's no need to keep FeedbackNamingConvention that separate since it has little use on its own.

I can see how one could have multiple Validators setting instance data to "invalid mesh" and then a single Feedbacker select all invalid meshes that came from all Validators. Yet from an artist/user perspective I would prefer to get individual feedback per Validator to actually understand what is wrong with the feedback I received. Once we get to the point where we hook up a single Feedbacker per Validator then I see no point in keeping them separate.

@mottosso
Copy link
Member Author

mottosso commented Feb 8, 2015

I can see how one could have multiple Validators setting instance data to "invalid mesh" and then a single Feedbacker select all invalid meshes that came from all Validators.

Sounds good to me. :)

@mottosso
Copy link
Member Author

mottosso commented Feb 8, 2015

Yet from an artist/user perspective I would prefer to get individual feedback per Validator to actually understand what is wrong with the feedback I received.

I suppose you'll have to pick your poison. Either you have the "universal" Feedbacker as mentioned above, or you choose to have one coupled to a particular Validator. Either way sounds good to me. :)

@mottosso
Copy link
Member Author

mottosso commented Feb 8, 2015

Once we get to the point where we hook up a single Feedbacker per Validator then I see no point in keeping them separate.

That's true, this proposal might completely cave in on itself and not need an implementation. It's good to have a discussion about it I think.

@mottosso
Copy link
Member Author

mottosso commented Feb 8, 2015

I've been thinking about how one would manage 'actions' that can be performed on a Plug-in that can be expanded by plug-in developers.

Maybe this could somehow be merged with this: #94?

@BigRoy
Copy link
Member

BigRoy commented Feb 8, 2015

Maybe this could somehow be merged with this: #94?

I would consider that a different topic. The idea of adding an Action to a Plug-in is to allow a specific new 'functionality' that operates on that Plug-in (or type of Plug-in). Kind of like how one could add a menu item to context menus in an application. I think it's mostly unrelated to the hierarchy issue. Right?

@mottosso
Copy link
Member Author

mottosso commented Feb 8, 2015

Maybe, I'm just spawning.

ValidateNamingConvention:
 - ProvideLinkToDocs
 - RepairNames
 - ValidateNameIsVerb:
   - ProvideLinkToValidVerbs
   - ...

:)

@BigRoy
Copy link
Member

BigRoy commented Feb 8, 2015

Maybe, I'm just spawning.

I've yet to see how they get implemented in the same manner (so how they are actually the same). Does ValidateNameIsVerb require ValidateNamingConvention? If so then I think one is an Extension on a Validator (a way to maybe add building blocks into a Validator's process?) as opposed to just an Action operating on it. (Not sure if that terminology actually helps clarify my view.)

This was referenced Oct 21, 2015
mottosso added a commit to mottosso/pyblish-base that referenced this issue Oct 22, 2015
@mottosso
Copy link
Member Author

image

Overview

This functionality is meant to replace "repair", along with adding an abundance of flexibility in terms of context-sensitive functionality. Attach any functionality to a plug-in and tailor it to a particular state; like an action only available via a failed validator, or a successful extraction, or just all-round functionality associated with a particular plug-in.

Each action have access to both the Context and it's parent plug-in via dependency injection, along with any other custom dependencies already available to plug-ins in general.

Actions in QML are arranged in a menu with optional customisable groups and separators. Actions with any kind of implementation error show up as well, including a helpful error message for simplified debugging.

Full list of features

  • Per-plugin actions
  • Action API ~= Plug-in API, it is more or less a 1-1 match between their interfaces, including process() and label.
  • Standard logging and exception reporting, identical to plug-ins
  • Standard dependency injection still applies; can still inject custom functionality
  • Customisable icon per action, from Awesome Icon
  • Customisable availability
    • all: Always
    • processed: After plug-in has been processed
    • failed: After plug-in has been processed, and failed
    • succeeded: After plug-in has been processed, and succeeded

Basic use

class OpenInExplorer(pyblish.api.Action):
    label = "Open in Explorer"
    on = "failed"  # This action is only available on a failed plug-in
    icon = "hand-o-up"  # Icon from Awesome Icon

    def process(self, context):
        import subprocess
        subprocess.call("start .", shell=True)  # Launch explorer at the cwd


class Validate(pyblish.api.Validator):
    actions = [
        # Order of items is preserved
        pyblish.api.Category("My Actions"),
        MyAction,
        pyblish.api.Separator,
    ]

    def process(self, context, plugin):
        """The Context and parent Plug-in are available via dependency injection"""
        self.log.info("Standard log messages apply here.")
        raise Exception("Exceptions too.")

Showcase

Every possible combination of an action.

class ContextAction(pyblish.api.Action):
    label = "Context action"

    def process(self, context):
        self.log.info("I have access to the context")
        self.log.info("Context.instances: %s" % str(list(context)))


class FailingAction(pyblish.api.Action):
    label = "Failing action"

    def process(self):
        self.log.info("About to fail..")
        raise Exception("I failed")


class LongRunningAction(pyblish.api.Action):
    label = "Long-running action"

    def process(self):
        self.log.info("Sleeping for 2 seconds..")
        time.sleep(2)
        self.log.info("Ah, that's better")


class IconAction(pyblish.api.Action):
    label = "Icon action"
    icon = "crop"

    def process(self):
        self.log.info("I have an icon")


class PluginAction(pyblish.api.Action):
    label = "Plugin action"

    def process(self, plugin):
        self.log.info("I have access to my parent plug-in")
        self.log.info("Which is %s" % plugin.id)


class LaunchExplorerAction(pyblish.api.Action):
    label = "Open in Explorer"
    icon = "folder-open"

    def process(self, context):
        import os
        import subprocess

        cwd = context.data["cwd"]
        self.log.info("Opening %s in Explorer" % cwd)
        result = subprocess.call("start .", cwd=cwd, shell=True)
        self.log.debug(result)


class ProcessedAction(pyblish.api.Action):
    label = "Success action"
    icon = "check"
    on = "processed"

    def process(self):
        self.log.info("I am only available on a successful plug-in")


class FailedAction(pyblish.api.Action):
    label = "Failure action"
    icon = "close"
    on = "failed"


class SucceededAction(pyblish.api.Action):
    label = "Success action"
    icon = "check"
    on = "succeeded"

    def process(self):
        self.log.info("I am only available on a successful plug-in")


class BadEventAction(pyblish.api.Action):
    label = "Bad event action"
    on = "not exist"


class InactiveAction(pyblish.api.Action):
    active = False


class PluginWithActions(pyblish.api.Validator):
    optional = True
    actions = [
        pyblish.api.Category("General"),
        ContextAction,
        FailingAction,
        LongRunningAction,
        IconAction,
        PluginAction,
        pyblish.api.Category("OS"),
        LaunchExplorerAction,
        pyblish.api.Separator,
        FailedAction,
        SucceededAction,
        pyblish.api.Category("Debug"),
        BadEventAction,
        InactiveAction,
    ]

    def process(self):
        self.log.info("Ran PluginWithActions")

Maya example

import time
import pyblish.api
import pyblish_qml


class Collect(pyblish.api.Collector):
    def process(self, context):
        i = context.create_instance("MyInstance")
        i.data["family"] = "default"
        i.append("pCube1")


class SelectInvalidNodes(pyblish.api.Action):
    label = "Select broken nodes"
    on = "failed"
    icon = "hand-o-up"

    def process(self, context):
        self.log.info("Finding bad nodes..")
        nodes = []
        for result in context.data["results"]:
            if result["error"]:
                instance = result["instance"]
                nodes.extend(instance)

        self.log.info("Selecting bad nodes: %s" % ", ".join(nodes))
        cmds.select(deselect=True)
        cmds.select(nodes)


class Validate(pyblish.api.Validator):
    actions = [
        pyblish.api.Category("Scene"),
        SelectInvalidNodes
    ]

    def process(self, instance):
        raise Exception("I failed")


pyblish.api.register_plugin(Collect)
pyblish.api.register_plugin(Validate)

import pyblish_maya
pyblish_maya.show()

@mottosso
Copy link
Member Author

Implemented in 1.2.1.

@BigRoy
Copy link
Member

BigRoy commented Oct 23, 2015

This is so badass! +1
Looking great.

@mottosso
Copy link
Member Author

Thanks, will make a proper introduction to this in the forum in a bit.

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

No branches or pull requests

2 participants