Will Group get a .tiles attribute like Blueprint?

[tephyr]

I see that Group handle entities like Blueprint, but there is no associated TileList attached to Group.

Would love to have Group reach feature-parity with Blueprint in regards to entities & tiles, so that containment and nesting work for both types of objects.

Is this on the roadmap? Does it need a volunteer?

[redruin1]

Yes, this was always something that you'd think would be available by default. The reason I haven't implemented it has been because Group has inherited quite a bit of technical debt that I want to (or at least attempt to) remedy before adding this feature.

Blueprints and Groups have an entities list which contains all of their entities and owns the data associated with them, and a entity_map which is a acceleration structure to allow quicker spatial querying of blueprints. This entity_map is primarily used for user searches within given regions, as well as issuing OverlappingEntityWarnings when needed. Because the entity_map was added after the entities list as an acceleration structure, it means that any time you add an entity to the list it has to lookup what parent Group/Blueprint the entity is being added to, and get it's entity_map from there in a roundabout way.

This was kept this way due to legacy reasons, but also the hypothetical ability to swap out the entity_map structure for something else if your use case needed it (sparse quadtree instead of spatial hash, for example), but in terms of practical reality this has basically never actually happened in all the time I've been using the module and no-one else has ever expressed desire for this functionality.

Because of this, it seems like a better design philosophy would be to integrate the acceleration structure into EntityList directly instead of having it be a member of the parent blueprintable; this way I would also be able to remove all of the clunky callbacks like on_entity_insert from these classes and the whole thing would likely be much cleaner and clearer.

If you do decide to prototype this feature, I would either base it off of the 2.0 branch or wait a little bit longer for me to get my ducks in a row. 2.0 has changed a lot from 1.0 already, but ideally shouldn't change too much more until another major version, so using that as a head will probably be more productive.

[redruin1]

I've gotten much closer to implementing this feature on 2.0. However, I'm wondering if simply adding a tiles component to Group is actually the smartest way forward.

group = Group()
group.tiles.append(...)
blueprint = Blueprint()
blueprint.entities.append(group)
# Accessing `tiles` will have to go through `entities`, confusingly
blueprint.entities[0].tiles = ... # ???
# And the root blueprint `tiles` will be unintuitively empty
print(blueprint.tiles) # -> []

The next solution I can think of is to split Group into two classes, similar to EntityCollection and TileCollection:

blueprint.entities.append(EntityGroup(...)) # Only accepts EntityGroup
blueprint.tiles.append(TileGroup(...)) # Only accepts TileGroup

In addition to cluttering up the namespace, this isn't really how you think about groups; most of the time you would want to consider a "Group" a logically connected set of entities and tiles. Splitting them for technical reasons feels inelegant, even if it is closer to how Factorio stores it's information internally.

I'm wondering if a better option would be to remove the distinction between entities and tiles altogether, since most of the time it's not important to the user; only Draftsman really needs to split them into two categories when it resolves them to a blueprint string. Hypothetically, we could create a super list of content that contains entities or tiles:

blueprint = Blueprint()
blueprint.content.append("some-entity", tile_position=(0, 0))
blueprint.content.append("some-tile", tile_position=(0, 0))
print(blueprint.to_dict())
# { "blueprint": {"entities": [{"some-entity", ...}], "tiles": [{"some-tile", ...}]}}

This idea very well could be the best, but it would be a massive API shift. Furthermore, it would add additional complications to behavior (what if an entity and a tile share the same name?) that would ultimately need to be resolved. If I'm going to go further on this project, I want some more viewpoints before I invest too much time into the wrong solution.

[redruin1]

This feature is now present on Draftsman 3.0.0 on the attrs-swap branch. Blueprintable objects now contain a groups attribute:

blueprint = Blueprint()

group = Group()
group.entities.append(...)
group.tiles.append(...)

blueprint.groups.append(group)

This gets around the awkwardness of the previously proposed blueprint.entities[...].tiles[...] syntax, while keeping the clear delineation between the entitites/tiles lists.

The entities and tiles lists are now 1-dimensional, meaning that accessing subchild entities/tiles now behaves more "normally" in how you would expect Python access chains to work:

blueprint.groups["group_a_name"].entities["entity_1"]
blueprint.groups["group_b_name"].groups["subgroup_1"].entities["blah"]
blueprint.groups["group_a_name"].tiles[0]

However, when the type of searched object is fixed to be either entities or tiles (such as add_circuit_connection() method), the shorthand ID tuple still works:

blueprint.add_circuit_connection("red", ("group_a_name", "entity_1"), ("group_b_name", "subgroup_1", "blah"))