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

Update material and generate functions with MCprepError structure #612

Open
TheDuckCow opened this issue Aug 5, 2024 · 0 comments
Open
Labels
enhancement Feature requests or new functionality suggestions

Comments

@TheDuckCow
Copy link
Member

As a follow on to this PR (#611), let's go the rest of the way to use the MCprepError structure as an Optional[] response for functions where relevant.

Something else that would be nice with this effort: making the lineno and file fields optional, if there's a decent way to grab them automatically when not specifying the function to construct the error (though if I understand how it works properly now, the simple way of doing this would mean the file and lineno would always be the same, so somehow still needs to inherit from the calling location).

Since I started this effort, but stopped in favor of the 3.6.0 release, here's what I did so far (also attaching as a patch file):

wip.patch

diff --git a/MCprep_addon/materials/generate.py b/MCprep_addon/materials/generate.py
index 5dc5437..b8f9347 100644
--- a/MCprep_addon/materials/generate.py
+++ b/MCprep_addon/materials/generate.py
@@ -31,8 +31,8 @@ from ..conf import MCprepError, env, Form
 AnimatedTex = Dict[str, int]
 
 # Error codes used during mat prep
-NO_DIFFUSE_NODE = 1
-IMG_MISSING = 2
+NO_DIFFUSE_NODE_ERR = "Could not find diffuse node"
+IMG_MISSING_ERR = "Image datablock not loaded"
 
 
 class PackFormat(Enum):
@@ -328,7 +328,7 @@ class PrepOptions:
 	use_emission: bool
 
 
-def matprep_cycles(mat: Material, options: PrepOptions) -> Optional[bool]:
+def matprep_cycles(mat: Material, options: PrepOptions) -> Optional[MCprepError]:
 	"""Determine how to prep or generate the cycles materials.
 
 	Args:
@@ -1239,7 +1239,7 @@ def generate_base_material(
 	return mat, None
 
 
-def matgen_cycles_simple(mat: Material, options: PrepOptions) -> Optional[bool]:
+def matgen_cycles_simple(mat: Material, options: PrepOptions) -> Optional[MCprepError]:
 	"""Generate principled cycles material."""
 
 	matGen = util.nameGeneralize(mat.name)
@@ -1249,16 +1249,19 @@ def matgen_cycles_simple(mat: Material, options: PrepOptions) -> Optional[bool]:
 
 	if not image_diff:
 		print(f"Could not find diffuse image, halting generation: {mat.name}")
-		return NO_DIFFUSE_NODE
+		line, file = env.current_line_and_file()
+		return MCprepError(AttributeError(), line, file, NO_DIFFUSE_NODE_ERR)
 	elif image_diff.size[0] == 0 or image_diff.size[1] == 0:
 		if image_diff.source != 'SEQUENCE':
 			# Common non animated case; this means the image is missing and would
 			# have already checked for replacement textures by now, so skip.
-			return IMG_MISSING
+			line, file = env.current_line_and_file()
+			return MCprepError(FileNotFoundError(), line, file, IMG_MISSING_ERR)
 		if not os.path.isfile(bpy.path.abspath(image_diff.filepath)):
 			# can't check size or pixels as it often is not immediately avaialble
 			# so instead, check against firs frame of sequence to verify load
-			return IMG_MISSING
+			line, file = env.current_line_and_file()
+			return MCprepError(FileNotFoundError(), line, file, IMG_MISSING_ERR)
 
 	mat.use_nodes = True
 	animated_data = copy_texture_animation_pass_settings(mat)
@@ -1356,8 +1359,6 @@ def matgen_cycles_simple(mat: Material, options: PrepOptions) -> Optional[bool]:
 	nodeTexDiff["MCPREP_diffuse"] = True
 	nodeSaturateMix["SATURATE"] = True
 
-	return 0
-
 
 def matgen_cycles_principled(mat: Material, options: PrepOptions) -> Optional[bool]:
 	"""Generate principled cycles material"""
@@ -1369,16 +1370,19 @@ def matgen_cycles_principled(mat: Material, options: PrepOptions) -> Optional[bo
 
 	if not image_diff:
 		print(f"Could not find diffuse image, halting generation: {mat.name}")
-		return NO_DIFFUSE_NODE
+		line, file = env.current_line_and_file()
+		return MCprepError(AttributeError(), line, file, NO_DIFFUSE_NODE_ERR)
 	elif image_diff.size[0] == 0 or image_diff.size[1] == 0:
 		if image_diff.source != 'SEQUENCE':
 			# Common non animated case; this means the image is missing and would
 			# have already checked for replacement textures by now, so skip
-			return IMG_MISSING
+			line, file = env.current_line_and_file()
+			return MCprepError(FileNotFoundError(), line, file, IMG_MISSING_ERR)
 		if not os.path.isfile(bpy.path.abspath(image_diff.filepath)):
 			# can't check size or pixels as it often is not immediately avaialble
 			# so instead, check against first frame of sequence to verify load
-			return IMG_MISSING
+			line, file = env.current_line_and_file()
+			return MCprepError(FileNotFoundError(), line, file, IMG_MISSING_ERR)
 
 	mat.use_nodes = True
 	animated_data = copy_texture_animation_pass_settings(mat)
@@ -1524,16 +1528,19 @@ def matgen_cycles_original(mat: Material, options: PrepOptions):
 
 	if not image_diff:
 		print(f"Could not find diffuse image, halting generation: {mat.name}")
-		return NO_DIFFUSE_NODE
+		line, file = env.current_line_and_file()
+		return MCprepError(AttributeError(), line, file, NO_DIFFUSE_NODE_ERR)
 	elif image_diff.size[0] == 0 or image_diff.size[1] == 0:
 		if image_diff.source != 'SEQUENCE':
 			# Common non animated case; this means the image is missing and would
 			# have already checked for replacement textures by now, so skip
-			return IMG_MISSING
+			line, file = env.current_line_and_file()
+			return MCprepError(FileNotFoundError(), line, file, IMG_MISSING_ERR)
 		if not os.path.isfile(bpy.path.abspath(image_diff.filepath)):
 			# can't check size or pixels as it often is not immediately avaialble
 			# so instea, check against firs frame of sequence to verify load
-			return IMG_MISSING
+			line, file = env.current_line_and_file()
+			return MCprepError(FileNotFoundError(), line, file, IMG_MISSING_ERR)
 
 	mat.use_nodes = True
 	animated_data = copy_texture_animation_pass_settings(mat)
@@ -1741,8 +1748,6 @@ def matgen_cycles_original(mat: Material, options: PrepOptions):
 	# reapply animation data if any to generated nodes
 	apply_texture_animation_pass_settings(mat, animated_data)
 
-	return 0
-
 
 def matgen_special_water(mat: Material, passes: Dict[str, Image]) -> Optional[bool]:
 	"""Generate special water material"""
diff --git a/MCprep_addon/materials/prep.py b/MCprep_addon/materials/prep.py
index 1d29993..2aff5ce 100644
--- a/MCprep_addon/materials/prep.py
+++ b/MCprep_addon/materials/prep.py
@@ -210,8 +210,6 @@ class MCPREP_OT_prep_materials(bpy.types.Operator, McprepMaterialProps):
 		count_img_not_found = 0
 		count_misc_no_prep = 0
 
-		print("Orig mat list: ", len(mat_list))
-
 		for mat in mat_list:
 			if not mat:
 				env.log(
@@ -263,9 +261,9 @@ class MCPREP_OT_prep_materials(bpy.types.Operator, McprepMaterialProps):
 					mat=mat,
 					options=options
 				)
-				if res == 0:
+				if res is None:
 					count += 1
-				elif res == generate.IMG_MISSING:
+				elif res.msg == generate.IMG_MISSING_ERR:
 					count_img_not_found += 1
 				else:
 					count_misc_no_prep += 1
@@ -615,6 +613,7 @@ class MCPREP_OT_load_material(bpy.types.Operator, McprepMaterialProps):
 			self.report({"ERROR"}, err)
 			return {'CANCELLED'}
 		elif res is False:
+			print(res, err)
 			self.report({"ERROR"}, "Failed to prep generated material")
 			return {'CANCELLED'}
 
@@ -657,6 +656,9 @@ class MCPREP_OT_load_material(bpy.types.Operator, McprepMaterialProps):
 				mat=mat,
 				options=options
 			)
+			if res is not None:  # MCprepError
+				print(res)
+				return False, res.msg
 		else:
 			return False, "Only Cycles and Eevee supported"
 
@TheDuckCow TheDuckCow added the enhancement Feature requests or new functionality suggestions label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests or new functionality suggestions
Projects
None yet
Development

No branches or pull requests

1 participant