Multi-Input LDPs - #7825
Conversation
Cardinalstars
added
Enhancement
…5-Unofficial into Multi-input-ldp
| gt.blockmachines.long_distance_item_pipeline.desc=Sends items over long distances | ||
| gt.blockmachines.long_distance_fluid_pipeline.desc=Sends fluids over long distances | ||
| gt.blockmachines.long_distance_pipeline.desc=Only one Input and Output are allowed per pipeline\nOnly Input and Output have to be chunkloaded\nTransfer rate is solely limited by input rate\nMinimum distance: %s blocks | ||
| gt.blockmachines.long_distance_pipeline.desc=One output and many inputs allowed\nOnly Input and Output have to be chunkloaded\nTransfer rate is solely limited by input rate\nMinimum distance: %s blocks |
There was a problem hiding this comment.
| gt.blockmachines.long_distance_pipeline.desc=One output and many inputs allowed\nOnly Input and Output have to be chunkloaded\nTransfer rate is solely limited by input rate\nMinimum distance: %s blocks | |
| gt.blockmachines.long_distance_pipeline.desc=One output and many inputs allowed\nOnly output and inputs have to be chunkloaded\nTransfer rate is solely limited by input rate\nMinimum distance: %s blocks |
| || mTarget.mSender.mTarget.isDead()) { | ||
| mTarget.mSender = this; | ||
| mTarget.mTooCloseSender = null; | ||
| if (mTooCloseTarget != null && !mTooCloseTarget.mSenders.contains(this)) { |
There was a problem hiding this comment.
I'm not sure this is a correct translation of the previous condition. This condition reads as "does this have a tooCloseTarget, and does that TooCloseTarget not have this as a sender". The second condition should always be true if the first is, and so is unneeded.
| mTooCloseSenders.removeIf(p -> p.isDead() || p.mTarget != null); | ||
|
|
||
| if (mTarget == null || mTarget == this) return false; | ||
|
|
||
| // prune stale senders on the target, then register ourselves always | ||
| mTarget.mSenders.removeIf(p -> p.isDead() || p.mTarget != mTarget); |
There was a problem hiding this comment.
These pruning steps are now linear, which means for the whole pipeline potentially exponential with multiple inputs. This path is fairly hot (called by every inventory-altering function), so could this get moved to only fire where the line is altered?
Also, the comments are basically just restating the code, and could either be removed or combined into one block that states what the whole section does, if you think they're worthwhile.
| Arrays.asList( | ||
| "Is Pipeline Output", | ||
| "Pipeline Input is at: X: " + coords.posX + " Y: " + coords.posY + " Z: " + coords.posZ)); | ||
| if (!mSenders.isEmpty()) { |
There was a problem hiding this comment.
If an end is removed, but the middle of the pipe is not chunkloaded, then the machine block update never gets here, mSenders is not reset, and may hold stale MTEs. Should re-add the isDead and contains checks.
| // prune stale senders on the target, then register ourselves always | ||
| mTarget.mSenders.removeIf(p -> p.isDead() || p.mTarget != mTarget); | ||
| mTarget.mSenders.add(this); | ||
| mTarget.mTooCloseSenders.remove(this); |
There was a problem hiding this comment.
This statement should be a no-op. this cannot be a member of mTooCloseSenders of mTarget unless mTarget = mTooCloseTarget. Pre-existing, but worth removing. The moved pruning of mTooCloseSenders should be sufficient.
| protected MTELongDistancePipelineBase mTarget = null; | ||
| // these two are updated by machine block update thread, so must be volatile | ||
| protected volatile MTELongDistancePipelineBase mSender = null; | ||
| // these two are updated by machine block update thread, so must be volatile or Concurrent hash map |
| mTarget.mSenders.add(this); | ||
| mTarget.mTooCloseSenders.remove(this); | ||
|
|
||
| return mTarget.mSenders.contains(this); |
There was a problem hiding this comment.
Condition is always true given two lines up, so can just return true.
3 participants
Summary
This Pr changes LDP's to be able to have multiple inputs while still being locked to only one output. I feel that is a natural design to the LDP to make them useful when you have multiple outposts if you have multiple outposts on the way to a far outpost meaning you don't need to have 3 LDPs for 3 outposts if they are on the way to each other.
Testing was done to ensure that it's never possible to LDP between short distances even if there are valid inputs at a distance.
Validation of all inputs and outputs should happen if any of the inputs, outputs, or pipelines change.
Checklist