Differential D5255
Fix T64827 (part two): "Batch-Generate Previews" fails with certain files Authored by Philipp Oeser (lichtwerk) on Jul 15 2019, 12:04 PM. Tags None Subscribers None
Details Some values (not properly handled by do_versions?) could cause I've checked folowwing demo files:
Here the offending seetings were:
For now, these are now excluded in exclude_props
Diff Detail
Event TimelineComment Actions Eventually we should really make this preview script not sensitive to this type of issue at all. Comment Actions I strongly disagree here. This patch is not a fix, it is a hack around the actual issue, it’s merely putting it under the carpet. I cannot see how one could consider OK to have invalid data (as per current RNA code) in our .blend file, and I certainly would not be happy with this script overwriting .blend files silently with such fuzzy things being left unnoticed. The thing to fix here is those invalid enum values, not bl_previews_render.py script, which has absolutely nothing to do with the issue. Comment Actions There can be a test for catching such invalid enum values. But having this batch preview script effectively being that test is not good, most of the time these issues would not even be noticed by users. Comment Actions We cannot leave this 'unnoticed from user', might imply saving some invalid .blend file over some (presumably) valid one. We have to be absolutely sure that our changes in that script did not break users' files! Having a test for that would be much better for sure, but am not sure how to set this up, we'd need a full regression suit over our versioning code, with .blend files from all previous Blender versions… In the mean while, will do proper fix about those enums. Comment Actions @Bastien Montagne (mont29): should have waited for you, sorry... see the point here... should I revert? Or will you do with the correct fix? Comment Actions @Philipp Oeser (lichtwerk) sorry, did not saw your comment earlier… yeah, reverted and done proper fix now. And sorry for not reacting earlier, was afk this afternoon… :/ |