[Bf-committers] proposal for a small UI improvement for driver setup

[Joshua Leung]

Hi,

I agree that the current situation isn't great, and does need to be
improved.

However, IMO the proposed solution is not good either. Specifically, as
presented, it does seem that to imply that that checkbox will always get
shown. There are several issues with this:
1) It is highly unlikely that you will want to disable the thing again once
enabled.
2) That setting applies to all scripts/settings, not just the current
driver. It is therefore misleading to include it as a checkbox there.
3) When loading existing files (with the autorun setting disabled), there's
the duplication that Sergey mentioned (i.e. info header + this panel).
4) IIRC, just putting the checkbox there isn't enough to get everything
running properly for the current session only.

-------

That said, when this autorun setting is disabled (via userpref or
commandline options), there *is* the annoying problem that newly created
drivers cannot be run, with no obvious way to get them working. This is a
usability issue.

As a short-term compromise, I propose that beside/under the error prompt,
we include the "Reload Trusted" operator button (shown in the info header
when loading files when autorun is disabled). We'd have to prompt the user
to save their work (or do it for them), and perhaps the label would need to
be different (something like "Enable Python autorun").

------

In the long-term though, IMO we really should look into building our own
little "mini-Python interpreter" for interpreting driver expressions.
Campbell's work with sandboxing the Py interpreter (e.g. by whitelisting
bytecode opcodes) is a promising if fragile approach. However, it doesn't
mitigate the multithreading problems with Python and the GIL (i.e. there
can only be a single Py interpreter instance running/evaluating code at
one). I know that Bassam and a few other riggers have been having problems
with some of their rigs running slowly (or even crashing) under the new
depsgraph due to multiple pydrivers getting scheduled to run at once, and
everything grinding to a halt as they're evaluated one by one. Thus, by
building our own "driver expression interpreter" that only handles the
subset of Python-like syntax actually needed to evaluate driver
expressions, we can solve both security + performance issues at once.

As a fallback, "full" Python evaluation can still occur if our interpreter
cannot handle a particular expression (i.e. the rigger tried to access a
custom function, or tried to do something "fancy" with indexing/attribute
access). That then would still required autorun to be enabled, and would
still be subject to the GIL restrictions. However, since fewer drivers
should now be affected by the Py bottleneck/limitation, it's still a net
positive for users in general over the current situation.

Regards,
Joshua





On Sun, Jun 4, 2017 at 11:06 PM, Gaia Clary <gaia.clary at machinimatrix.org>
wrote:

> The current solution to this situation is also not a good idea i believe :)
>
> However, isn't there a difference here between ?
>
> - a global definition in user preferences
> - a session related setting
>
> In that sense i believe my proposal is not that bad, as it allows to
> set the autorun option right on spot (where the drivers are defined)
> but only for the current session. And it avoids the display of an error
> and it seemsvery intuitive to me.
>
> Also i believe when there is a bad idea at all here, then it is to force
> the user to enable scripting when all they want is to use drivers.
> Of course i know Campbell has put some effort into this to see how python
> could be made more secure so that using it within drivers would no longer
> be a security problem, and "python in drivers" could be always enabled.
> But as far as  i know there is no satisfying solution for this,
> so drivers still need to have auto run enabled.
>
> Anyways, it was just a proposal, if its not useful, then its ok for me.
> I asked at least :)
>
> cheers,
> Gaia
>
> On 04.06.2017 10:26, Sergey Sharybin wrote:
> > Hi,
> >
> > This isn't a good idea. You should not expose same user setting all over
> > the interface. All those settings should be kept in a centralized place.
> >
> > On Sun, Jun 4, 2017 at 1:04 AM, Aaron Carlisle <carlisle.b3d at gmail.com>
> > wrote:
> >
> >> I think it is a good idea, I also think that it would be fine to have
> this
> >> in 2.79.
> >>
> >> Aaron Carlisle
> >>
> >> Picture taker | Bit cruncher | Pixel pusher | Document writer | Artist
> >> Project administrator for the Blender 3D Documentation Project
> >>
> >> On Sat, Jun 3, 2017 at 1:04 PM, Gaia Clary <
> gaia.clary at machinimatrix.org>
> >> wrote:
> >>
> >>> Hi;
> >>>
> >>> Assume you have disabled "Autorun Python Scripts" by default.
> >>> Now add a new driver and step into the graph editor to edit
> >>> the Driver python expression.
> >>>
> >>> Currently you will see an error text in the panel.
> >>> But what about a change like in the image here:
> >>>
> >>>       http://pasteall.org/pic/show.php?id=116080
> >>>
> >>> If this is an acceptable improvement, is that something that
> >>> could possibly already go into Blender 2.79 or would that be only
> >>> good for Blender 2.8 ?
> >>>
> >>> cheers,
> >>> Gaia
> >>>
> >>> _______________________________________________
> >>> Bf-committers mailing list
> >>> Bf-committers at blender.org
> >>> https://lists.blender.org/mailman/listinfo/bf-committers
> >>>
> >> _______________________________________________
> >> Bf-committers mailing list
> >> Bf-committers at blender.org
> >> https://lists.blender.org/mailman/listinfo/bf-committers
> >>
> >
> >
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> https://lists.blender.org/mailman/listinfo/bf-committers
>