redruin1 / factorio-draftsman

A complete, well-tested, and up-to-date module to manipulate Factorio blueprint strings. Compatible with mods.
MIT License
94 stars 17 forks source link

Circuit connections inside group within a blueprint fail to create blueprint string #16

Closed elswindle closed 2 years ago

elswindle commented 2 years ago

After adding circuit connections to entities within a group and adding that Group to a Blueprint, when I try to convert the blueprint to a string, I get a ValueError that the connected entity is not in the list. More specifically, I load up a blueprint and add a few circuit connections (at this point, the blueprint is still good and can export to string with no issue). I then create a Group from the entities of the blueprint. I add the Group to a new Blueprint and try to create the string from it, but it breaks. If I do not make the circuit connections, there's no problem and can import it to Factorio no problem. I've attached my code and the error code:

Exception has occurred: ValueError <LogisticRequestContainer '5'>{'name': 'logistic-chest-requester', 'position': {'x': 43.5, 'y': -0.5}, 'connections': {'1': {'green': [{'entity_id': <Association to Inserter with id '0'>}], 'red': [{'entity_id': <Association to Inserter with id '4'>}]}}} is not in list

# Labs bp
bp_str = "0eJy1ldtugzAMht/F10kFAUrhcq8xTRMHq4sGgZFQDVW8+5K2q+ja0ZCtVxzsfL/j2PEe8qrHtuNCQboHXjRCQvq8B8m3IqvMPzW0CClwhTUQEFltvnLMtCuMBLgo8RNSfyR3F1VZPlnBxhcCKBRXHI+aEzcCbSO1RWtomvam/ioiMOhlq8gwNFgaU9s1ZV8ovuNqoLV+r5AGBn5iD6+ir3PsThFehn+tEpxUoksV2SKW83hmg/ec8YEFnjnTwwldqqx4p1xI7JS2/b4J7yDzgxQtIM2C1hNQ1Wy5VLygxRtKRTv86PVzNjh6Exo7QeeZG5fcmXLWqJJ3WBzt4RU4cUilDdf3plm41WzfvWbawbHZrLrtHPV6ccX6Nv3G/sC3aTjKLitjkUBoewruN160JEcOW1jf2wL9h0qKl9zbLie9eejV6iePLVTmmTl6GLTpZJgT2GEnDwps44dxwuKQJUmQ+MQcFeopDU9n73H8AlTBmZE="
bp = Blueprint(bp_str)
bp2 = Blueprint(bp_str)

new_id = 0
inserters = bp.find_entities_filtered(type='inserter')
for ins in inserters:
    ins.id = str(new_id)
    new_id += 1
    base_pos = ins.position
    pos1 = (base_pos['x'], base_pos['y']+1)
    nb = bp.find_entities_filtered(position=pos1)
    if(nb[0].type == 'logistic-container'): 
        nb[0].id = str(new_id)
        new_id += 1
        bp.add_circuit_connection('red', ins.id, nb[0].id)
    pos1 = (base_pos['x'], base_pos['y']-1)
    nb = bp.find_entities_filtered(position=pos1)
    if(nb[0].type == 'logistic-container'):
        nb[0].id = str(new_id)
        new_id += 1
        bp.add_circuit_connection('green', ins.id, nb[0].id)

g1 = Group('g1', entities=bp2.entities)
g2 = Group('g2', entities=bp.entities)

for entity in g2.entities:
    entity.position['x'] += 9
bp3 = Blueprint()
# bp3.entities.append(g1)
bp3.entities.append(g2)

print(bp3.to_string())
elswindle commented 2 years ago

A quick addition to this, but this error is not limited to circuit connections, but power connections as well. That's probably a given, but better to be thorough.

redruin1 commented 2 years ago

Alright, this is finally fixed in 0.9.6. The code you provided above is verified to work and produce the correct blueprint string. However, I've rewritten some of the code to highlight some potential syntactic improvements:

# Removed duplicate beacons from the string causing OverlappingObjectsWarnings
bp_str = "0eNqtldtugzAMht/F10lVApTC5V5jmiYOVhcNAktCNYR49yVtN9G1oyTaFQc73x87djxCUffYSS40ZCPwshUKsucRFD+IvLb/9NAhZMA1NkBA5I39KjA3rjAR4KLCT8iCiTxcVOfFbAWbXgig0FxzPGvO3Ah0rTIWo2FoxpsGm5jAYJZtYsswYGVNnWyrvtT8yPVAG/NeIw0t/MIeXkXfFCgvO7ze/q1KeFGJr1VUh1gt49ka/NYbH67AM296NKMrnZfvlAuFUhvb30FsTzK/SLEDaRG0m4Hq9sCV5iUt31BpKvGjN8/FzdG70MQLuszc++TOlrNBVVxiebZHN+DUI5VruMF2noV7zfbda7YdPJstcGkHunOu2IA5NIQPf03DUXZdGU4C0dpT8L/xYpcceYSwexQC/YdKSlzubZ+T3ttBdJpU2WwaEjiiVCcJtg+iJGVJxNI0TANiY0Uz5uDpx3uavgA9W1xx"
# unecessary to create two blueprints, the data is copied anyway
bp = Blueprint(bp_str)
# bp2 = Blueprint(bp_str)

new_id = 0
inserters = bp.find_entities_filtered(type='inserter')
for ins in inserters:
    ins.id = str(new_id)
    new_id += 1
    base_pos = ins.position
    pos1 = (base_pos['x'], base_pos['y']+1)
    # I should probably write a function `find_entity_at_position``, or rewrite
    # `find_entity` to have `name` as an optional parameter. Thoughts?
    nb = bp.find_entities_filtered(position=pos1)
    if(nb[0].type == 'logistic-container'): 
        nb[0].id = str(new_id)
        new_id += 1
        bp.add_circuit_connection('red', ins.id, nb[0].id)
    # Im also not happy with having to extract and construct a new tuple for simple offsets
    # like this; I should probably integrate some 2d vector library and convert everything to 
    # that
    pos1 = (base_pos['x'], base_pos['y']-1)
    nb = bp.find_entities_filtered(position=pos1)
    if(nb[0].type == 'logistic-container'):
        nb[0].id = str(new_id)
        new_id += 1
        bp.add_circuit_connection('green', ins.id, nb[0].id)

# also unecessary to create two groups, as we can simply modify one group
# If you still need the different IDs, you can set them in a loop similar to 
# above
g1 = Group(entities=bp.entities)
# g2 = Group('g2', entities=bp.entities)

# for entity in g2.entities:
#     entity.position['x'] += 9

final_bp = Blueprint()
final_bp.entities.append(g1) # copy at (0, 0)

# Setting a group's position automatically translates sub-entities on export
final_bp.position = (9, 0)

final_bp.entities.append(g1) # copy at (9, 0)

print(final_bp.to_string())

Perhaps I should make an optional constructor to initialize a Group from a blueprint string, which would eliminate the malarkey of creating a "scratch" Blueprint just to import it into a Group. Would that be useful?

elswindle commented 2 years ago

Having a constructor for Group that accepts a string would be nice since it'll save computation time converting from a blueprint to a group. That will be good considering my comment in #20.

redruin1 commented 2 years ago

Alright, I'll put it in now. What attributes would you like to acquire from a blueprint string? Should it just be the entities, or is there extra information that you would like to extract?

elswindle commented 2 years ago

I think entities is all I should need. If not, I can just override the __init__ function and grab more data. I feel like the only other thing I might need is some location data, like local/absolute position. I haven't gotten to that point yet.