search

raiguard / Factorio-InfinityMode

A modernized and optimized replacement for Creative Mode in Factorio. Tools are consolidated and updated to be both more functional and more streamlined.
MIT License
8 stars 2 forks source link

Infinity loader snapping/belt type logic is unreliable #2

Closed zero318 closed 5 years ago

zero318 commented 5 years ago

After experimenting with the infinity loaders for awhile, I've noticed several abnormal behaviors in the snapping/belt type logic. (I've attempted to provide screenshots when possible, but I haven't actually tried linking images in an issue before, so hopefully it works.)

  1. When a loader set to input mode tries to change belt type, the loader changes direction instead of switching to output mode. (Before/After)

  2. Existing loaders don't respond to placing/rotating splitters despite the placement code specifically checking for them. (Before/After)

  3. Placing loaders facing the sides of splitters can trigger a change in belt type despite the belts not connecting. (Before/After)

  4. No checks are made for regular "loader" type entities at any point.

For 1, I've determined that the update_loader_types function is the source of the issue, but I'm not sure which line is causing the effect. Switching the order of the calls to update_loader_types and snap_to_belt in the on_built_entity listener seems to solve the problem, but the underlying bug should still be investigated.

Regarding 2, 3, and 4, it might be possible to solve all three by switching some of the logic to use the belt_neighbours entity property that was added in 0.17.59. This property only returns adjacent belt-related entities that have actually formed a connection, thus preventing 3 by default.

raiguard commented 5 years ago

Thanks for letting me know that belt_neighbors exists! I need to read the documentation more often...

Anyway, I'll get on it. I thought the snapping logic was too simple to actually work...

zero318 commented 5 years ago

Actually, I think I just finished making a version that seems to work properly.

Belts/undergrounds ended up working ~95% properly just by simplifying line 175 to be "direction = direction", so I ended up leaving the original logic for belts/undergrounds and splitting splitters/regular loaders into separate functions. This still doesn't account for side-loading the input or output infinity loaders with belts/undergrounds, but a full/proper/better implementation of belt_neighbors logic will end up covering that.

The code is a bit of a mess since I was just trying to understand all the issues I saw, but it seems to work. The fact that belt_neighbors returns a read only table makes it annoying to use. Ended up having to store information about the neighbors and then refind them with find_entities_filtered in order to get a writable table.

Github wouldn't let me attach a .lua file to my comment, so I've forked the repository and committed the file to the fork.

raiguard commented 5 years ago

Ooo, that's nice! I was in the process of trying to figure it out, but you just did the hard part for me!

If you submit a pull request, I can give you credit for the work instead of just copying it and committing it myself. I'll clean up your code a bit and make some other changes I planned on. Thanks for the contribution!

zero318 commented 5 years ago

Went ahead and made the pull request. This issue should probably remain open until belt/underground sideloading logic can be figured out since it's currently present for splitters/loaders.

raiguard commented 5 years ago

Will close this once the patch with the new logic is released.

raiguard commented 5 years ago

v0.3.0 is out, closing!