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

Fix positive and negative offsets causing Plug-Ins to run twice #128

Conversation

Etherkirby
Copy link

Closes #127

@BigRoy Suggested to call round on the plugin.order before we subtract 0.5 from it, but after some testing I found this to be a bit unsafe because of rounding errors (round(0.5) --> 0).

Comment on lines +243 to +252
def _reset_iterator(self, start_from=-float("inf"), subtract_offset=True):
# In most cases when passed the int value of an order, we need to subtract
# 0.5, because each order is a range of values, from -0.5 to 0.5.
# E.g. ExtractorOrder is 2, and spans between 1.5-2.5
start_from = start_from - 0.5 if subtract_offset else start_from
self.is_running = True
self.pair_generator = self._iterator(
self.plugins,
self.context,

# Minus 0.5, because each order is a range of values,
# from -0.5 to 0.5. E.g. ExtractorOrder is 2, and spans
# between 1.5-2.5
start_from - 0.5
start_from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's the intended functionality - but this would reset the iterator to the order of the current plug-in. Is that intended? I'd assume that then it would process that plug-in again. No?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weirdly enough, at the point where publish is getting the "current pair", self.current_pair[0] is actually pointing to the next Plug-In to evaluate:

plugin = self.current_pair[0]

Here's a snippet that proves this:

from __future__ import print_function

import os

# Remove artificial delay from GUI
os.environ["PYBLISH_DELAY"] = "0"

import pyblish.api
import pyblish.lib
from pyblish_lite import control

count = {"#": 0}

class MyCollector(pyblish.api.ContextPlugin):
    order = pyblish.api.CollectorOrder

    def process(self, context):
        count["#"] += 1

class MyCollectorPosOffset(pyblish.api.ContextPlugin):
    order = pyblish.api.CollectorOrder + 0.4

    def process(self, context):
        count["#"] += 1

class MyValidatorNegOffset(pyblish.api.ContextPlugin):
    order = pyblish.api.ValidatorOrder - 0.4

    def process(self, context):
        count["#"] += 10

class MyValidator(pyblish.api.ContextPlugin):
    order = pyblish.api.ValidatorOrder

    def process(self, context):
        count["#"] += 10

pyblish.api.deregister_all_plugins()

test_plugins = (
    MyCollector, MyCollectorPosOffset, MyValidatorNegOffset, 
    MyValidator
)
for plugin in test_plugins:
    pyblish.api.register_plugin(plugin)

ctrl = control.Controller()
print("Current pair before collection:", ctrl.current_pair)
ctrl.reset()
print("Current pair after collection:", ctrl.current_pair)
assert count["#"] == 2, count

ctrl.publish()
print("Current pair after publish:", ctrl.current_pair)
# Disabling cause this fails in the current version and is getting fixed here.
# assert count["#"] == 22, count

-->

Current pair before collection: (None, None)
Current pair after collection: (<class 'pyblish.plugin.MyValidatorNegOffset'>, None)
Current pair after publish: (None, None)

TL;DR: It's actually using the order of the next Plug-In which wasn't processed yet, so no Plug-Ins are processed twice.

@mottosso
Copy link
Member

Closing based on discussion in #127

@mottosso mottosso closed this Sep 24, 2022
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

Successfully merging this pull request may close these issues.

Using positive and negative offsets leading to some Plug-Ins being processed multiple times
3 participants