Page MenuHome

Fix T64827 (part two): "Batch-Generate Previews" fails with certain files
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Jul 15 2019, 12:04 PM.

Details

Summary

Some values (not properly handled by do_versions?) could cause
'rna_backup_restore()' to throw an error resulting in the whole preview
generation not being saved to the .blend file.

I've checked folowwing demo files:

  • race_spaceship.blend
  • wasp_bot.blend

Here the offending seetings were:

  • UnitSettings (length_unit, ...)
  • FFmpegSettings (ffmpeg_preset, ...)

For now, these are now excluded in exclude_props

Diff Detail

Repository
rB Blender

Event Timeline

Eventually we should really make this preview script not sensitive to this type of issue at all.

This revision is now accepted and ready to land.Jul 15 2019, 3:29 PM

Eventually we should really make this preview script not sensitive to this type of issue at all.

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.

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.

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.

@Bastien Montagne (mont29): should have waited for you, sorry... see the point here... should I revert? Or will you do with the correct fix?

@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… :/