INetworkNode needs to be a capability
raoulvdberge opened this issue ยท 7 comments
I was having a look at this issue (before I try to have a look at enderio cables), and I'd like to suggest a few refactoring on the INetworkNode
:
- Remove
getNodeWorld()
and addgetDimension()
toIWirelessTransmitter
- Remove
canConduct(...)
and usetile.hasCapability(...)
instead (it has face check) - Remove
getPosition()
and replace withgetItemStack()
(will also allow other mods to set nbt tags)
And at last one I'm not really sure about:
- Remove
isConnected()
, replace with getNetwork() != null
I can work on a pull request for that if that helps.
getNodeWorld()
is only used to get the dimension of a IWirelessTransmitter. That's why I'd like to remove it after putting getDimension()
getPosition()
is only used to get the item of a node, for displaying in the controller UI, so I think a getItemStack()
would be more appropriate - maybe with a better name.
Ah. Good point. You were ahead of me there :)
In that case, feel free to work on the PR but wait til I make the 1.11 branch in that case (which should be later today).
My current work is there
I'm waiting your new branch to rebase and open a pull request.
The changes were pretty big regarding the refactoring of isConnected()
, and rework of rebuild()
to integrate the capability. By the way, is there an actual need to getConnectableConditions()
?
At first sight, most changes seem OK (I like the network graph refactor). However, a proper PR will be needed for further review.
For the justification of the connectable conditions see #650
No work has been done on 1.11 so far because the Forge DL servers are down for the past day already, unfortunately
Why do you want to remove getNodeWorld()? It's used in a bunch of places.
getDimension() in IWirelessTransmitter seems like a good idea, yes. Ideally, IWirelessTransmitter should extend INetworkNode though, which gives us world access (and thus dimension ID access).
The canConduct() change seems good, yes.
Why is getItemStack() a valid replacement for getPosition()?
And I think we can remove isConnected(), yes.
@Volune The 1.11 branch is now public