Enhance ways of accessing active modifier for poll functions of operators #79735

New Issue

Bastien Montagne · 2020-08-12T16:23:57+02:00

Bastien Montagne commented

2020-08-12 16:23:57 +02:00

Currently, poll function of operators can only get the modifier they are supposed to work on from the context. This is often missing (when called from shortcuts e.g., also with drag-n-drop of modifier panels...).

This is a sever problem for liboverrides, as some modifiers should then be edited (the locally added ones), and others not.

I can see two ways to solve that:

Add current 'active' modifier to context (not sure how, or even if, this is always possible).
Work around the issue by adding some kind of second 'polling' helper function to be used from invoke/exec functions (since when not found in context, those use event to get the modifier to work on, but events are not passed to poll functions).

Currently, poll function of operators can only get the modifier they are supposed to work on from the context. This is often missing (when called from shortcuts e.g., also with drag-n-drop of modifier panels...). This is a sever problem for liboverrides, as some modifiers should then be edited (the locally added ones), and others not. I can see two ways to solve that: - Add current 'active' modifier to context (not sure how, or even if, this is always possible). - Work around the issue by adding some kind of second 'polling' helper function to be used from invoke/exec functions (since when not found in context, those use event to get the modifier to work on, but events are not passed to poll functions).

Bastien Montagne commented

2020-08-12 16:23:57 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Bastien Montagne commented

2020-08-12 16:23:57 +02:00

Added subscribers: @mont29, @brecht, @ideasman42

Hans Goudey commented

2020-08-13 21:46:27 +02:00

Added subscriber: @HooglyBoogly

Hans Goudey commented

2020-08-13 21:46:27 +02:00

This is definitely an issue. It seems the operator system might need to be more flexible here. Two ideas:

Maybe unrelated, but it would be really helpful to be able to pass an RNA pointer directly to an operator in these situations.
The poll functions should have access to the properties the operator buttons are defined with.

Maybe these two ideas expand the scope of this task a bit, but together they would solve these sorts of situations.

This is definitely an issue. It seems the operator system might need to be more flexible here. Two ideas: 1. Maybe unrelated, but it would be really helpful to be able to pass an RNA pointer directly to an operator in these situations. 2. The poll functions should have access to the properties the operator buttons are defined with. Maybe these two ideas expand the scope of this task a bit, but together they would solve these sorts of situations.

Campbell Barton commented

2020-08-13 23:39:05 +02:00

Firstly, is there an example of what's currently not possible? (bug report for eg), even though the issue described makes sense, I'd like to have some steps that fail.

Active Panels

This could be done using a lookup function that can be used for active modifier, constraint etc.

Accessing the "modifier" or "constraint" in the properties space could use a shared panel-lookup function that returns the top-level panel the user is interacting with, the modifier can be accessed using this.

Expand Poll Functions (accessing properties as suggested).

