Skip to content

Commit 9aa913c

Browse files
committed
Stop destroying user data and silently swallowing failures
Six issues from a review pass, all cases where the add-on either discarded something it did not own or failed without saying so. apply_world cleared the node tree of whatever world the scene happened to have, so Apply Background wiped a hand-built HDRI setup with no warning and no undo. It now owns MV_World, alongside MV_Cameras and MV_Lighting, and leaves every other world alone. Importing a STEP file looked up an existing collection by basename, so two files called housing.step merged into one product, and an unrelated collection named housing collected the geometry instead. Each import now creates its own collection and lets Blender resolve the name. Add Empty Product Collection did nothing at all when the name already existed -- no collection, no message. It now always creates one and reports the name it got. The blocking import path called proc.wait() with no timeout, so only the modal path honoured FREECAD_TIMEOUT and a wedged FreeCAD hung a scripted run indefinitely. Both paths now share the deadline. Rebuilding cameras or lighting removed the objects but orphaned their camera and light datablocks, which accumulated until a save and reload. Unused data is now removed with the object, leaving pinned or shared data untouched. bl_info author was a placeholder, and Start Prefix did not mention that only its first character is read.
1 parent 5977f93 commit 9aa913c

1 file changed

Lines changed: 56 additions & 10 deletions

File tree

‎multiview_product_renderer/__init__.py‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
bl_info = {
22
"name": "MultiView Product Renderer",
3-
"author": "MultiView",
3+
"author": "Danyal Sarfraz",
44
"version": (1, 3, 0),
55
"blender": (4, 0, 0),
66
"location": "View3D > Sidebar > MultiView",
@@ -84,6 +84,7 @@
8484
CAMERA_COLL = "MV_Cameras"
8585
LIGHT_COLL = "MV_Lighting"
8686
PRODUCT_COLL = "MV_Products"
87+
WORLD_NAME = "MV_World"
8788

8889
GUIDE_LINES = [
8990
"1. Scene Setup: pick a lighting preset and background, then Apply.",
@@ -207,7 +208,9 @@ class MV_Settings(PropertyGroup):
207208

208209
start_prefix: StringProperty(
209210
name="Start Prefix",
210-
description="Two-letter prefix for the first product (AA, BB, ...)",
211+
description="Letter the first product group is labelled with. The "
212+
"label repeats it (AA, BB, ...), so only the first "
213+
"character is read -- typing AB gives AA",
211214
default="AA", maxlen=3,
212215
)
213216
start_number: IntProperty(
@@ -321,10 +324,25 @@ def look_at(location, target, up=Vector((0, 0, 1))):
321324

322325

323326
def clear_collection_objects(coll):
327+
"""Remove a collection's objects along with the data they leave behind.
328+
329+
objects.remove() drops the object but not its camera or light datablock, so
330+
every rebuild of the rig orphaned another set and the next one came back as
331+
MV_Cam_Front.001, .002, ... The file kept growing until a save and reload
332+
purged them. Data another object still uses, or that is pinned with a fake
333+
user, is left alone.
334+
"""
324335
if coll is None:
325336
return
326337
for obj in list(coll.objects):
338+
data = obj.data
327339
bpy.data.objects.remove(obj, do_unlink=True)
340+
if data is None or data.users or data.use_fake_user:
341+
continue
342+
if isinstance(data, bpy.types.Camera):
343+
bpy.data.cameras.remove(data)
344+
elif isinstance(data, bpy.types.Light):
345+
bpy.data.lights.remove(data)
328346

329347

330348
def collection_meshes(coll):
@@ -902,9 +920,17 @@ def build_lighting(context, preset_key):
902920

903921

904922
def apply_world(context):
923+
"""Point the scene at the add-on's own world and rewrite that one.
924+
925+
This used to reuse whatever world the scene already had and clear its node
926+
tree, so pressing Apply Background on a file with a hand-built HDRI setup
927+
destroyed it with no warning and no undo. The add-on now owns a single
928+
MV_World, the same way it owns MV_Cameras and MV_Lighting. Any other world
929+
keeps its nodes and stays selectable from World properties.
930+
"""
905931
settings = context.scene.mv_settings
906932
scene = context.scene
907-
world = scene.world or bpy.data.worlds.new("MV_World")
933+
world = bpy.data.worlds.get(WORLD_NAME) or bpy.data.worlds.new(WORLD_NAME)
908934
scene.world = world
909935
world.use_nodes = True
910936
nodes = world.node_tree.nodes
@@ -998,11 +1024,16 @@ def _fail(self, path, reason):
9981024
self._done += 1
9991025

10001026
def _link_new(self, context, path, new_objs):
1027+
# Always a fresh collection. Looking the name up first meant two STEP
1028+
# files sharing a basename -- partA/housing.step and partB/housing.step
1029+
# -- landed in one collection and rendered as a single product, and an
1030+
# unrelated collection that happened to be called "housing" would have
1031+
# the geometry dropped into it. Blender resolves the clash itself by
1032+
# suffixing .001, which keeps the two products separate.
10011033
root = get_products_root()
10021034
base = os.path.splitext(os.path.basename(path))[0]
1003-
product_coll = bpy.data.collections.get(base) or bpy.data.collections.new(base)
1004-
if product_coll.name not in root.children:
1005-
root.children.link(product_coll)
1035+
product_coll = bpy.data.collections.new(base)
1036+
root.children.link(product_coll)
10061037
for obj in new_objs:
10071038
for c in list(obj.users_collection):
10081039
c.objects.unlink(obj)
@@ -1071,7 +1102,19 @@ def _advance(self, context, blocking):
10711102
"""One slice of work. False once everything is finished."""
10721103
if self._job is not None:
10731104
if blocking:
1074-
self._job["proc"].wait()
1105+
# Bounded even here. A bare wait() meant a wedged FreeCAD hung
1106+
# a scripted or headless run forever, with only the modal path
1107+
# honouring the timeout. Same deadline either way, measured
1108+
# from when the process started.
1109+
remaining = FREECAD_TIMEOUT - (time.monotonic() - self._job_started)
1110+
try:
1111+
self._job["proc"].wait(timeout=max(remaining, 0.0))
1112+
except subprocess.TimeoutExpired:
1113+
job, self._job = self._job, None
1114+
freecad_cleanup(job)
1115+
self._fail(self._job_path,
1116+
f"FreeCAD exceeded {int(FREECAD_TIMEOUT)}s and was stopped")
1117+
return bool(self._queue)
10751118
elif freecad_running(self._job):
10761119
if time.monotonic() - self._job_started > FREECAD_TIMEOUT:
10771120
job, self._job = self._job, None
@@ -1289,10 +1332,13 @@ def invoke(self, context, event):
12891332
return context.window_manager.invoke_props_dialog(self)
12901333

12911334
def execute(self, context):
1335+
# Create unconditionally and report the name that came back. Skipping
1336+
# when the name was taken made the button look broken on the second
1337+
# press -- no collection, no message, nothing in the outliner.
12921338
root = get_products_root()
1293-
n = self.name or "Product"
1294-
if bpy.data.collections.get(n) is None:
1295-
root.children.link(bpy.data.collections.new(n))
1339+
coll = bpy.data.collections.new(self.name or "Product")
1340+
root.children.link(coll)
1341+
self.report({'INFO'}, f"Added product collection '{coll.name}'.")
12961342
return {'FINISHED'}
12971343

12981344

0 commit comments

Comments
 (0)