search

friggog / tree-gen

Procedural generation of tree models in blender
GNU General Public License v3.0
827 stars 74 forks source link

ZeroDivisionError on 4-level trees #23

Closed samipfjo closed 5 years ago

samipfjo commented 5 years ago

This error is due to several of the stock parameter files having zero in their fourth-tier Length parameter. It can be remedied by simply providing a non-zero length parameter.

How should we want to handle this? I suggest a sanity-check function that executes before anything else when the generate button is pressed. Said function would read the parameters, check for conflicts, and throw a helpful error if it finds one. This would have the added benefit of centralizing most of the addon's "you're doing it wrong" messages, and would be easily expandable as we/others find naughty combinations. Thoughts?

To reproduce: 1) Load Apple tree preset 2) Seed 1000 3) Keep leaf generation enabled 4) Set level count to 4 5) Generate

Full stack trace:

Traceback (most recent call last):
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\gui.py", line 196, in _construct
    scene.generate_leaves_input)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 1242, in construct
    t.make()
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 180, in make
    self.create_branches()
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 257, in create_branches
    self.make_stem(turtle, Stem(0, trunk))
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 589, in make_stem
    self.make_branches(turtle, stem, seg_ind, branches_on_seg, prev_rotation_angle)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 913, in make_branches
    self.make_stem(dir_tur, Stem(d_plus_1, new_spline, stem, b_offset, rad), pos_corr_turtle=pos_tur)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 589, in make_stem
    self.make_branches(turtle, stem, seg_ind, branches_on_seg, prev_rotation_angle)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 913, in make_branches
    self.make_stem(dir_tur, Stem(d_plus_1, new_spline, stem, b_offset, rad), pos_corr_turtle=pos_tur)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 589, in make_stem
    self.make_branches(turtle, stem, seg_ind, branches_on_seg, prev_rotation_angle)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 913, in make_branches
    self.make_stem(dir_tur, Stem(d_plus_1, new_spline, stem, b_offset, rad), pos_corr_turtle=pos_tur)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 430, in make_stem
    leaf_count = self.calc_leaf_count(stem)
  File "C:\Users\Luke\AppData\Roaming\Blender Foundation\Blender\2.79\scripts\addons\ch_trees\parametric\gen.py", line 1035, in calc_leaf_count
    result = leaves * (stem.length / (stem.parent.length_child_max * stem.parent.length))
ZeroDivisionError: float division by zero
friggog commented 5 years ago

I think if length is 0 for a level then we should automatically stop generation of that level. We could just put a check within the generation process or we could check the parameters before generation as you suggest and reduce the value of levels to the correct number (i.e. the highest that won't include branches of length 0). Given that the end behaviour would be as expected (i.e. no branches at that level = branches of 0 length at the level), I think this is ok.

I'm not sure whether there are many parameter values that would result in crashes like this, and we'd have to hardcode the checks (whatever they may be) so I'm not sure it's worth implementing some big sanitisation system for all parameters at this stage. I would just put in a check for this specific case and leave it at that for now. Ideally I want to avoid restricting what parameters people can input as much as possible, so rather than getting errors they simply get the model they expect (like I describe above by just skipping the broken level) or stupid looking models causing them to choose better parameters.

samipfjo commented 5 years ago

Fair enough.