The user has a mental model of the application state  `WindowType + Mode + Data` (roughly speaking), if you run an operator it may be supported depending on this *(there are cases where operators pass-through, however this isn't heavily used)*.

I'm wary of tweaking this, mainly because making poll checks too fine-grained could backfire in difficult to predict ways.

Complicate redo.

  Currently redoing an operator executes it as long as poll succeeds, having poll fail as a number is dragged will hide/disable the redo panel.

  Working around this could be done by storing the original properties, however we then risk running the operator with *illegal* values it wont expect... or, we need to define an in-between state where it wont execute because of a failed poll but adjusting settings is allowed.

Make key-bindings behave unpredictably from a user perspective (a key might do different things based on something the user wouldn't expect), while it's the responsibility of developers to use this sensibly, we still end up in unexpected situations with the current system as some operators are written for a spesific context - while users end up using them in a different way and running into problems.

  I'd worry we would get bug reports where a user managed to setup a key binding that did nothing because the properties it used caused the poll function to fail.

When accessed from Python. It would make poll fail in more difficult to detect ways. Since the operators poll could fail based on the arguments (admittedly accessing operators from Python isn't that nice at the moment, especially WRT knowing why the context isn't supported).

Since this can be solved using an active modifier, this is a more conservative choice, although I'd still like to look into the failing case in more detail.

Firstly, is there an example of what's currently not possible? (bug report for eg), even though the issue described makes sense, I'd like to have some steps that fail. - Active Panels ``` This could be done using a lookup function that can be used for active modifier, constraint etc. Accessing the "modifier" or "constraint" in the properties space could use a shared panel-lookup function that returns the top-level panel the user is interacting with, the modifier can be accessed using this. ``` - Expand Poll Functions (accessing properties as suggested). ``` The user has a mental model of the application state `WindowType + Mode + Data` (roughly speaking), if you run an operator it may be supported depending on this *(there are cases where operators pass-through, however this isn't heavily used)*. ``` ``` I'm wary of tweaking this, mainly because making poll checks too fine-grained could backfire in difficult to predict ways. ``` - Complicate redo. ``` Currently redoing an operator executes it as long as poll succeeds, having poll fail as a number is dragged will hide/disable the redo panel. ``` ``` Working around this could be done by storing the original properties, however we then risk running the operator with *illegal* values it wont expect... or, we need to define an in-between state where it wont execute because of a failed poll but adjusting settings is allowed. ``` - Make key-bindings behave unpredictably from a user perspective *(a key might do different things based on something the user wouldn't expect)*, while it's the responsibility of developers to use this sensibly, we still end up in unexpected situations with the current system as some operators are written for a spesific context - while users end up using them in a different way and running into problems. ``` I'd worry we would get bug reports where a user managed to setup a key binding that did nothing because the properties it used caused the poll function to fail. ``` - When accessed from Python. It would make poll fail in more difficult to detect ways. Since the operators poll could fail based on the arguments *(admittedly accessing operators from Python isn't that nice at the moment, especially WRT knowing why the context isn't supported)*. ---- Since this can be solved using an active modifier, this is a more conservative choice, although I'd still like to look into the failing case in more detail.

Campbell Barton changed title from ~~Enhance ways to access to 'active' modifier for poll funtions of operators.~~ to Enhance ways to access to 'active' modifier for poll functions of operators.

2020-08-13 23:45:36 +02:00

Bastien Montagne commented

2020-08-14 10:27:51 +02:00

@ideasman42 ah, thought the report that triggered that task would be more visible from this side... It was #79635 (Library overrides - modifiers).

In a nutshell, on liboverride objects, many modifier operators are only allowed on local (added to the override) modifiers, and not on the ones coming from the linked data. Thus the need to know on which modifier we are working on (we'd need the same for constraints btw, did not check yet whether those were always properly accessible from the poll functions of their operators either).

So always having access to an active modifier would work for that specific case at least.

@ideasman42 ah, thought the report that triggered that task would be more visible from this side... It was #79635 (Library overrides - modifiers). In a nutshell, on liboverride objects, many modifier operators are only allowed on local (added to the override) modifiers, and not on the ones coming from the linked data. Thus the need to know on which modifier we are working on (we'd need the same for constraints btw, did not check yet whether those were always properly accessible from the `poll` functions of their operators either). So always having access to an active modifier would work for that specific case at least.

Hans Goudey commented

2020-08-20 16:39:27 +02:00

In #79735#995740, @ideasman42 wrote:

Active Panels

This could be done using a lookup function that can be used for active modifier, constraint etc.

Accessing the "modifier" or "constraint" in the properties space could use a shared panel-lookup function that returns the top-level panel the user is interacting with, the modifier can be accessed using this.

This is basically already implemented if I understand this idea correctly. UI_region_panel_custom_data_under_cursor is that lookup function. The problem is this is not accessible without the event (not in poll functions) so we need to store the active panel custom data somewhere. The invoke functions should probably be refactored to use that stored version instead of the lookup function then.

> In #79735#995740, @ideasman42 wrote: > - Active Panels > > This could be done using a lookup function that can be used for active modifier, constraint etc. > > Accessing the "modifier" or "constraint" in the properties space could use a shared panel-lookup function that returns the top-level panel the user is interacting with, the modifier can be accessed using this. This is basically already implemented if I understand this idea correctly. `UI_region_panel_custom_data_under_cursor` is that lookup function. The problem is this is not accessible without the event (not in poll functions) so we need to store the active panel custom data somewhere. The invoke functions should probably be refactored to use that stored version instead of the lookup function then.

Hans Goudey commented

2020-11-25 17:01:24 +01:00

@mont29 This image might make you happy:

{F9373310 size=full}

We're using an active modifier to set the node editor context for geometry nodes. Then the modifier shortcuts only affect the active modifier.

@mont29 This image might make you happy: {[F9373310](https://archive.blender.org/developer/F9373310/image.png) size=full} We're using an active modifier to set the node editor context for geometry nodes. Then the modifier shortcuts only affect the active modifier.

Hans Goudey changed title from ~~Enhance ways to access to 'active' modifier for poll functions of operators.~~ to Enhance ways of accessing active modifier for poll functions of operators

2020-11-30 03:45:36 +01:00

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Enhance ways of accessing active modifier for poll functions of operators #79735