Skip to content
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

[F3D] Upgrade improvements (+related) #375

Merged
merged 8 commits into from
Jul 19, 2024

Conversation

Lilaa3
Copy link
Collaborator

@Lilaa3 Lilaa3 commented Jul 8, 2024

Changes:

  1. Upgrades now copy over the current f3d library node tree onto the existing material, this has a lot of benifits such as no longer needing to purge materials, no naming changes, carying over all props, removal of hacks for material pointer propreties a̶n̶d̶ ̶t̶h̶e̶ ̶s̶p̶e̶e̶d̶ ̶o̶f̶ ̶u̶p̶g̶r̶a̶d̶e̶s̶ and on par speed (sometimes faster and sometimes slower?). Orphan data text is removed from upgrade popup.

  2. Removed generateF3DNodeGraph.

  3. Added warning for old materials and an early return for such.
    image

  4. Added "Recreate F3D Shader Nodes" operator, copies over the node tree from the f3d library and updates everything, useful for broken nodes of any kind.

  5. Fixed f3d_light upgrading, along with now popping the props instead of just getting them.

  6. Fixed recursiveCopyOldPropertyGroup which was interperting non set data as a non group, and trying to set new prop to it, it can also now fail and continue (for some reason not an issue before? maybe I should look into that).

  7. Failed material upgrades now print traceback.

  8. Fixed all the errors during draw layer updates by locking the updates and passing in a correct material context, fixed auto props error (Same as 6?? It should be having a lot of errors but it was somehow avoiding them previously?).

@Lilaa3
Copy link
Collaborator Author

Lilaa3 commented Jul 8, 2024

There is one very weird bug where materials upgraded via the f3d material panel (added in pr) will have bad nodes, weird

Changes:
1. Upgrades now copy over the current f3d library node tree onto the existing material, this has a lot of benifits such as no longer needing to purge materials, no naming changes, carying over all props, removal of hacks for material pointer propreties and the speed of upgrades. Orphan data text is removed from upgrade popup.
2. Removed generateF3DNodeGraph
3. Added warning for old materials and an early return for such
4. Added "Recreate F3D Shader Nodes" operator, copies over the node tree from the f3d library and updates everything, useful for broken nodes of any kind
5. Fixed f3d_light upgrading, along with now popping the props instead of just getting them
6. Fixed recursiveCopyOldPropertyGroup which was interperting non set data (for example something set by auto prop) as a non group, and trying to set new prop to it, it can also now fail and continue
7. Failed material upgrades now print traceback
8. Fixed all the errors during draw layer updates by locking the updates and passing in a correct material context, fixed auto props error
@Lilaa3 Lilaa3 force-pushed the use_node_tree_copy_for_upgrades branch from cc32157 to d4defd7 Compare July 9, 2024 11:08
@Lilaa3
Copy link
Collaborator Author

Lilaa3 commented Jul 9, 2024

Last issue is fixed and I ended up removing my dumb active material hack and just used a temporary override

Copy link
Contributor

@sauraen sauraen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, unfortunately this does not work. I've test-merged this into my F3DEX3 preview branch, which is doing upgrades from materials v5 to v6. When I try to do the upgrade on a test scene, I get a ton of errors like:

Execution type: <class 'KeyError'>

Execution value: 'bpy_prop_collection[key]: key "SceneProperties" not found'

Traceback: <traceback object at 0x7fb491811880>
Failed to upgrade VtxColMat
Traceback (most recent call last):
  File "/home/sauraen/.config/blender/3.2/scripts/addons/fast64/fast64_internal/f3d_material_converter.py", line 158, in convertF3DtoNewVersion
    update_all_node_values(material, bpy.context)  # Reload everything
  File "/home/sauraen/.config/blender/3.2/scripts/addons/fast64/fast64_internal/f3d/f3d_material.py", line 1410, in update_all_node_values
    update_node_values_without_preset(material, context)
  File "/home/sauraen/.config/blender/3.2/scripts/addons/fast64/fast64_internal/f3d/f3d_material.py", line 1420, in update_node_values_without_preset
    update_node_values(self, context, update_preset=False)
  File "/home/sauraen/.config/blender/3.2/scripts/addons/fast64/fast64_internal/f3d/f3d_material.py", line 1404, in update_node_values
    update_node_values_of_material(material, context)
  File "/home/sauraen/.config/blender/3.2/scripts/addons/fast64/fast64_internal/f3d/f3d_material.py", line 1791, in update_node_values_of_material
    update_fog_nodes(material, context)
  File "/home/sauraen/.config/blender/3.2/scripts/addons/fast64/fast64_internal/f3d/f3d_material.py", line 1589, in update_fog_nodes
    link_if_none_exist(material, nodes["SceneProperties"].outputs["FogColor"], nodes["FogColor"].inputs[0])
KeyError: 'bpy_prop_collection[key]: key "SceneProperties" not found'

And the resulting materials after the upgrade are broken. Some of them seem to work all right, but some are clearly wrong, including ones not using new EX3 features. Selecting one of these and clicking Recreate F3D Shader Nodes gives a very similar error. In that material's node tree, the error message is correct, there indeed is no SceneProperties node. This node is created in code, so I think you're doing something in the wrong order.

Upgrading v5 to v6 works correctly without the changes in this PR.

@Lilaa3
Copy link
Collaborator Author

Lilaa3 commented Jul 12, 2024

I may need to create scene properties before updating the nodes, sorry

@Lilaa3 Lilaa3 requested a review from sauraen July 12, 2024 08:12
Copy link
Contributor

@sauraen sauraen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works correctly now, thank you!

@sauraen sauraen merged commit 3fe2654 into Fast-64:main Jul 19, 2024
1 check passed
@Lilaa3 Lilaa3 deleted the use_node_tree_copy_for_upgrades branch August 24, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants