Changeset View
Standalone View
tests/python/modules/object_generator.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> | |||||
| # Inspired by mesh_test | |||||
| import bpy | |||||
| import os | |||||
| import random | |||||
| class TestObject: | |||||
| """ | |||||
| To hold specs to create a test object | |||||
| """ | |||||
zazizizou: This class does way more than just holding specs. It creates the test object with an offset too. | |||||
| def __init__(self, collection_name, obj_dict, count=1): | |||||
| """ | |||||
Not Done Inline ActionsWhy not use the a similar interface to OperatorSpec for selection? zazizizou: Why not use the a similar interface to `OperatorSpec` for selection?
| |||||
| :param collection_name: str - name of the collection, also acts a unique test name | |||||
| :param obj_dict: dict - key/value pair for object name and an attribute(call) to it. | |||||
| :param count: int - to make duplicate copies of it. | |||||
| """ | |||||
| self.collection_name = collection_name | |||||
| self.obj_dict = obj_dict | |||||
| self.count = count | |||||
| def create_obj(self): | |||||
| """ | |||||
| Calls for _create_object function for count of objects. | |||||
| """ | |||||
| for index_of_object in range(self.count): | |||||
| self._create_object(index_of_object) | |||||
| def _create_object(self, index): | |||||
| """ | |||||
| Create a test object and expected object. | |||||
| :param index: to name the objects test/exp objects accordingly, e.g. testObj1, testObj2 etc | |||||
| """ | |||||
| offset_y = self._get_last_location() + 5 | |||||
| list_of_added_objects = [] | |||||
Not Done Inline ActionsWhy? you can always extract the directory from the full path and therefore simplify the interface zazizizou: Why? you can always extract the directory from the full path and therefore simplify the… | |||||
| for obj_name, obj_type in self.obj_dict.items(): | |||||
| test_obj_name = "testObj" + obj_name + str(index) | |||||
| if test_obj_name not in bpy.data.objects.keys(): | |||||
| try: | |||||
| getattr(bpy.ops.mesh, obj_type) | |||||
| mesh_obj = getattr(bpy.ops.mesh, obj_type) | |||||
| except AttributeError: | |||||
| raise AttributeError("Incorrect parameter: {} for adding object.".format(obj_type)) | |||||
| mesh_obj(location=(0, offset_y, 0)) | |||||
| bpy.context.active_object.name = test_obj_name | |||||
| bpy.ops.object.duplicate_move(TRANSFORM_OT_translate={"value": (5, 0, 0)}) | |||||
| exp_obj_name = "expObj" + obj_name + str(index) | |||||
| bpy.context.active_object.name = exp_obj_name | |||||
| self._link_to_collection(test_obj_name, exp_obj_name, list_of_added_objects, index) | |||||
| offset_y += 5 | |||||
| else: | |||||
| print("Object already present.") | |||||
| self._output_log_and_unlink(list_of_added_objects) | |||||
zazizizouUnsubmitted Done Inline Actionslist_of_added_objects is a parameter that GenObject should take and and not TestObject itself, because as mentioned above, GenObject should be responsible for coordinating TestObject class instances zazizizou: `list_of_added_objects` is a parameter that `GenObject` should take and and not `TestObject`… | |||||
| def _link_to_collection(self, test_obj_name, exp_obj_name, object_list, index): | |||||
| """ | |||||
| Add test and expected objects to a Collection | |||||
| """ | |||||
| test_obj = bpy.data.objects[test_obj_name] | |||||
Not Done Inline ActionsPrefer more specific exceptions, e.g. NotADirectoryError or FileNotFoundError zazizizou: Prefer more specific exceptions, e.g. `NotADirectoryError` or `FileNotFoundError` | |||||
| object_list.append(test_obj) | |||||
| exp_obj = bpy.data.objects[exp_obj_name] | |||||
| object_list.append(exp_obj) | |||||
| collection = bpy.data.collections.new(name=self.collection_name) | |||||
| collection.name = self.collection_name + str(index) | |||||
| bpy.context.scene.collection.children.link(collection) | |||||
| collection.objects.link(test_obj) | |||||
| collection.objects.link(exp_obj) | |||||
| def _get_last_location(self): | |||||
| """ | |||||
| To find the location of last created object. | |||||
| """ | |||||
| farthest = 0 | |||||
| all_y_locations = [] | |||||
| for obj in bpy.data.objects: | |||||
| all_y_locations.append(obj.location.y) | |||||
| for y in all_y_locations: | |||||
| if y > farthest: | |||||
| farthest = y | |||||
| return farthest | |||||
Not Done Inline ActionsThis is not helpful. It's better to re-raise the original exception. zazizizou: This is not helpful. It's better to re-raise the original exception. | |||||
| def _output_log_and_unlink(self, object_list): | |||||
zazizizouUnsubmitted Not Done Inline Actionsthis looks like a cleanup function that should be called in the destructor of GenObject zazizizou: this looks like a cleanup function that should be called in the destructor of `GenObject` | |||||
calraAuthorUnsubmitted Done Inline ActionsNot very clear how a destructor would be called in here. calra: Not very clear how a destructor would be called in here. | |||||
| """ | |||||
| :param object_list: To keep track objects already added to Collection | |||||
Not Done Inline ActionsCan you explain the responsibility of the class GenerateTestObject? Does it not necessarily generate test objects? The original idea was to define a class that specifies how an object should be created, e.g. TestObject and another one that actually creates the object, i.e. GenerateTestObject, so any property of the test object (selection, vertex group, etc..) should be stored in TestObject zazizizou: Can you explain the responsibility of the class `GenerateTestObject`? Does it not necessarily… | |||||
Done Inline ActionsYes but it also generates additional supporting for test objects. Hmm, I see. Giving Vertex Group, a separate class would allow for less dependence. Adding vertex groups to already existing objects. Also I can't recall what exactly is meant by selection here? (Is it related to selecting vertices in a vertex group?) Also having so many optional parameters would complicate the interface, no? calra: Yes but it also generates additional supporting for test objects.
Hmm, I see. Giving Vertex… | |||||
Not Done Inline ActionsSelection is useful for operators because they operate on selection and not on the whole mesh. For the interface making most of the parameters optional should solve it. See how bpy.ops.wm.save_as_mainfile has so many parameters and it's still easy to use because you usually only set those that you need. zazizizou: Selection is useful for operators because they operate on selection and not on the whole mesh. | |||||
| """ | |||||
| scene_name = bpy.context.scene.name | |||||
| # If objects are present in the Master Collection | |||||
| master_collection_list = list(bpy.data.scenes[scene_name].collection.objects) | |||||
| if len(master_collection_list) != 0: | |||||
| for obj in object_list: | |||||
| bpy.data.scenes[scene_name].collection.objects.unlink(obj) | |||||
Not Done Inline Actionsoutdated doc zazizizou: outdated doc | |||||
| object_list.remove(obj) | |||||
campbellbartonUnsubmitted Not Done Inline ActionsAssigning scene_name here seems redundant, assigning scene = bpy.context.scene is enough, no need for lookups to bpy.data.scenes after that. campbellbarton: Assigning `scene_name` here seems redundant, assigning `scene = bpy.context.scene` is enough… | |||||
| # If objects gets added to the default collection - 'Collection' | |||||
| for obj in object_list: | |||||
| print("{} was successfully created.".format(obj.name)) | |||||
zazizizouUnsubmitted Done Inline ActionsThis looks like the wrong place for such a message. It should be shown where the object was actually created and not when it's about to get removed from the default collection zazizizou: This looks like the wrong place for such a message. It should be shown where the object was… | |||||
| try: | |||||
| if self.collection_name != "Collection": | |||||
| bpy.data.collections["Collection"].objects.unlink(obj) | |||||
| object_list.remove(obj) | |||||
| except: | |||||
| pass | |||||
zazizizouUnsubmitted Done Inline ActionsI think you should take care of correct linking where you create the object. It really seems weird that a TestObject instance cleans up or unlinks other object it might not know about. zazizizou: I think you should take care of correct linking where you create the object. It really seems… | |||||
| class GenObject: | |||||
zazizizouUnsubmitted Done Inline ActionsGenObject -> GenerateTestObjects zazizizou: `GenObject` -> `GenerateTestObjects` | |||||
| """ | |||||
| Holds specs for generating a TestObject | |||||
zazizizouUnsubmitted Done Inline ActionsThis class also does more than that, but it's fine. All test and file creation should happen in this class zazizizou: This class also does more than that, but it's fine. All test and file creation should happen in… | |||||
| """ | |||||
| def __init__(self, path, filename, test_object_list): | |||||
| """ | |||||
| :param path: str - path/to/the/file excluding the filename | |||||
| :param filename: str - name of the blend file | |||||
| :param test_object_list: list - list of objects we want to create TestObject. | |||||
| """ | |||||
| self.path = path | |||||
| self.filename = filename | |||||
| self.path_to_file = self.path + str("\\") + self.filename | |||||
campbellbartonUnsubmitted Done Inline ActionsUse os.path.join. campbellbarton: Use `os.path.join`. | |||||
| self.is_new_file = False | |||||
| self._check_for_valid_path() | |||||
| self.test_object_list = test_object_list | |||||
| def _check_for_valid_path(self): | |||||
| """ | |||||
| Checks if the path is valid and if the file exists or not. | |||||
| """ | |||||
| if os.path.exists(self.path): | |||||
| if os.path.exists(self.path_to_file): | |||||
| bpy.ops.wm.open_mainfile(filepath=self.path_to_file) | |||||
| else: | |||||
| self.is_new_file = True | |||||
| bpy.ops.wm.save_as_mainfile(filepath=self.path_to_file) | |||||
| print("No existing file found. New file created {}".format(self.filename)) | |||||
| # Cleaning up a new blend file | |||||
| bpy.data.objects['Light'].hide_set(True) | |||||
| bpy.data.objects['Camera'].hide_set(True) | |||||
zazizizouUnsubmitted Done Inline Actions_check_for_valid_path() should really only check if the given path is valid. Hiding objects should be the responsibility of another function. zazizizou: `_check_for_valid_path()` should really //only// check if the given path is valid. Hiding… | |||||
| else: | |||||
| raise Exception("Incorrect path for the file") | |||||
| def create(self): | |||||
| """ | |||||
| Generates objects present in the list. | |||||
| """ | |||||
| try: | |||||
| for create_type in self.test_object_list: | |||||
| if isinstance(create_type, TestObject): | |||||
| create_type.create_obj() | |||||
| bpy.ops.wm.save_mainfile(filepath=self.path_to_file) | |||||
| except: | |||||
| if self.is_new_file: | |||||
| os.remove(self.path_to_file) | |||||
| print("The {} file was deleted!".format(self.filename)) | |||||
| raise Exception("Something went wrong!") | |||||
This class does way more than just holding specs. It creates the test object with an offset too.
I think this is too much. Object creation should be the responsibility of GenObject, because only GenObject knows about all the test objects at the same time and therefore can handle the offset between objects much better.