Add network signal insulators #3963
Conversation
|
Your Pull Request was automatically labelled as: "🎈 Feature" |
Slimefun preview buildA Slimefun preview build is available for testing! https://preview-builds.walshy.dev/download/Slimefun/3963/813b55b7
|
src/main/java/io/github/thebusybiscuit/slimefun4/api/network/Network.java
Show resolved
Hide resolved
J3fftw1
added
the
🎯 Needs testing
|
I like this feature, only worried about performance on this. Also we need some general testing on this. This is a major change for 2 essential things in Slimefun. |
|
Agreed, needs some good testing :) |
|
Can we also get a toggle on this maybe within a gui. |
What about right clicking the insulator to enable/disable it? Also yes, please test this a lot. I have tried my best to break this already, but I would feel a lot safer if someone else tested this as well. |
|
Yes that’s fine by me as well |
|
both cargo and energy insulators seem to be working as intended. my suggestion would be alter the appearance as the only difference is a white dot on the top. |
|
Have you tested this with a large network or small network? |
|
i didnt have a large network setup to test on. it was a pretty basic one |
|
Alright, good to know. Maybe someone with a big network setup can also test this a little? |
src/main/java/io/github/thebusybiscuit/slimefun4/utils/HeadTexture.java
Outdated
Show resolved
Hide resolved
| for (int i = 1; i <= getRange(); i++) { | ||
| Location newLocation = l.clone().add(i * xDiff, i * yDiff, i * zDiff); |
There was a problem hiding this comment.
Slightly confused by this. What is this method supposed to do?
As it is written I don't see it ever being useful to pass anything but one unitary value (with sign) and two zeros, like you do below, therefore the method only updatesNeighbors in one direction.
But then the question arises on whether this format even makes sense, it would be a lot more readable (though not as compact) if it accepted a BlockFace at that point.
Was the idea just to avoid having to "decode" what versor each BlockFace refers to?
There was a problem hiding this comment.
This method checks if any Locations need to be added to the network when an insulator is broken.
It indeed is intended to only check the 6 directions in which a network can connect. The reason I went with this signature is because the method is similar to discoverNeighbors and that used the same way of passing the arguments. If you'd rather have the method take a BlockFace I would be happy to change it. Then perhaps discoverNeighbors should also be changed to take a BlockFace for consistency sake.
There was a problem hiding this comment.
Oh my god I had not even noticed the other one. If you could make both accept a blockface that would be awesome
Co-authored-by: JustAHuman-xD <[email protected]>
Description
A feature for energy and cargo networks to stop nodes from connecting.
Proposed changes
Add 2 items, 1 for energy and 1 for cargo, that you can place in between network nodes to stop them form connecting.
To accomplish this the following is necessary:
INSULATORto the network and energy component enums to classify them.Related Issues (if applicable)
Implements approved suggestion #1652
Checklist
NonnullandNullableannotations to my methods to indicate their behaviour for null values