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

Fix desaturation check with new mix node #368

Closed
TheDuckCow opened this issue Jan 30, 2023 · 6 comments · Fixed by #374
Closed

Fix desaturation check with new mix node #368

TheDuckCow opened this issue Jan 30, 2023 · 6 comments · Fixed by #374
Assignees
Labels
Milestone

Comments

@TheDuckCow
Copy link
Member

TheDuckCow commented Jan 30, 2023

This does not affect prod, only our current dev build.

The recent update to support the new and legacy mix rgb node missed an edge case where we prep/generate materials on a water shader AND "Animate textures" is enabled, it yields this error:

Running set_saturation on water_still
Checking image for grayscale water_still_0001.png
Traceback (most recent call last):
  File "/Users/patrickcrawford/Library/Application Support/Blender/3.4/scripts/addons/MCprep_addon/tracking.py", line 827, in wrapper
    res = function(self, context)
  File "/Users/patrickcrawford/Library/Application Support/Blender/3.4/scripts/addons/MCprep_addon/materials/prep.py", line 555, in execute
    res, err = self.update_material(context, mat)
  File "/Users/patrickcrawford/Library/Application Support/Blender/3.4/scripts/addons/MCprep_addon/materials/prep.py", line 631, in update_material
    sequences.animate_single_material(
  File "/Users/patrickcrawford/Library/Application Support/Blender/3.4/scripts/addons/MCprep_addon/materials/sequences.py", line 122, in animate_single_material
    generate.set_saturation_material(mat)
  File "/Users/patrickcrawford/Library/Application Support/Blender/3.4/scripts/addons/MCprep_addon/materials/generate.py", line 957, in set_saturation_material
    sat_node.inputs[2].default_value = desat_color
TypeError: bpy_struct: item.attr = val: NodeSocketFloat.default_value expected a float type, not list

This seems specific to the generate materials function when animate textures is on. We should define a new test that ensures a material is created and then properly desaturation checks. Oddly enough, even with this error, the end result actually seems to look ok.

@TheDuckCow TheDuckCow added the bug label Jan 30, 2023
@TheDuckCow TheDuckCow added this to the v3.4.2 milestone Jan 30, 2023
@TheDuckCow TheDuckCow self-assigned this Jan 30, 2023
@StandingPadAnimations
Copy link
Collaborator

StandingPadAnimations commented Feb 20, 2023

I can confirm this bug in MCprep Kaion as well:

Python: Traceback (most recent call last):
  File "C:\Users\Mahid\AppData\Roaming\Bforartists\Bforartists\3.5\scripts\addons\MCprep_addon\tracking.py", line 806, in wrapper
    res = function(self, context)
  File "C:\Users\Mahid\AppData\Roaming\Bforartists\Bforartists\3.5\scripts\addons\MCprep_addon\materials\prep.py", line 477, in execute
    generate.set_saturation_material(mat)
  File "C:\Users\Mahid\AppData\Roaming\Bforartists\Bforartists\3.5\scripts\addons\MCprep_addon\materials\generate.py", line 957, in set_saturation_material
    sat_node.inputs[2].default_value = desat_color
TypeError: bpy_struct: item.attr = val: NodeSocketFloat.default_value expected a float type, not list

I also recieved this error, which is related to Kaion removing tracking code (looks like I have my work cut out for me):

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\Mahid\AppData\Roaming\Bforartists\Bforartists\3.5\scripts\addons\MCprep_addon\tracking.py", line 835, in wrapper
    bpy.ops.mcprep.report_error('INVOKE_DEFAULT', error_report=err)
  File "C:\Program Files\Bforartists 3\3.4.1\3.5\scripts\modules\bpy\ops.py", line 111, in __call__
    ret = _op_call(self.idname_py(), C_dict, kw, C_exec, C_undo)
AttributeError: Calling operator "bpy.ops.mcprep.report_error" error, could not be found

However, instead of using a water shader with animate textures, I was swapping a texturepack and prepping with the SEUS preset (default options) on an OBJ that was also prepped with SEUS (but with the useEmission option disabled, that's more of a Kaion thing):
image

@StandingPadAnimations
Copy link
Collaborator

I've isolated it down to this section:

	elif engine == 'CYCLES' or engine == 'BLENDER_EEVEE':
		sat_node = None
		for node in mat.node_tree.nodes:
			if "SATURATE" not in node:
				continue
			sat_node = node
			break

		if not sat_node:
			return  # requires regenerating material to add back
		if len(desat_color) == 3:
			desat_color += [1]  # add in alpha
		sat_node.inputs[2].default_value = desat_color

@StandingPadAnimations
Copy link
Collaborator

Ahh I see, desat_color seems to be some sort of list. It's set to the following early on:

desat_color = conf.json_data['blocks']['desaturated'][canon]
"blocks" : {
		"desaturated": {
			"acacia_leaves": [
				0.227161,
				0.614651,
				0.089036
			],
			"attached_melon_stem": [
				0.227161,
				0.614651,
				0.089036
			],
			"attached_pumpkin_stem": [
				0.227161,
				0.614651,
				0.089036
			],
			// So on so forth
		}
}

Later on, an extra element is added:

if len(desat_color) == 3:
		desat_color += [1]  # add in alpha

In other words, the edge cases we're seeing is related to this list being used to assign a value.

@StandingPadAnimations
Copy link
Collaborator

So I decided to perform a test. I added the following:

try:
    print("Float")
    print(desat_color)
    sat_node.inputs[2].default_value = desat_color
except Exception:
    print(desat_color)

And this is what I got:
image

It seems like every time we desaturate colors, we're doing so with a list. I think we have the wrong input then

@StandingPadAnimations
Copy link
Collaborator

StandingPadAnimations commented Feb 21, 2023

Ok, the issue is we're using the wrong index

I made a simple script that prints out the inputs with their index and input type, and this is what I got (with one material that only had a Mix node):

0
<bpy_struct, NodeSocketFloatFactor("Factor") at 0x7f63ba65dc08>
1
<bpy_struct, NodeSocketVector("Factor") at 0x7f63abc83008>
2
<bpy_struct, NodeSocketFloat("A") at 0x7f63abcdd408>
3
<bpy_struct, NodeSocketFloat("B") at 0x7f63d8df2608>
4
<bpy_struct, NodeSocketVector("A") at 0x7f63d8df2808>
5
<bpy_struct, NodeSocketVector("B") at 0x7f63d8df2a08>
6
<bpy_struct, NodeSocketColor("A") at 0x7f63d8df2c08>
7
<bpy_struct, NodeSocketColor("B") at 0x7f63d8df2e08>

We're currently trying to set input 2, but input 2 is a NodeSocketFloat

@StandingPadAnimations
Copy link
Collaborator

StandingPadAnimations commented Feb 21, 2023

This is confirmed when I directly access the index.

<bpy_struct, NodeSocketFloat("A") at 0x7f31d2b36e08> 
<bpy_struct, NodeSocketColor("A") at 0x7f31d2b5b608>

The top happens when I directly print node.inputs[2] and the bottom is what happens when I print node.inputs[6] (which I assume is what we actually want)

@StandingPadAnimations StandingPadAnimations linked a pull request Feb 21, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants