Construction Planner Continued

by azaghal

Ghosts require player's approval before construction bots are dispatched. This gives the player improved control over what parts of the factory get built, and when.

Utilities
1 year, 6 months ago
1.1
1.58K

b [FIXED] Underground belt changes direction in unapproved mode

1 year, 9 months ago

I guess I'm on a bug finding spree today. :)

When placing an underground belt in the unapproved mode, the belt reverses direction when placing the end of the belt:
https://i.imgur.com/o17mhqk.mp4

As it can also be seen in the video, when you're placing an underground belt in the unapproved mode, the guidelines for placing underground belt aren't there, though I don't know if that can be fixed given how the mod does its magic.
https://i.imgur.com/WaXtxHV.png
https://i.imgur.com/C1Sk356.png

1 year, 9 months ago

Hm... Well, you might be one of the rare users of the mod most likely - I haven't really started using the mod myself yet - still trying to assemble a nice little modpack first :)

I kinda recall having these issues with the original mod as well, but now at least I kinda understand why it happens. Undergrounds are I guess a bit special because the game does corrections to how they are oriented etc.

Hopefully it is possible to detect orientation of undergrounds and somehow correct them during placement.

1 year, 9 months ago
(updated 1 year, 9 months ago)

I haven't gotten much into Factorio modding (at least not yet ;) ), so I don't know if I'm on the right track here... I remember another modder had an issue with blueprints not remembering the correct direction of underground belts and it was an issue with there being two separate types for linked and unlinked belts. Quickly looking through the API I found this on LuaEntity:

linked_belt_type :: string [Read/Write]
Type of linked belt: it is either "input" or "output". Changing type will also flip direction so the belt is out of the same side
linked_belt_neighbour :: LuaEntity? [Read]
Neighbour to which this linked belt is connected to, if any.
Notes

Can also be used on entity ghost if it contains linked-belt

May return entity ghost which contains linked belt to which connection is made
Can only be used if this is LinkedBelt

Maybe that will be helpful in solving this issue?

1 year, 9 months ago

Linked belts are apparently a completely separate type of entity that kinda let you connect belts across surfaces etc. I don't even want to think what this code would do to those, and I'll just wait for someone to crash it and try to fix it then :)

I think I have found a way to figure what you ran into, but I just need to think a bit on how to take care of it. Generally speaking:

  1. Detect when a placed underground ghost is about to get unapproved (this is easy).

  2. Based on its direction and type (input/output), find closest matching underground that is also unapproved.

  3. Remember matched underground's type (input/output).

  4. Unapprove the ghost.

  5. Fix rotation for unapproved ghost and his connecting counterpart.

Step (2) is the main thing I need to wrap my head around a bit.

1 year, 9 months ago

Ok, so the new release should be able to handle the underground belts now - for the most part. There still might be some weird things going on if you having a mix of approved/unapproved ghosts in a straight line, but not much I can do about it.

P.S.
Erm, and just as I was typing this, I found one more bug - same-faced undergrounds (like > > ) end-up affecting each-others orientation, and they really shouldn't.

1 year, 9 months ago

Yeah, I missed a whole lot of cases in here when filtering the candidates... That's what you get for testing the happy path only :)

1 year, 9 months ago

Yup, the updated version works nicely, the belts go in the right direction now. If you can also figure out how to add the underground belt (and pipe) guidelines, that would be perfect! :) Without them it can be difficult to judge whether you're not placing the end part too far for them to still be connected. For now though, building using mouse press + drag works nice as a workaround in most places.

Though there is an issue with the dragging build - when placing buildings (not ghosts), moving over unapproved ghosts sets them as approved:
https://i.imgur.com/SQWF9Fi.mp4

1 year, 9 months ago

Well, the underground belt handling is going to be a bit more complex - I had some minor fixes to ensure new ones are placed oriented as expected, but then got into trying to get rotation working properly as well (when you have approved/base force on one side, and unapproved on the other). That's going to be a bit more complicated, and maybe not 100% satisfactory. There is also handling of rotation/direction when they are mixed etc, and that's going to be a bit of a pain.

The main concern for me right now is how all of this will affect the performance, but... We'll see :)

Guidelines will probably get added to todo/future improvements list for now - it would be nifty, but probably want to sort out main functionality first and clean-up all the bugs :)

Hm... Interesting thing with that dragging thing, initially I had a lot of issues like that with dragging ghosts/blueprints and managed to fix it by checking if you are dragging blueprints and ghosts. With placing of real buildings, not sure how easy it would be to fix (or possible). That auto-approval happens so that recipes of unapproved ghosts are preserved when player places building on top, and during that pre-build event you don't know what the player is trying to place down. Maybe I could read what's under cursor (if possible), but there'd be a whole lot of logic needed there (partial overlaps etc).

1 year, 9 months ago

