-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add Blender 2.80 support (this will resolve #361) #466
base: master
Are you sure you want to change the base?
Conversation
Thank you hound... :) |
I think I've fixed the main issues for now. The Travis build failing is because of Python 3 specific syntax in the Blender part (as I already mentioned in my first comment). |
Could you have a look to see whether you can fence Python 3-specific tests behind a
Good question. I would like us to use them, but it will be a good few years before anyone in the Maya-camp can actually join in, as it would mean stripping support for Python 2 altogether. They would make reading and understanding more challenging for anyone until that time, but I also don't think it's a good idea to hold Blender back due to a slowly moving remainder of the VFX industry. We'll have to pay attention to what is exclusive to Blender, such as utility functions that may or may not be useful elsewhere, but it shouldn't be too hard considering we've got the For completeness, not sure this works from a comment, but to trigger the GitHub automation gods, this PR fixes #361 (which should automatically close that upon merging this PR). Finally, merging this may be tricky, for the same reason #438 is challenging; testing it is hard. Blender + Avalon is still a niche, so this will be likely be your baby, at least initially. I'm happy delegating responsibility for this section of the codebase - ensuring code consistency and that it works - to you, if you'd be happy with that. Finally again, can you post screenshots? :) I think we're all eager to see it in action, Blender user or no. And finally, great work!! Wonder if this would make us eligable for an announcement on one of the many Blender forums? Following the latest convention, I suspect people are in an open frame of mind with regards to switching things around and trying new things. |
Screenshots etc.: yes, I should definitely show them somewhere and probably make some documentation... Where would you like for screenshots/documentation to go? I will get back later to your other points. |
Agreed. Go for it, as long as it's only inside the code areas that are supposed to only have be Python 3+ where it has support for it, e.g. the
Moving footage of it working, like publishing and loading/update/removing content would be amazing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quickly checked the code itself. Looks very clean! 🚀 I put down two comments regarding code style - consider it a start to a discussion at this stage.
Nice work!
We actually recently started using blender for certain tasks, so we'll be able to test it out fairly quickly I think. I'll try to find time today to pull it and give it a spin. Looking forward |
Yes, I can definitely have a look at that. Can you point me in the right direction where I can find the relevant code/tests? I had a look at the test things, but couldn't quite figure out what is exactly run on Travis.
I completely understand it's tricky to merge this. If I could chop it into smaller pieces, I would, but I don't see any way to do that.
In due time, have patience... 😛
Thanks! Before posting it I would prefer let it mature a bit. Also the configuration for the Blender part needs more, so there is actually something to show. |
Cool. And yes, it will only be in
Yes, yes... I will dig up some nice looking assets, possibly from one of the Bender movies so I have more to show then just some cubes and monkeys. |
Cool! Looking forward to hear how it works for you. |
So I have a few comments / questions. I've managed to make it work in pype config (just copying the blender parts). I'm running it on windows and run into some issues. Creator and Workfiles are not loading. I didn't dig into code details before checking that on linux all the key parts are working for you. What I'm currently struggling with is Creator that crashes with this
And workio that can't see registered host that I'm able to trace back to api.install(blender) that throws this
I'm assuming this is not happening to you @jasperges |
Should be fixed with #469 The second one sounds like you are running |
@mkolar Nope, that doesn't happen to me. As @BigRoy mentions, the first error should be fixed if you update Avalon. The second issue also feels to me like Roy mentions. This kind of thing mostly happens to me if the Exception ignored in: <function ImagePreviewCollection.__del__ at 0x000001961F2FFF28>
Traceback (most recent call last):
File "C:\Program Files\Blender Foundation\Blender\2.80\scripts\modules\bpy\utils\previews.py", line 79, in __del__
f"{self!r}: left open, remove with "
ResourceWarning: <ImagePreviewCollection id=0x1961f77fe60[1], <super: <class 'ImagePreviewCollection'>, <ImagePreviewCollection object>>>: left open, remove with 'bpy.utils.previews.remove()' is related to the Avalon Blender integration. It could be because I 'register' the Pyblish icon as a custom icon. If it is, something is definitely wrong and an earlier Today I have some Windows testing planned. So let's see how that will work out. |
@mkolar are you able to give this another go shortly? :) |
@mottosso Can you shed some light on this? ^^ I would like to fix the tests on Travis, but couldn't figure out how it works. |
If possible I would like to merge this integration in the near future. The things I would like to do before a merge:
Do you have any thoughts on this? Do you see any problems or things that need to be fixed or changed? |
I would add |
Good list @jasperges - hit me up if you have a simple test config, etc. and you want me to run through it once too. For the |
Don't! Consider what happens if another vendor did that at the same time, or if Blender themselves at one point installed one for their own internal purposes. There can only be one of those; Avalon should be able to handle exceptions internally, to carry its own weight. If not, then we need to give the user an option to install it, explaining the risks (such as random breakage of anything else that depend on it, including Blender).
I think we solved this quite well in the Qt.py project, see here for example. if PYTHON == 2:
def test_sip_api_already_set():
"""Raise ImportError with sip was set to 1 with no hint, default"""
__import__("PyQt4.QtCore") # Bypass linter warning
import sip If would simply exclude that block of code in anything that isn't Python 2. The more complicated option would be to have separate modules, and run the Python 2 interpreter for one, and Python 3 for another. We would need to modify the CI file for that which can get complicated fast. In general, still strive to write code that run on both Python's so that you won't need such an if-block. It'll still be necessary to support both in parallel for at least another few years. |
Oh my, tests are running in Python 2.6? :O I don't see how, it should be using mayapy (which is 2.7). Unsure about that one. |
Maya and Houdini are also excluded, so it seems safe and the sensible thing to do.
@mottosso The fix was actually very simple. Nose was failing during the discovery of tests, no code was actually tested. Maya and Houdini were already excluded, now I added Blender (here and here)
It seems that either |
Given these considerations I see 2 options:
Personally I would prefer the second option. |
Thanks, will do.
That is exactly what I was doing. 😄 Nice resource by the way. At some point I quickly wanted to test Avalon with Houdini (Apprentice) but didn't have a clue how to set it up. |
@jasperges any updates about blender?
Few questions: |
@iLLiCiTiT A quick answer for now. Will check more thoroughly on Monday.
|
@iLLiCiTiT I double checked and and do not have the problem you describe in 1 and 2. In Blender I do not keep track of the Could it be that you call the tools differently then I do or have a different approach of getting the |
@iLLiCiTiT I'm now hitting the same I don't have time to further investigate the issue right now, but I will get back about this. I really don't know if it is in any way related, but Pyblish uses a 'modal operator'. I will re-work it so it uses the new To be honest I'm stumbling in the dark here, but we'll see... |
@iLLiCiTiT Finally had some time today to dive deeper into the issue. Apart from the import bpy
count = 0
def timed_function():
global count
count += 1
print(f"Crashing in {count}...")
if count > 2:
bpy.ops.wm.open_mainfile(filepath=<blendfilepath>)
return 1 # run every second
bpy.app.timers.register(timed_function, persistent=True) On Windows it might take a second try. But after it has crashed it will crash consistently afterwards unless you log out in back in again (in my tests). Now to find a solution for this... EDIT: this might be related to these: EDIT 2: You can check this out in this branch. |
So, this is a bigger one. :) Adding Blender 2.80+ as host to Avalon.
Some notes up front:
That being said, let's have a discussion how we can work towards adding this to Avalon. Do you see issues, is there missing functionality, did you encounter bugs, etc.?
Looking forward to your comments. :)