Changeset View
Changeset View
Standalone View
Standalone View
tests/python/modules/transform_test.py
- This file was added.
| # ##### BEGIN GPL LICENSE BLOCK ##### | |||||
| # | |||||
| # This program is free software; you can redistribute it and/or | |||||
| # modify it under the terms of the GNU General Public License | |||||
| # as published by the Free Software Foundation; either version 2 | |||||
| # of the License, or (at your option) any later version. | |||||
| # | |||||
| # This program is distributed in the hope that it will be useful, | |||||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | |||||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |||||
| # GNU General Public License for more details. | |||||
| # | |||||
| # You should have received a copy of the GNU General Public License | |||||
| # along with this program; if not, write to the Free Software Foundation, | |||||
| # Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | |||||
| # | |||||
| # ##### END GPL LICENSE BLOCK ##### | |||||
| # <pep8 compliant> | |||||
| import bpy | |||||
| def get_area(): | |||||
sybren: The function tries to find a 3D View. This should be clear from the name, and if that's not… | |||||
| for area in bpy.context.screen.areas: | |||||
| if area.type == 'VIEW_3D': | |||||
| return area | |||||
| raise | |||||
sybrenUnsubmitted Done Inline ActionsNever, ever use a plain raise. Choose the appropriate exception type, and pass it an explanation of what's going wrong. sybren: Never, ever use a plain `raise`. Choose the appropriate exception type, and pass it an… | |||||
sybrenUnsubmitted Done Inline ActionsOf course this comment also applies to all the other plain raise invocations. sybren: Of course this comment also applies to all the other plain `raise` invocations.
| |||||
| def get_region_window(area): | |||||
| for region in area.regions: | |||||
| if region.type == 'WINDOW': | |||||
| return region | |||||
| raise | |||||
| def get_rv3d(area): | |||||
| for space in area.spaces: | |||||
| if space.type == 'VIEW_3D': | |||||
| return space.region_3d | |||||
| raise | |||||
| class TestCompare: | |||||
| __slots__ = (\ | |||||
| "obj_a", | |||||
| "obj_b", | |||||
| "compare_matrix", | |||||
| "compare_data", | |||||
Done Inline ActionsSince this class is contained in a test-specific file, and it's not a test itself, there is not really a need to have "Test" in the name. MeshCompare or ObjectCompare or something along those lines would be better. sybren: Since this class is contained in a test-specific file, and it's not a test itself, there is not… | |||||
| "description",) | |||||
| def __init__(self, obj_a, obj_b, compare_matrix=False, compare_data=False, description=""): | |||||
| self.obj_a = obj_a | |||||
Done Inline ActionsNo need to repeat what's already in the __init__(); these kind of comments tend to rot quickly. I think it's more important to describe what compare_data actually means. sybren: No need to repeat what's already in the `__init__()`; these kind of comments tend to rot… | |||||
| self.obj_b = obj_b | |||||
| self.compare_matrix = compare_matrix | |||||
| self.compare_data = compare_data | |||||
| self.description = description | |||||
| def run(self, index, verbose=True): | |||||
| if verbose: | |||||
| print() | |||||
| print("Test comparison {}: {}".format(index, self.description)) | |||||
| compare_success = True | |||||
| compare_result = "" | |||||
| if self.compare_matrix: | |||||
| mat_diff = self.obj_a.matrix_world - self.obj_b.matrix_world | |||||
| for f in mat_diff: | |||||
| for f in f: | |||||
| if abs(f) > 0.0001: | |||||
| compare_success = False | |||||
Done Inline ActionsDon't use single-letter names. sybren: Don't use single-letter names. | |||||
| compare_result = "Matrix mismatch" | |||||
Done Inline ActionsDon't change the type associated with a name. What's wrong with a loop like for row in mat_diff: for value_diff in row:? sybren: Don't change the type associated with a name.
What's wrong with a loop like `for row in… | |||||
| break | |||||
| else: | |||||
| continue | |||||
| break | |||||
| if compare_success and self.compare_data: | |||||
| mesh_a = self.obj_a.data | |||||
Done Inline ActionsHaving break/else/continue/break on consecutive lines makes it really hard to follow what's going on. Move this code into its own function, then you can replace the first break with a return and all the other keywords can go. sybren: Having `break/else/continue/break` on consecutive lines makes it really hard to follow what's… | |||||
| mesh_b = self.obj_b.data | |||||
| compare_result = mesh_a.unit_test_compare(mesh=mesh_b) | |||||
| compare_success = (compare_result == 'Same') | |||||
| if compare_success: | |||||
Done Inline ActionsThe fact that only mesh objects are supported, and that self.compare_data actually means self.compare_meshes should be documented. sybren: The fact that only mesh objects are supported, and that `self.compare_data` actually means… | |||||
| if verbose: | |||||
Done Inline ActionsThis should also be its own function. sybren: This should also be its own function. | |||||
| print("Success!") | |||||
| return True | |||||
| else: | |||||
| print("Test comparison result: {}".format(compare_result)) | |||||
| print("Resulting object '{}' did not match expected object '{}' from file {}". | |||||
| format(self.obj_a.name, self.obj_b.name, bpy.data.filepath)) | |||||
| return False | |||||
| class TestContext: | |||||
| __slots__ = (\ | |||||
| "cleanup", | |||||
| "compare_queue", | |||||
| "dupli_objects", | |||||
| "exit", | |||||
| "funcs", | |||||
| "index_curr", | |||||
| "override_context", | |||||
| 'on_error', | |||||
| 'on_exit', | |||||
| "verbose",) | |||||
| def __init__(self, funcs): | |||||
| import os | |||||
| context = bpy.context | |||||
| area = get_area() | |||||
| region = get_region_window(area) | |||||
Done Inline ActionsNo need to do a late import here. sybren: No need to do a late import here.
| |||||
| self.cleanup = True | |||||
| self.compare_queue = [] | |||||
| self.dupli_objects = [] | |||||
| self.funcs = funcs | |||||
| self.index_curr = 0 | |||||
| self.override_context = { | |||||
| 'area': area, | |||||
| 'edit_object': context.edit_object, | |||||
| 'mode': context.mode, | |||||
| 'object': context.object, | |||||
| 'region': region, | |||||
| 'scene': context.scene, | |||||
| 'screen': context.screen, | |||||
| 'selected_objects': context.selected_objects, | |||||
| 'view_layer': context.view_layer, | |||||
| 'window': context.window, | |||||
| 'workspace': context.workspace, | |||||
| } | |||||
| self.on_error = None | |||||
| self.on_exit = None | |||||
| self.verbose = os.environ.get("BLENDER_VERBOSE") is not None | |||||
| def _dispatch_test_events_func(self): | |||||
| ret = None # Unregister | |||||
| try: | |||||
| if self.index_curr < len(self.funcs): | |||||
| func_index = self.index_curr | |||||
| self.index_curr += 1 | |||||
| ret = 0.0 #seconds | |||||
| self.default_settings_set() | |||||
| self.funcs[func_index](self) | |||||
| else: | |||||
| # Disable use_event_simulate to check the result. | |||||
| bpy.app.use_event_simulate = False | |||||
| self.default_settings_set() | |||||
Done Inline ActionsIf I read the code correctly, ret can get two values: None and 0.0. These are not exactly meaningful values, and the handling of ret is not None is so far away from ret = 0.0 that it makes it hard to understand what exactly is going on. sybren: If I read the code correctly, `ret` can get two values: `None` and `0.0`. These are not exactly… | |||||
Done Inline ActionsThis method is the callback used in timers (I will move it closer, improve description and change the name to make it clear). When a timers callback returns None, it is unregistered. If the return is a float, the float value represents the minimum time for the test to be run again. The fact that the tests need to run on a timers callback makes it difficult to use with unittest module. Unfortunately this is the only way I have found to be able to use --event-simulate in tests. (This is also done in other test files). mano-wii: This method is the callback used in timers (I will move it closer, improve description and… | |||||
| failed = [] | |||||
| for i, comparison in enumerate(self.compare_queue): | |||||
| sucess = comparison.run(i, self.verbose) | |||||
| if not sucess: | |||||
| failed.append(i) | |||||
| if self.cleanup: | |||||
| self.select_objects((self.dupli_objects[0],)) | |||||
| for obj_dupl in self.dupli_objects[1:]: | |||||
| obj_dupl.select_set(True) | |||||
| self.override_context['selected_objects'].append(obj_dupl) | |||||
| self.compare_queue.clear() | |||||
| bpy.ops.object.delete(self.override_context) | |||||
| if failed: | |||||
| if self.verbose: | |||||
| tot = len(failed) | |||||
| print('Ran', tot, 'tests,' if tot > 1 else 'test,', failed, 'failed') | |||||
| raise Exception("Tests {} failed".format(failed)) | |||||
| except Exception as e: | |||||
| print(e) | |||||
| if self.on_error: | |||||
| self.on_error() | |||||
| if ret is None and self.on_exit: | |||||
| self.on_exit() | |||||
| return ret | |||||
| def mode_cur(self): | |||||
| mode = self.override_context['mode'] | |||||
| if mode == 'EDIT_MESH': | |||||
| return 'EDIT' | |||||
| return mode | |||||
| def default_settings_set(self): | |||||
| rv3d = get_rv3d(self.override_context['area']) | |||||
| if rv3d.view_perspective != 'CAMERA': | |||||
| bpy.ops.view3d.view_camera(self.override_context) | |||||
| scene = self.override_context['scene'] | |||||
| tool_settings = scene.tool_settings | |||||
| scene.transform_orientation_slots[0].type = 'GLOBAL' | |||||
| tool_settings.transform_pivot_point = 'MEDIAN_POINT' | |||||
| tool_settings.snap_elements = {'INCREMENT'} | |||||
| tool_settings.use_snap = False | |||||
Done Inline ActionsWhat is this mapping for? sybren: What is this mapping for? | |||||
Done Inline ActionsBlender should use a consistent enum for "mode", no matter if it is the mode in the context or the mode of the object. I will split things. mano-wii: Blender should use a consistent enum for "mode", no matter if it is the mode in the context or… | |||||
| tool_settings.use_snap_grid_absolute = False | |||||
| tool_settings.use_snap_translate = True | |||||
| tool_settings.use_snap_rotate = False | |||||
| tool_settings.use_snap_scale = False | |||||
| tool_settings.use_proportional_edit_objects = False | |||||
| tool_settings.proportional_edit_falloff = 'SMOOTH' | |||||
| self.mode_set('OBJECT') | |||||
| bpy.ops.object.select_all(self.override_context, action="DESELECT") | |||||
| self.override_context['selected_objects'].clear() | |||||
| def compare_data(self, obj_a, obj_b, description): | |||||
| self.compare_queue.append(TestCompare(obj_a, obj_b, compare_data=True, description=description)) | |||||
| def compare_matrix(self, obj_a, obj_b, description): | |||||
| self.compare_queue.append(TestCompare(obj_a, obj_b, compare_matrix=True, description=description)) | |||||
| def mode_set(self, mode): | |||||
| mode_cur = self.mode_cur() | |||||
| if mode != mode_cur: | |||||
| bpy.ops.object.mode_set(self.override_context, mode=mode) | |||||
| if mode == 'OBJECT': | |||||
| self.override_context['mode'] = 'OBJECT' | |||||
| self.override_context['edit_object'] = None | |||||
| elif mode == 'EDIT': | |||||
| self.override_context['mode'] = 'EDIT_MESH' | |||||
| self.override_context['edit_object'] = self.override_context['object'] | |||||
| elif mode == 'WEIGHT_PAINT': | |||||
| self.override_context['mode'] = 'PAINT_WEIGHT' | |||||
Done Inline ActionsParentheses aren't necessary here. Same for other if-statements. sybren: Parentheses aren't necessary here. Same for other `if`-statements. | |||||
| self.override_context['edit_object'] = None | |||||
| else: | |||||
| raise | |||||
| def select_objects(self, select_objs, active=None): | |||||
| mode_cur = self.mode_cur() | |||||
| self.mode_set('OBJECT') | |||||
| bpy.ops.object.select_all(self.override_context, action="DESELECT") | |||||
| for sel_obj in select_objs: | |||||
| sel_obj.select_set(True) | |||||
| self.override_context['selected_objects'].append(sel_obj) | |||||
| if not active: | |||||
| active = select_objs[-1] | |||||
| self.override_context['view_layer'].objects.active = active | |||||
| self.override_context['object'] = active | |||||
| self.mode_set(mode_cur) | |||||
| def duplicate_object(self, obj): | |||||
| mode_cur = self.mode_cur() | |||||
| self.mode_set('OBJECT') | |||||
| self.select_objects((obj,)) | |||||
| bpy.ops.object.duplicate(self.override_context) | |||||
| obj_dupl = self.override_context['view_layer'].objects.active | |||||
| bpy.ops.object.select_all(self.override_context, action="DESELECT") | |||||
| obj_dupl.select_set(True) | |||||
| self.override_context['object'] = obj_dupl | |||||
| self.override_context['selected_objects'].clear() | |||||
| self.override_context['selected_objects'].append(obj_dupl) | |||||
| self.dupli_objects.append(obj_dupl) | |||||
| self.mode_set(mode_cur) | |||||
| return obj_dupl | |||||
| def duplicate_selected(self): | |||||
| mode_cur = self.mode_cur() | |||||
| orig_sel = self.override_context['selected_objects'][:] | |||||
| bpy.ops.object.duplicate(self.override_context) | |||||
| self.override_context['selected_objects'].clear() | |||||
| for obj in self.override_context['view_layer'].objects: | |||||
| if obj.select_get(): | |||||
| if obj not in orig_sel: | |||||
| self.override_context['selected_objects'].append(obj) | |||||
| self.dupli_objects.append(obj) | |||||
| else: | |||||
| obj.select_set(False) | |||||
| obj_dupl_act = self.override_context['view_layer'].objects.active | |||||
| self.override_context['object'] = obj_dupl_act | |||||
| self.mode_set(mode_cur) | |||||
| return obj_dupl_act | |||||
| def run(self, on_error=None, on_exit=None): | |||||
| self.on_error = on_error | |||||
| self.on_exit = on_exit | |||||
| bpy.app.timers.register(self._dispatch_test_events_func, first_interval=0.0) | |||||
The function tries to find a 3D View. This should be clear from the name, and if that's not possible, from a docstring.