Using new mix node in Blender 3.4

[zNightlord]

This adds create_node() to helps with node generation between version with different socket index

[StandingPadAnimations]

@zNightlord can this be merged into the dev branch? Master is what we use for releases

Edit: ignore this comment for now since this feature relates to compatibility, we'll decide later

Edit 2: all new features go to dev regardless

[StandingPadAnimations]

I'll do further review later this week, but through my quick skim of the code here's what I did notice the use of bpy.app.version. MCprep provides a utility function in util called minBv which essentially does the same thing, just more readable and takes into account version differences in the API

[StandingPadAnimations]

Just requesting a review from @.TheDuckCow for this pull request, though I'll also be reviewing the code and noting where changes have to be made to account for #348

[TheDuckCow]

Marking to request changes, but I recognize I'm not being very explicit yet as I need to take a look myself. The main comment is below, about having consistent return times. I don't love the idea of having to have every single node return both its reference and a sockets function just due to this one change, but the higher-level point for me is that functions have a consistent return structure regardless of what happens inside (ie number of return vars)

Regardless, thanks for jumping so quick on this!

[TheDuckCow]

I need to live test a little, but this feels a bit funny to me. I think it's important that a function generally has the same return structure regardless of how it runs (here, sometimes it returns one value, sometimes two, which makes it unclear what you get back down the line).

I'll probably propose an edit later today unless you beat me to it.

Did the socket names change, or just the order? If the names we need to use are the same

[zNightlord]

For the sockets part, I understand. It's quite odd for me sometimes returns one, two.
The new Mix changes the socket index and the name. If you use the socket name, you will get the result of a float mix instead of RGBA.

[StandingPadAnimations]

This may be the C++/Rust part of my brain acting up, but couldn't we just either:
A. Always return a tuple regardless of the amount of objects, where the first item is the node and the second object depending on the version can be the sockets
B. Use a small wrapper class to handle this situation

Knowing how CPython works though, the C++/Rust part of my brain is screaming "unnecessary copy!" as I write this

[zNightlord]

Maybe we could split the sockets part out to a different function like get_node_socket("ShaderMixNode", "outputs",1)
It will do versioning for the socket of that node type seperate.

[zNightlord]

Oops.

[StandingPadAnimations]

Alright I'll start my review (sorry for the delay, I was installing Arch on my system)

[StandingPadAnimations]

Note that anything I mark as NOTE_FOR_INTERNAL_REFACTORING is something I'll deal with as part of #348

[StandingPadAnimations]

Overall, some good things. There are a couple of sections that may need to be changed for internal refactoring, depending on what we decide for 3.4 version detection

[TheDuckCow]

Sorry for the long delay getting back to you - I'll try to finish my review towards the end of this weekend, and then hopefully we can just merge it and address any follow ups ad hoc so we don't lose track. Thanks again for going through the iterations here!

[zNightlord]

Sorry for the long delay getting back to you - I'll try to finish my review towards the end of this weekend, and then hopefully we can just merge it and address any follow ups ad hoc so we don't lose track. Thanks again for going through the iterations here!

No problem.
I just add in some small adjustment for nodegroup from the node group test

[TheDuckCow]

FYI I'm almost but not quite done with some local updates I'm going to push onto the branch before approving/merging, so please avoid making any changes until I get that in (just didn't quite get it done tonight).

[TheDuckCow]

Hey @zNightlord can you add me as an editor to your fork here? (settings deeplink: https://github.com/zNightlord/MCprep/settings/access). Then I can push the commit I've got locally. On that, I'll approve and we can merge it in!

[TheDuckCow]

Approved.

I was quite hesitant about having the *attrs argument for the node function. But in the end, it does provide upside, and from a code completion standpoint, we don't have static typing really with the node objects being passed around anyways.

All in, thanks for making this change, glad to have it merged in 👍 (oh and local testing went well enough, I'll do some more testing in older blender versions too; but all automated tests through 2.80+ work fine).

[zNightlord]

Alright I don't have write access. So you can merge it now