Refined Storage

Refined Storage

77M Downloads

INetworkNode needs to be a capability

raoulvdberge opened this issue ยท 7 comments

commented
commented

Hi @raoulvdberge

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 add getDimension() to IWirelessTransmitter
  • Remove canConduct(...) and use tile.hasCapability(...) instead (it has face check)
  • Remove getPosition() and replace with getItemStack() (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.

commented

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.

commented

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).

commented

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() ?

commented

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

commented

@Volune

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.

commented

@Volune The 1.11 branch is now public