From 453ec0d8340913453c21d1681988df51305b10af Mon Sep 17 00:00:00 2001 From: Rosie Baish Date: Wed, 27 Dec 2023 16:50:24 +0000 Subject: [PATCH 1/2] Add option to extend a list to an entity list in addition to just appending When combined with the fact that extending Blueprint.entities using += notation causes a deep copy, there wasn't a clean way to add a list of entities to a blueprint. This adds one, which testing shows is more performant than the += method, and produces identical results to simply appending in sequence. --- draftsman/classes/entitylist.py | 29 ++++++++++++++++++++++++++++- test/test_entitylist.py | 12 ++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/draftsman/classes/entitylist.py b/draftsman/classes/entitylist.py index b21cc1e..4fb83f7 100644 --- a/draftsman/classes/entitylist.py +++ b/draftsman/classes/entitylist.py @@ -14,7 +14,7 @@ from collections import MutableSequence from copy import deepcopy import six -from typing import Union, Any, TYPE_CHECKING +from typing import Union, Any, List, TYPE_CHECKING import warnings if TYPE_CHECKING: # pragma: no coverage @@ -126,6 +126,33 @@ def append(self, name, copy=True, merge=False, **kwargs): """ self.insert(len(self.data), name, copy=copy, merge=merge, **kwargs) + @utils.reissue_warnings + def extend(self, entities, copy=True, merge=False): + # type: (List[Union[str, EntityLike]], bool, bool, **dict) -> None + """ + Extends the entity list with the list provided. + Functionally the same as appending one element at a time + + :param copy: Passed through to append for each element + :param merge: Passed through to append for each element + + :example: + + .. code-block :: python + + blueprint = Blueprint() + assert isinstance(blueprint.entities, EntityList) + + # Append Entity instance + blueprint.entities.extend([Container("steel-chest"), Container("wooden-chest", tile_position=(1, 1)]) + assert blueprint.entities[-2].name == "steel-chest" + assert blueprint.entities[-1].name == "wooden-chest" + assert blueprint.entities[-1].tile_position == {"x": 1, "y": 1} + """ + for entity in entities: + self.append(entity, copy=copy, merge=merge) + + @utils.reissue_warnings def insert(self, idx, name, copy=True, merge=False, **kwargs): # type: (int, Union[str, EntityLike], bool, bool, **dict) -> None diff --git a/test/test_entitylist.py b/test/test_entitylist.py index f417dd9..731be3e 100644 --- a/test/test_entitylist.py +++ b/test/test_entitylist.py @@ -69,6 +69,18 @@ def test_insert(self): with self.assertRaises(ValueError): blueprint.entities.append(Container(), copy=False, merge=True) + def test_extend(self): + # Test appending a list vs individually + blueprint1 = Blueprint() + blueprint2 = Blueprint() + entity_list = [new_entity("transport-belt", tile_position=(0, 0)), + new_entity("wooden-chest", tile_position=(1, 1))] + blueprint1.entities.extend(entity_list) + for e in entity_list: + blueprint2.entities.append(e) + + self.assertEqual(blueprint1.to_dict(), blueprint2.to_dict()) + def test_remove(self): pass # TODO From ee8c39d966ba40e2bef56c081eb8e2a82fae9e1c Mon Sep 17 00:00:00 2001 From: Rosie Baish Date: Wed, 27 Dec 2023 17:14:14 +0000 Subject: [PATCH 2/2] Add performance note regarding Blueprint.entities If you do `blueprint.entities += [foo, bar, baz]` then this triggers a deep copy of the resulting list when it assigns it back to blueprint.entities. By changing the code to: ``` for entity in [foo, bar, baz]: blueprint.entities.append(entity) ``` or just `blueprint.entities.extend([foo, bar, baz])` the performance improves by an order of magnitude. (>1min to <8 seconds on the script I'm developing). This is (to me) sufficiently unintuitive that it's worth documenting. --- draftsman/classes/blueprint.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/draftsman/classes/blueprint.py b/draftsman/classes/blueprint.py index d7b6e43..ad570b1 100644 --- a/draftsman/classes/blueprint.py +++ b/draftsman/classes/blueprint.py @@ -447,6 +447,8 @@ class named :py:class:`.EntityList`, which has all the normal properties of a regular list, as well as some extra features. For more information on ``EntityList``, check out this writeup :ref:`here `. + Note - assigning to this triggers a deep copy, so use .append() + or .extend() if you're building incrementally. """ return self._root["entities"]