Improve canConnectToExternal() #111
Conversation
Pilzinsel64
changed the title
| @@ -174,6 +174,9 @@ public boolean canConnectToExternal(ForgeDirection dir, boolean ignoreDisabled) | |||
| if (node == null) { | |||
| node = part.getGridNode(); | |||
| } | |||
| if (node == null) { | |||
There was a problem hiding this comment.
That looks a bit suspicious tbh. What if this part w/o grid node is facing the conduit?
There was a problem hiding this comment.
Not sure. But with my Ultra Dense PR I had to change the code of this method. However, my change with this PR here adds a missing case from the code BEFORE my Ultra Dense PR to cover a few cases where ME Conduits wont connect anymore now (one example shown on the picture).
I know it looks bad, but not sure how to merge the code more without having some lines twice.
There was a problem hiding this comment.
@repo-alt See the previous code here: https://github.com/GTNewHorizons/EnderIO/blob/b22915cc9cc2903fed7f3d9cb4936025b30df644/src/main/java/crazypants/enderio/conduit/me/MEConduit.java#LL155C37-L155C37
It doesn't even check the node itself. With this PR I cover the case that the node is empty. In this case it probably can connect (at least most of the times I know), as long as the conduit is not Ultra Dense.
There was a problem hiding this comment.
@repo-alt I'm sorry, let me try again...
Before my PR that adds Ultra Dense Me Conduits the code was this:
https://github.com/GTNewHorizons/EnderIO/blob/b22915cc9cc2903fed7f3d9cb4936025b30df644/src/main/java/crazypants/enderio/conduit/me/MEConduit.java#LL155C37-L155C37
Then I had to adjust the methode canConnectToExternal() to get a similar behavior as the AE2 cables.
But this adjusted method doesn't catch the case anymore that the GridNode can be null, so it won't connect to AE2 cables in a few cases (one shown in the picture at the top).
Now, with this PR I want to fix that by handling this simple case again where there is no GridNode.
There was a problem hiding this comment.
Yes, I understand that, I just asked if it is possible to have a part w/o a grid node (how is that even possible?) facing the conduit. Obviously it should not connect in that case, and that was checked before.
There was a problem hiding this comment.
@repo-alt Ah ok.
I don't know why a part can not have a GridNode and I also don't know why that even happens here. I got it via debugging and did see that it can't find a GridNode. I also see that original Ender IO code also just look for the part and the code for Ender IO for v1.12.2 also looks similar.
My guess is that some Conduits get loaded before a GridNode for specific parts does exist. But that is only an idea.
For more I probably haven't the knowledge in Minecraft modding and in AE2 code.
There was a problem hiding this comment.
Hm, the only thing we could do here is to provide a method like canConnectFrom(direction) via the AE2 API and shrink the code of this method canConnectFrom() here probably to a few lines.
But I suggest to do that in another step and I suggest to merge this as first step as otherwise some ME Systems can break softly.
There was a problem hiding this comment.
You see, the code you add only works in the situation where there is a part attached to the cable, but in the picture you provide there is no part, so, how exactly can that change affect the situation on the picture?
Also, maybe there is an easy way to reproduce it? If so I might have a look what's going on there
There was a problem hiding this comment.
Ah, now I understand, I think...
There is of course a part on the picture: PartGlassCable. This is what the debugger tells me.
To reproduce:
- Put three me interfaces and connect them via a fluix glass cable. Then put a another glass cable below, as shown in the picture. Then, under this cable now put a me conduit (normal or dense) and see it connects.
- Save & Quit
- Load world again
- See the conduit doesn't connect unless you trigger a reconnection (e.g. with the wrench or by placing it again)
If you want I can provide you the test world shown on the picture.
Please let me know what you found out! :)
|
That's what will happen if the change is implemented as is |
|
Ah, I see. Hm, not sure what a "directional part" is, but you second idea sounds good. Sure it's not a good way how I implemented it at all yet, but it seems to work. What do you think? |
|
That is at least more universal solution |
Fixes a case where at loading the Grid sometimes the conduits may not connect.
I've tested this case with all conduit types and all me cables and some buses and mashines, still work as it should.
Before:
After:
