theodox / mGui

Python module for cleaner maya GUI layout syntax
MIT License
123 stars 23 forks source link

Version specific controls? #72

Closed bob-white closed 7 years ago

bob-white commented 7 years ago

How do we want to handle version specific codes?

I was looking at adding a wrapper from cmds.workspaceControl and it's related friends, but they only exist in 2017+ so we'd need some way to signify that constraint.

Easiest option would be inside an if MAYA_VERSION >= 2017 style block. Though a separate module could also work.

theodox commented 7 years ago

It's pretty easy to imagine a tweak to the metaclass that would handle this, maybe adding version data to the control and layout objects that triggered the checks.

The more interesting thing is how not to do the checks on every button or whatnot we ever make -- that's paying a big cost for 3/4 of the installs at least.

Maybe we just add the wrapper and let it fail, and address it by adding this as a clear annotation in #4 . Less code, some clariity. It's not a mistake a lot of people will make often,

Whaddya think?

bob-white commented 7 years ago

So for now I've been testing this by just blocking out an if MAYA_VERSION >= '2017': section and defining the class template inside that. In the calling code, I've been waffling back and forth between doing a try / except block and just catching the AttributeError and providing a fallback for older versions, or doing an other version check.

So far it seems to be pretty successful, or at least the dock is showing up in both versions, and not complaining about missing classes.

An interesting followup question that comes from this is: How far back in the Maya timeline do we provide support for? Is there some point where we say, everyone should be on at least VERSION_XXXX and go back through and remove the blocks?

bob-white commented 7 years ago

I'll try to get a patch posted this weekend as an example. Mucking about with the docking stuff actually uncovered some oddities with dockControl that I'll make a different issue for. That command is weird.

theodox commented 7 years ago

Maybe there's value in adding something to the tools module which checks for inappropriate controls? My gut says since we're not shipping actual GUI it's gonna ultimately be the user's problem if they decide to use a 2017 control for their 2014 users. It's be good for them to have accurate error messages so they know they made a mistake -- but I don't know if we can fix it for them....

bob-white commented 7 years ago

The main reason I wrapped it in a if check was because we get an AttributeError just importing the module when it looks up cmds.workspaceControl in the class body and can't find it.

We might be able to tweak the metaclass to do the command lookup by name, and if it can't be found return a class that just raises NotImplementedError. That would potentially defer the error until use-time instead of import or compile time.

theodox commented 7 years ago

I took a look at the PR, but I think we can do it more simply. We just move any later controls into modules (maya2017, maya2018 etc) and import them inside of controls, layouts and menus if the user is on the appropriate version:

if MAYA_VERSION >= 2017:
   from maya2017 import *

If we're on a lower version, the code won't be executed or imported at all (unless somebody explicitly tries to import it from the versioned files) so it won't raise the AttributeError at initialization time. A Gui which depends on stuff which doesn't exist will still except with an AttributeError but that's seems like it's tolerable. I'd expect the debug sequence to be "look in controls.py... oh, it's not there... hey what's this 'if 2017' block... oh" . If we really wanted we could make a parallel 201X file which had all the names but all aliased to a NotInThisMaya class, but I don't see it being worth the work.

The total amount of new controls is fairly small, yes?

bob-white commented 7 years ago

Versioned modules seems much cleaner. Thinking controls_<version>, layouts_<version> etc... as needed.

Yeah, there are I think 2-3 new commands with 2017, and up until this new set of controls, most of the API changes have been isolated to adding new flags.

I was also thinking we should do the from controls_<version> import * at the bottom of the file, this way, if a flag has been added the new version should properly shadow the older version.

bob-white commented 7 years ago

Updated the PR with an if check and a controls_2017.py module. Makes for a much smaller and cleaner change.

theodox commented 7 years ago

much simpler!