SpatialHashMap not updated when assigning Group's entities to an EntityList
elswindle opened this issue · 4 comments
In entities.setter
of Group
, the _entity_hashmap
is not updated when the argument is an EntityList
object. This causes anything that would use it to return data, i.e. find_entities_filtered
, to return nothing. I'm not sure what your intention with this was, but maybe you want to initialize a new EntityList
instead of just assigning _entities
to the new list? Basically just make the first elif
look for both list
and EntityList
. This method works. Alternatively, it can be handled the same way that Blueprint
does by adding each entity into the hashmap.
This is related to #16 and #20. This is fixed in 0.9.6
, for both Group
and Blueprint
. The new setter code is now:
# entities.setter():
self._entity_hashmap.clear()
if value is None:
self._entities.clear()
elif isinstance(value, list):
self._entities = EntityList(self, value)
elif isinstance(value, EntityList):
# Just don't ask
self._entities = copy.deepcopy(value, memo={"new_parent": self})
else:
raise TypeError("'entities' must be an EntityList, list, or None")
self.recalculate_area()
Similarly, EntityLike
, Group
, and Blueprint
now all have working __deepcopy__
methods, so they can all be properly copied as well. EntityList
can be deepcopied, though as shown in the code above there's a bunch of extra baggage that comes with this so in practice it's recommended you simply deepcopy the parent object and it should just work™.
These deepcopy methods follow the following logic:
EntityLikes
can be copied, but Associations to other entities are not preserved (they will still point to the original Entities). I haven't decided whether or not this should throw anInvalidAssociationError
or if all associations should be cleared on copy, but for now they still point to the original entities unchanged. If anEntity
that exists inside someEntityCollection
is copied, the parent of the copiedEntity
will be set toNone
, as it does not get added to theEntityCollection
. That must be done manually:blueprint = Blueprint() blueprint.entities.append("wooden-chest") assert blueprint.entities[0].parent is blueprint # Create the copy entity_copy = copy.deepcopy(blueprint.entities[0]) assert entity_copy.parent is None assert entity_copy not in blueprint.entities # Add the copy to the blueprint explicitly blueprint.entities.append(entity_copy)
Groups
can be copied, and Associations local to that group (fully contained) will be properly copied as well. Attempting to copy aGroup
that has associations that point outside of theGroup
will raiseInvalidAssociationError
on copy. The parent attribute of any contained Entities will be modified to point to the correct, copied value. If theGroup
object being copied has aparent
of it's own, similar toEntityLike
it is set toNone
:blueprint = Blueprint() group = Group("group") # also I changed group so that you don't *need* an associated ID group.entities.append("wooden-chest", id="chest") blueprint.entities.append(group) # Assert that the blueprint contains the group assert blueprint.entities["group"].parent is blueprint # Assert that the group contains the entity assert blueprint.entities[("group", "chest")].parent is blueprint.entities["group"] # Create a copy of that group in blueprint group_copy = copy.deepcopy(blueprint.entities["group"]) # group_copy is identical to group assert group_copy.id == "group" assert len(group.entities) == 1 assert isinstance(group.entities[0], Container) # Copied subentities correctly point to copied parent assert group_copy.entities["chest"].parent is group_copy # But group_copy does not point to the original blueprint assert group_copy.parent is not blueprint assert group_copy.parent is None # Nor is it contained within it assert group_copy not in blueprint.entities # Again, it must be manually added if such a thing is desired
Blueprints
behave nearly identically toGroups
, save for the fact that you don't have to worry about non-local Associations as connections inBlueprints
can only be inside that particular blueprint.
Both of the entities
setters for Blueprint
and Group
create a use deepcopy to create a new set of data, as due to the nature of EntityList
having two or more that point to the same data is undesirable. This makes
blueprint_or_group.entities = other_blueprint_or_group.entities
# conceptually equivalent to
blueprint_or_group.entity_hashmap.clear()
blueprint_or_group.entities = copy.deepcopy(other_blueprint_or_group.entities)
blueprint_or_group.recalculate_area()
As a reminder, EntityList.append
also copies anything passed into it by default unless explicitly set with copy = False
:
group = Group()
test_entity = Container("wooden-chest")
group.entities.append(test_entity, copy=False)
assert test_entity is group.entities[0]
Impressive, I appreciate the work and am excited to try and break it in new and exciting ways.
I developed my own workaround with a class based on Group
that simply replaced all Associations
with one pointing to an entity in the current group. This did require me to give IDs to an entity in the parent blueprint that was either power or circuit connectable prior to copying so I could retrieve the ID from the old entity and create a new association pointing to the same ID within the group. I created the following functions to perform this update which are called at the end of the entities
setter:
@Group.entities.setter
def entities(self, value):
# Copy entities
# ...
updateNeighbours()
updateConnections()
def updateNeighbours(self):
for entity in self.entities:
if(entity.power_connectable):
for idx in range(len(entity.neighbours)):
neighbour = entity.neighbours[idx]
na = Association(self.entities[neighbour().id])
entity.neighbours[idx] = na
def updateConnections(self):
for entity in self.entities:
if(entity.circuit_connectable):
for side in entity.connections:
for color in entity.connections[side]:
for idx in range(len(entity.connections[side][color])):
point = entity.connections[side][color][idx]
connection = point["entity_id"]
na = Association(self.entities[connection().id])
entity.connections[side][color][idx]["entity_id"] = na
Getting this to work required you to add the group the blueprint first, and then set entities for the group to the entities of the blueprint:
bp_old = Blueprint(bp_string)
id = 0
for entity in bp_old.entities:
entity.id = str(id)
id += 1
g = Group('g1')
bp_new = Blueprint()
bp_new.append(g)
bp.entities['g1'].entities = bp_old.entities
This is all a bit hacky, but it worked. Take what you will from this, but just wanted to share this.
I developed my own workaround with a class based on Group that simply replaced all Associations with one pointing to an entity in the current group
This is almost exactly what the current algorithm does, except it uses the id()
of each object (so you don't have to manually set it yourself) using the deepcopy
metamethod. I'm glad that you managed to find a working solution; this patch should be out either later today or tomorrow assuming I can squash some warnings being issued incorrectly.
Alright, all of your issues should be resolved if you update to version 0.9.6
. For the sake of brevity I'm going to close these issues, but if you still have troubles feel free to make a new issue so we have a "clean-slate", so to speak.