Disallow pushing into invalid faces

[oversword]

Fixes #72

This is actually independent of ongoing discussions, as this uses get_secondary_node to determine validity, which will be modified by any alternate solution to the valid faces problem.

i.e. This fix should work with current and future code

[joe7575]

We could move this code into the function get_dest_node, which saved us 2 lines of code:

local _,node = Tube:get_node(pos)
local dir = side_to_dir(side, node.param2)

And it will also prevent also any pulls from invalid faces.
But anyhow, this is a heavy patch, because this check has to be done for each and every node in each and every push. And it is only needed for the teleporter so far, right? Or do you see other use cases?

I have to think of another case: https://github.com/joe7575/techpack/blob/master/tubelib_addons1/detector.lua#L136
This was necessary, in order not to be able to push into the detector node from the wrong side. Unfortunately the teleporter does not have this callback...

[oversword]

I definitely think there are more use-cases for this, but I am happy to try and simplify / speed up.

One use case is the liquid sampler, and perhaps the quarry too, it doesn't make sense to me that you would be able to push in from the active side (front in these cases)

Technically this is fine (existing functionality, not this PR) as pushing into a teleporter does not cause it to get redirected and confused, however it currently causes the "spooky connection" bug (when removing the source teleporter you are pushing into) simply because we only update the back & front of the teleporter when we remove it, if we updated all sides of the teleporter it would also fix the "spooky connection" bug by triggering the pusher to update no mater what side it's on.

However, I would like to come up with a solution that 100% disallows us to pass items in/out of these new "invalid faces", as I do think there are good use cases (as mentioned). I do however keep coming up with ways to circumvent this, I think this is why flux keeps saying this needs a re-write, just so complex :P

I actually gave get_dest_node a shot, that's actually what caused my confusion in the original comment I kicked this discussion off with, it's because it was automatically jumping to the end of the teleporter once they were connected, telling me the destination was "air" and not the teleporter - that's why I thought the metadata was disappearing, because once I restarted the destination was cached as the air block.

I definitely think you have a point though, will take another look at this later.

[joe7575]

Yo are right, this teleporter feature is much to complex. First it is a secondary node, but then it becomes part of the tube connection and therefore, completely "invisible". One solution would be to implement the teleporters as real secondary node, like ender chests.

[P]-------------[T]      [T]--------------[P]

Each pusher/teleporter pair is one tube connection. The teleporters are real nodes with a pairing feature. This can be implemented independent on the existing teleporters. They can then be switched off.

[joe7575]

because it was automatically jumping to the end of the teleporter once they were connected

Yes, this connection management is the main part of tubelib2. The tubes are only needed once, to find the path to the destination. The positions then are stored in the cache. This is the difference to other tube/pipe/cable implementations and the main reason, why tubelib2/TechPack/Techage is that lean and efficient (server friendly)

[oversword]

One solution would be to implement the teleporters as real secondary node, like ender chests.

I have considered that, though I decided against it just because I found another solution that didn't require a re-write - as far as I knew at the time! It certainly is confusing having teleporters act as both though, lets see if we can't sort this out, and if not, we may end up removing the valid faces feature and just making teleporters less tubey.

Diving into the get_dest_node code now.. perhaps the solution is actually to make the path routing algorithm refuse to route through invalid faces, then get_dest_node would return what we expect when the end point is the wrong side of a teleporter

[oversword]

I think that did the trick, merging with with #74 as I have effectively re-written the whole valid-face feature at this point

@joe7575 FYI just remembered another really important use case: currently tubes will automatically try to connect to the sides of pushers even though that's clearly invalid. Expect a PR for this project... tomorrow maybe... with this feature implemented for all the secondary nodes I think need it