Porting to blender 4.x #289

Spiderguy-F · 2024-05-22T10:38:42Z

Merged with the windows fix in master, all seems to work in both versions now.

keianhzo

LGTM, just a couple of nits. Thanks for this!

keianhzo · 2024-05-24T10:16:00Z

addons/io_hubs_addon/io/gltf_exporter.py

@@ -37,7 +42,7 @@ def export_callback(callback_method, export_settings):
    # mid iteration and so multiple callbacks could be executed for the same
    # component/host.

-    for scene in bpy.data.scenes[:]:
+    for scene in bpy.data.scenes[:]:  # I don't think we need to copy the dicts [:], since we can't even alter the list, this would save mem


The explanation for this list copy is in the preview comment block. If that's not true anymore, we should remove that comment block, otherwise we should remove this one. @Exairnous

As far as I know, the preview comment block is still valid. Also, given the recent issue I found with data corruption when components were removed from the original list (for migrations, they do something similar to what is done here) but still present in the copy, we should maybe think about putting in some extra guards at some point.

Blender scenes, objects, data can only be modified with bpy.ops.
bpy.data.objects for example by delete or add operators. I don't think you want objects that were previously deleted in the list and I think adding objects should also add to that list. However the copy is discarded as soon as the export_callback is complete, so I don't think it matters at this point.
"when components were removed from the original list but still present in the copy," is there a reason why this should be possible at all?