Ok, handling underground belt rotation is a major pain... But I think I have mostly satisfactory solution for this at this point, and it even makes it possible to pair-up and rotate unapproved and approved ghost as if they are matched.

As for the issue you encountered with building while dragging, I'm not quite sure how to handle it. Anything that skips over distances (power poles, underground belts, underground pipes) will trigger the on_built_entity event for every tile they move over, in spite of nothing getting built. I would consider it a bug in modding API most likely, or maybe a limitation.

1 year, 9 months ago
(updated 1 year, 9 months ago)

From what I'm seeing, testing the events right now, on_built_entity fires only when the power pole is actually placed. The event that is firing while dragging is on_pre_build, which in Construction Planner Continued includes some code to approve unapproved ghosts (as a fix to some other mods causing problems). The fix will probably have to happen there. I'll dig around a bit more, maybe I'll figure something out. :)

Edit
Simply commenting out the approve_entities... on line 783 in control.lua fixes this issue, but I'm assuming that whole approval process was added because some other issue. The comments mention something about the Tapeline mod, but I don't seem to have any issues with it, whether that approval process is commented out or not.

1 year, 9 months ago

The purpose of approving entities during on_built_entity is actually to preserve the recipes etc that have been set on assemblers when player builds on top of unapproved ghost entities. It also allows undo do work correctly in such cases.

Tapeline used to cause issues, so I introduced that particular fix, but it was one of the first fixes, and maybe it is not necessary for it any longer - Tapeline caused approvals to happen when placing permanent tapelines (those would create temporary placeholders as player is dragging the tapeline).

To maybe spare some of your time - I have managed to create a pretty comprehensive set of fixes right now, and I have a decent (although non-perfect) solution for the undergrounds/electric poles etc.

One remaining case I want to cover there is to still make it possible to perform undo and to not have some leftover placeholders when dragging an underground results in placement on top of unapproved ghost. I've pushed the changes into fix-underground-handling branch (although, it handles way more than that...)

Final code I want to add is to:

  • Check if there are any valid candidates in the queue for unapproval (see a couple lines below when those get added) and unapprove them (since it's obvious we did not build on top of them). Empty out the queue and deregister on_tick handler if none are left at this point.
  • Check for event.created_by_moving and place_type condition in addition to seeing if it's undergrounds/electric poles etc.
  • Approve overlapping unapproved ghosts, but also store the entities/positions in case it's a "fake" build event. This is where that unapproval queue would get filled-in.
  • Add on_tick handler that's going to do a check if there are any valid candidates in the queue for unapproval. The handler will empty out the queue and then deregister itself. Basically we just want to run it once, and that's after the on_pre_built triggered.

So a bit convoluted, and also involves preserving some state, but I think it's going to work. If you use the branch for testing, don't save your main game - I use 999.999.999 version as development version, and it could mess with migrations and global data fixes if I need to introduce something in the last moment.

But if you can test out the branch, let me know if it works better :)

1 year, 9 months ago
(updated 1 year, 9 months ago)

Don't worry about me wasting time, I wanted to get into Factorio modding a long time ago, and digging through a live mod like that feels much better to learn than doing some abstract examples. ;)

I've tested the branch. It seems to be working as expected - no approval of in-between ghosts, proper rotation of undergrounds, undo is working well and leaves no placeholders behind. I guess it works! :)

1 year, 9 months ago

Yeah, I'm more than happy to see someone digging into the code for this one - what I wanted to say is that I had patches coming in already, so just didn't want you to spend too much time setting-up a PR :)

So, pushed one more commit, and this one actually tries to handle the case where unapproved ghost entity gets replaced by dragged "gapable" entity.

The only thing could not fix so far are rails because their selection boxes are kinda funky, and you don't really have access to them during on_built_entity. Although... I could maybe handle them in similar manner, but just by applying a bit more arbitrary selection box around them - would be a bit more suboptimal, but shouldn't bee too bad... However, I might leave that for next patch release, kinda been into this codebase a bit too much now :)

1 year, 9 months ago

Found some inconsistencies in last commit, I might be force-pushing some small changes, just a heads-up.

1 year, 9 months ago

And released finally... Unfortunately, when building rails in easy-build mode, they don't trigger the on_pre_build event, so probably won't be able to fix that one unfortunately. But it's good enough at the moment :)

Let me know if all the issues you posted got fixed, and I'll go ahead and update topic and lock the thread (just to keep it somewhat clean/short).

1 year, 9 months ago

The release version works fine. Alright, if anything new comes up, I'll start a new thread. Thanks for all the work you've done! ;)

1 year, 9 months ago

No worries, thanks for all the testing, I'm really glad you did not turn away in the face of bugs, and kept going. I think some of these additions have been really useful, and it did get me motivated to fix them :)

This thread has been locked.