Copying an EntityList from a loaded blueprint throws InvalidAssociationError if any entities have connections

[tephyr]

When loading a blueprint from an external source, or building it in code, and then copying its .entities to a Group, Draftsman throws an InvalidAssociationError if any entities have any connections of either kind (circuit or power). The external blueprint is valid in-game, and this can be reproduced in a blueprint with two entities: two power poles with a single electric connection. Test case available as gist.

Current Behavior

1. Either load an existing blueprint from disk into a Blueprint object, or create one in code.
2. Copy the blueprint's .entities to a new Group (group.entities.extend(bp.entities)).
3. IF there are no connections between any entities, the copy will succeed and the group will be ready for use.
4. If there are connections, an InvalidAssociationError exception will be thrown, with explanatory text like this:

Attempting to connect to <ElectricPole 'medium-02'>{'name': 'medium-electric-pole', 'position': {'x': 3.0, 'y': 0.0}, 'neighbours': [<Association to ElectricPole at 0x00007F62A442FED0>]} which lies outside this EntityCollection; are all Associations between entities contained within this EntityCollection being copied?

Expected behavior
Copying a list of entities from a blueprint should work the same as copying them from a group that was generated in code. All connections should be preserved, and no errors should be thrown.

Additional context
Related to #22.

Draftsman version: 1.1.0
Python version: 3.12.0

I wanted to find out if this can be supported natively, as I use this in a tool that "remixes" blueprints by loading them into Groups and modifying and combining them according to the user's config.

[redruin1]

Thanks for the report, I'll take a look at this over the weekend.

[tephyr]

Once I created a workaround, I discovered the Group.load_from_string() method, which does what I want and appears to not cause the InvalidAssociationError (presumably because the associations are created within the group).

I will throw some more scenarios at it and see if that resolves the issue.

[redruin1]

After taking a look at what you're intending, this seems to be another case of a sore spot between interacting with Blueprintables, similar to issue #48.

Basically, when you extend the entities from bp to group_sub, each entity is individually copied over, one at a time, as you would probably expect. How to handle an entity's Associations (power and circuit connections), however, is not entirely clear, because Draftsman doesn't know if the entities connected to it will also be in the group, and if so which ones they are. Consider the case where you only happened to move over one power pole to the new group; this case is obviously malformed, since there's no possible target for the entity to connect to.

Because Draftsman doesn't have this information when copying one-at-a-time, it defaults to keeping the Associations in the copied entity identical to the original. This means that the entities in group_sub are connected to the entities in bp, rather than the entities in their own group, hence the error.

The reason why group_sub.entities = bp.entities does work is because this statement asserts that all entities in one will all be in the other, meaning that I can figure out which entity corresponds to what and update their associations as you would expect. IIRC deepcopy on any Blueprintable also ensures this, as long as the entire thing is being copied all at once.

A clearer option might be to wipe any entity Associations when copying individually, giving a user a hint that what they're attempting to do is somewhat malformed by definition... but it could also be equally as confusing to have entities with connections mysteriously lose them after a copy. One could have it so that any entity associated with a copied entity is also copied to ensure that all Associations are preserved, but I'm not sure this is clearer, would probably not be always desired, and would likely have a performance impact to boot.

It should be noted that the group_sub.entities = bp.entities statement does create a copy EntityList, which is currently at least unavoidable. Each blueprintable object needs to "own" it's own data, as I believe being able to pass by reference would be even more confusing than the convoluted method we have right now.

Suggestions on improving this are obviously welcome, particularly from an outsider's perspective. Structural changes can also be made, since I'm planning on releasing 2.0 in the near future so any breaking change is tolerable there.

[tephyr]

I think my main issue was not understanding how best to copy entities with connections. The docs don't call out a best practice (which sounds like it should be group_sub.entities = bp.entities). I didn't consider that approach because I thought the source blueprint and all the groups copied from it would become "entangled."

As for making partial copies of the .entities: in my usage of draftsman, I have not encountered a use case for partially copying entities from a blueprint "blindly" (expecting everything to just work, regardless of existing connections). My expectation is that if I need to grab a subset of the entities, I will also need to query them for any connections, and then handle them appropriately. I believe the framework should (and TMK does now) simply allow that, and then apply the existing validations on export or usage.

I will look into making the best practice for copying between blueprints & groups clearer in the docs. That might be the best resolution to this particular issue.