I think we discussed this at a dev meetup, but I'm putting this here for the record. My experience has been that Blender data can be modified outside of bpy.ops and that anything added or removed during this loop you probably don't want to operate on within it (you want to stay with the original items unless they've been removed). As for removing, it's probably unlikely here, but the callbacks can pretty much do anything they want, so it's not out of the question, e.g. perhaps something gets added pre-export and removed post-export.

keianhzo · 2024-05-24T10:16:59Z

addons/io_hubs_addon/io/gltf_exporter.py

@@ -28,6 +28,11 @@ def patched_gather_gltf(exporter, export_settings):
 def get_version_string():
    from .. import (bl_info)
    return str(bl_info['version'][0]) + '.' + str(bl_info['version'][1]) + '.' + str(bl_info['version'][2])
+    #  or (shorter):


If there is a faster version of this I'm ok with replacing it. If you have a strong opinion about this go ahead and replace and remove the comments.

I just don't like multiple lookups and long lines, if I can prevent them, that's all.
I read that Blender addons should not use f"" for some reason. The join method should be fastest, I don't think it matters, bc. this is not called 100 times per second, but it's also shorter.

Interesting. According to this it's because fstrings don't work properly with translations; however, I was unable to verify if that's still an issue or not. We may want to take a closer look at strings displayed to Blender's UI, but I think for this case it's fine, and the most readable option. So I'd go with an fstring here, although I 'd probably go with using version as the variable name instead of info.
https://devtalk.blender.org/t/python-string-formatting/14040

keianhzo · 2024-05-24T13:30:47Z

I've found some signature issues that I've fixed here: #289

keianhzo

LGTM

Exairnous

Aside from the comments I left inline, I found that this is having trouble with video texture targets and lightmaps in 4.0 (and possibly more components, not sure); I don't think they get exported and I believe these are related errors in the terminal:

gather_scene_hook fails on <io_hubs_addon.io.gltf_exporter.glTF2ExportUserExtension object at 0x7fc9055b7be0>
ExportImage.blender_image() takes 1 positional argument but 2 were given

gather_material_hook fails on <io_hubs_addon.io.gltf_exporter.glTF2ExportUserExtension object at 0x7fc9055b7be0>
ExportImage.blender_image() takes 1 positional argument but 2 were given

gather_node_hook fails on <io_hubs_addon.io.gltf_exporter.glTF2ExportUserExtension object at 0x7fc9055b7be0>
ExportImage.blender_image() takes 1 positional argument but 2 were given

There were more, but I think adding in the changes from #292 fixed some of them.

Exairnous · 2024-06-05T11:54:55Z

addons/io_hubs_addon/io/gltf_exporter.py

@@ -28,6 +28,11 @@ def patched_gather_gltf(exporter, export_settings):
 def get_version_string():
    from .. import (bl_info)
    return str(bl_info['version'][0]) + '.' + str(bl_info['version'][1]) + '.' + str(bl_info['version'][2])
+    #  or (shorter):


Interesting. According to this it's because fstrings don't work properly with translations; however, I was unable to verify if that's still an issue or not. We may want to take a closer look at strings displayed to Blender's UI, but I think for this case it's fine, and the most readable option. So I'd go with an fstring here, although I 'd probably go with using version as the variable name instead of info.
https://devtalk.blender.org/t/python-string-formatting/14040

Exairnous · 2024-06-05T12:07:52Z

addons/io_hubs_addon/io/gltf_exporter.py

@@ -37,7 +42,7 @@ def export_callback(callback_method, export_settings):
    # mid iteration and so multiple callbacks could be executed for the same
    # component/host.

-    for scene in bpy.data.scenes[:]:
+    for scene in bpy.data.scenes[:]:  # I don't think we need to copy the dicts [:], since we can't even alter the list, this would save mem


I think we discussed this at a dev meetup, but I'm putting this here for the record. My experience has been that Blender data can be modified outside of bpy.ops and that anything added or removed during this loop you probably don't want to operate on within it (you want to stay with the original items unless they've been removed). As for removing, it's probably unlikely here, but the callbacks can pretty much do anything they want, so it's not out of the question, e.g. perhaps something gets added pre-export and removed post-export.

Exairnous · 2024-06-05T12:33:52Z

addons/io_hubs_addon/nodes/lightmap.py

+
+    def draw(self, context):
+        layout = self.layout
+        layout.operator("node.add_node", text="MOZ_lightmap settings").type = "moz_lightmap.node"


I think the node.add_node operator should use the use_transform option set to True so that it maintains the same behavior as in Blender 3.6

keianhzo · 2024-06-05T13:07:17Z

Aside from the comments I left inline, I found that this is having trouble with video texture targets and lightmaps in 4.0 (and possibly more components, not sure); I don't think they get exported and I believe these are related errors in the terminal:
gather_scene_hook fails on <io_hubs_addon.io.gltf_exporter.glTF2ExportUserExtension object at 0x7fc9055b7be0>
ExportImage.blender_image() takes 1 positional argument but 2 were given
gather_material_hook fails on <io_hubs_addon.io.gltf_exporter.glTF2ExportUserExtension object at 0x7fc9055b7be0>
ExportImage.blender_image() takes 1 positional argument but 2 were given
gather_node_hook fails on <io_hubs_addon.io.gltf_exporter.glTF2ExportUserExtension object at 0x7fc9055b7be0>
ExportImage.blender_image() takes 1 positional argument but 2 were given
There were more, but I think adding in the changes from #292 fixed some of them.

I believe my PR fixes this.

Exairnous · 2024-06-06T07:19:35Z

I believe my PR fixes this.

This was encountered with the changes in your PR applied while attempting to export this file:
Export_dev_test_2_packed.zip

Console Log

``` 06:47:35 | INFO: Starting glTF 2.0 export 06:47:35 | INFO: Extracting primitive: Skybox.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Directional Light.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Ambient Light.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Point Light.002 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Spot Light.002 06:47:35 | INFO: Primitives created: 1 gather_node_hook fails on ExportImage.blender_image() takes 1 positional argument but 2 were given 06:47:35 | INFO: Extracting primitive: Plane.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Cube 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Visible 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Nav Mesh 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Video Texture Source 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Ammo Shape 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Particle Emitter 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Waypoint 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Link 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Image 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Audio 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Video 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Billboard 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Text 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Media Frame 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Spawner 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Audio Target 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Audio Source 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Audio Zone 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Shadow 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Uv Scroll 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Loop Animation 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Personal Space Invader 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Video Texture Target 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Frustrum 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Simple Water 06:47:35 | INFO: Primitives created: 1 WARN (bpy.rna): source/blender/python/intern/bpy_rna.cc:1343 pyrna_enum_to_py: current value '524317' matches no enum in 'FloatProperty', 'tideSpeed', 'subtype' WARN (bpy.rna): source/blender/python/intern/bpy_rna.cc:1343 pyrna_enum_to_py: current value '524317' matches no enum in 'FloatProperty', 'tideSpeed', 'subtype' WARN (bpy.rna): source/blender/python/intern/bpy_rna.cc:1343 pyrna_enum_to_py: current value '524317' matches no enum in 'FloatProperty', 'waveSpeed', 'subtype' WARN (bpy.rna): source/blender/python/intern/bpy_rna.cc:1343 pyrna_enum_to_py: current value '524317' matches no enum in 'FloatProperty', 'waveSpeed', 'subtype' 06:47:35 | INFO: Extracting primitive: Hemisphere Light 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Audio Params 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Model 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Suzanne.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Plane 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Arrow 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Suzanne 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Image Spherical 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Video Spherical 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Uv Scroll.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Ground gather_material_hook fails on ExportImage.blender_image() takes 1 positional argument but 2 were given 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.003 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.004 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.005 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.006 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.007 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.008 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.009 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.010 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.011 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.012 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.013 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.014 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.015 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.016 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.017 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.018 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.019 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.020 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.023 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.025 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.026 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.027 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.028 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.029 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.030 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.031 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.032 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.033 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Shadow.001 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.034 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.035 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.036 06:47:35 | INFO: Primitives created: 1 06:47:35 | INFO: Extracting primitive: Label.002 06:47:35 | INFO: Primitives created: 1 gather_scene_hook fails on ExportImage.blender_image() takes 1 positional argument but 2 were given 06:47:35 | INFO: Finished glTF 2.0 export in 0.3055851459503174 s ```

Aside from these errors, I just found that there is one spot in the add-on that uses a context override for an operator and Blender 4.0 removed support for context overrides to operators. You now have to use a temp_override context manager. See: https://projects.blender.org/blender/blender/commit/ac263a9bce53e190d07d679a058a230e91e722be and https://docs.blender.org/api/4.0/bpy.types.Context.html#bpy.types.Context.temp_override

Temp overrides were introduced in Blender 3.2, so I think it's safe to just use them. The spot that needs changing begins at line 325 in components/operators.py

keianhzo · 2024-06-06T11:24:34Z

addons/io_hubs_addon/io/utils.py

@@ -196,7 +196,7 @@ def gather_node_property(export_settings, blender_object, target, property_name)
    blender_object = getattr(target, property_name)

    if blender_object:
-        if bpy.app.version < (3, 2, 0):
+        if bpy.app.version < (3, 2, 0) or bpy.app.version >= (4, 0, 0):


I think >= 4.0 uses the other signature so this change is not necessary

If I remember correctly this was the same for 3.2 and 4.x, but different in 3.5 and 3.6 and failed without that if

keianhzo · 2024-06-06T11:26:57Z

I've updated my related PR to fix some lightmap node issues in 4.0.

Regarding:

Aside from these errors, I just found that there is one spot in the add-on that uses a context override for an operator and Blender 4.0 removed support for context overrides to operators. You now have to use a temp_override context manager. See: https://projects.blender.org/blender/blender/commit/ac263a9bce53e190d07d679a058a230e91e722be and https://docs.blender.org/api/4.0/bpy.types.Context.html#bpy.types.Context.temp_override

Can you to open another PR with that so we can land this and the follow-up PRs?

Spiderguy-F added 6 commits April 10, 2024 17:17

Fixed Exporter For Windows

6f2d9f1

audio zone and videotexture are failing

4da39de

Merge branch 'master' into PortToBlender4

fbdc2b2

re-included try in gltf_expoerter.py

473aa96

Style fixes (comments)

ac72497

Restored original test_export.js

6446f65

keianhzo requested changes May 24, 2024

View reviewed changes

keianhzo mentioned this pull request May 24, 2024

Blender4 image constructor signature updates #292

Closed

Version String Shorter

ea79ae0

keianhzo requested review from Exairnous and keianhzo June 4, 2024 08:06

keianhzo approved these changes Jun 5, 2024

View reviewed changes

Exairnous requested changes Jun 5, 2024

View reviewed changes

keianhzo requested changes Jun 6, 2024

View reviewed changes

implemented image constructor signature

6f09438

Spiderguy-F closed this Jun 7, 2024

Merge branch 'Hubs-Foundation:master' into PortToBlender4

3cad758

Spiderguy-F reopened this Jun 7, 2024

Spiderguy-F closed this Jun 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Porting to blender 4.x #289

Porting to blender 4.x #289

Spiderguy-F commented May 22, 2024

keianhzo left a comment

keianhzo May 24, 2024

Exairnous May 28, 2024 •

edited

Loading

Spiderguy-F May 28, 2024

Exairnous Jun 5, 2024

keianhzo May 24, 2024

Spiderguy-F May 28, 2024 •

edited

Loading

Exairnous Jun 5, 2024

keianhzo commented May 24, 2024

keianhzo left a comment

Exairnous left a comment

Exairnous Jun 5, 2024

Exairnous Jun 5, 2024

Exairnous Jun 5, 2024

keianhzo commented Jun 5, 2024

Exairnous commented Jun 6, 2024

keianhzo Jun 6, 2024

Spiderguy-F Jun 7, 2024

keianhzo commented Jun 6, 2024 •

edited

Loading

Porting to blender 4.x #289

Porting to blender 4.x #289

Conversation

Spiderguy-F commented May 22, 2024

keianhzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Exairnous May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spiderguy-F May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo commented May 24, 2024

keianhzo left a comment

Choose a reason for hiding this comment

Exairnous left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo commented Jun 5, 2024

Exairnous commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo commented Jun 6, 2024 • edited Loading

Exairnous May 28, 2024 •

edited

Loading

Spiderguy-F May 28, 2024 •

edited

Loading

keianhzo commented Jun 6, 2024 •

edited

Loading