shanemadden / factorio-power-grid-comb

MIT License
9 stars 5 forks source link

inconsistent connections #2

Closed ST-DDT closed 6 years ago

ST-DDT commented 6 years ago

There seem to be slight inconsistencies in your connection combing.

grafik

Expected result

The both selected poles are connected with a straight line.

Actual result

Existing connections will be destroyed and will connect to the big power pole.


The same applies if you select three poles in a row.

grafik


You can partially fix that by selecting at least two columns/rows.

grafik

This will fix the top right connection issue, but not the top left one. It will also destroy the connection to in the bottom left.


This is the expected result:

grafik

shanemadden commented 6 years ago

That's definitely a lot cleaner, but unfortunately it's a little tricky to implement - the way the mod works right now is by just re-placing the poles in a predictable order (row by row, top left to bottom right) and letting the game's automatic wire connection logic handle it from there.

Doing it this way is simpler and probably a lot better from a performance perspective (and the tool can already stall the game for a bit if you drag it over a whole base) so I'd like to leave that alone for the most part, but I think I have an idea that might give you the behavior you want:

The tool can have a different behavior when holding down the shift key to select an area. My thinking was that I could make it so that when you shift-drag the tool, the poles selected will disconnect all of their wires then connect only with neighbors in range that are perfectly aligned with them either vertically or horizontally (then autoconnecting any poles that don't have a connection after we've wired up the perfectly-aligned poles). I think it'd be a little slower in the code, but might give you the behavior you'd want?

ST-DDT commented 6 years ago

Sounds like a good idea to me. But maybe this should be the main way. The performance impact can be reduced by only fixing 1 to 10 poles per tick. I can help you with that if you want.

Does destroying and re-placing the poles destroy circuit networks? Thats definitly an unexpected behaviour.

shanemadden commented 6 years ago

Yeah, the logic is pretty simple now so that I could avoid doing on_tick handling and queuing, but that would definitely help with performance issues. I'll probably have some free time this weekend to set up the orthogonal-preferred connection logic, we'll see how it performs and go from there. And yeah, re-placement of the poles removes circuit connections to them - the mod currently tracks and re-places red/green wire connections on affected poles, but the new mode may not need to destroy the poles at all since it'll be directly managing the copper wire connections with neighbors.

lossycrypt commented 6 years ago

I don't think you can do queueing at all, as the whole effect relies on the fact that all wires are simultaenously removed. For example if a queue tried to fix a "weird" wired pole without also removing all cables from the neighbors it would simply place the same "weird" pole again (unless ofc you do non-destructive fixing).

As a quick fix to the described problem i think you can get better results if you rebuild poles in order of wire reach. I.e. 1) Destroy all poles 2) Rebuild all small poles 3) Rebuild all medium poles 4) Rebuild all large poles Ofc with proper numeric sorting to work with modded poles :P

shanemadden commented 6 years ago

Ahh yeah, that does seem to help a lot, since it lets the smaller poles create their grid then the longer-reaching poles get placed, without having a chance to interfere.

@ST-DDT Try it out under 0.16.4, it gets the expected result from your example when selecting all of the poles, hopefully the behavior is a little closer to desired?

ST-DDT commented 6 years ago

Yes, for all setups that have a good way to be combed this works very well. There are some edge cases where the surrounding power poles mess things up, but I guess this is caused by the underlying/Factorio's connection search logic and thus there is nothing we can really do about it.