Skip to content

Teleport pipes bugfixes, code cleanup, and other misc bugfixes - #203

Closed
Munchy907 wants to merge 28 commits into
tcooc:masterfrom
Munchy907:master
Closed

Teleport pipes bugfixes, code cleanup, and other misc bugfixes#203
Munchy907 wants to merge 28 commits into
tcooc:masterfrom
Munchy907:master

Conversation

@Munchy907

@Munchy907 Munchy907 commented Sep 6, 2025

Copy link
Copy Markdown

Included Rishum-P's Fix for Distribution Pipe Stall
Updated gradle minor version (keep in line with Buildcraft)
Added way to set player name with a gradle property
Made PipeBehaviorTeleport properties protected, and replaced direct calls to to them with getters & setters
Moved translation keys & nbt tag strings to there own classes
Fixed teleport pipes not reconnecting on chunk load
Fixed fake pipes being added to the teleport manger
Added a Pipe UUID to make equals() a lot easier
Move teleport side to PipeBehaviorTeleport from PipeBehaviorTeleportItem, and made it settable through the GUI
Added GUI button to set teleport side, default is Top
Moved owner name to a ledger, exactly like how Buildcraft engines work
Made pipes below colorable:

  • Advance Wood Pipe
  • Gravity Feed Pipe
  • Item Switch Pipe
  • Fluid Switch Pipe

Added back classic teleport pipe recipe
Fixed the GUI of Advanced Wood pipes
Bump version to 6.0.0.9

Rishum-P and others added 15 commits December 24, 2022 16:04
…lank for default forge naming, aka player + a random number 3-digit number that changes each time you runClient
…oaded like chunk loaded, DOES NOT include when the pipe is place).

Commented out adding teleport pipe to teleport manager in the constructor as the pipe it added had no pipe behavior or IPipe defined yet, thus they would return null
Made public none final class variables protected, and replaced all calls to the variables directly to getters and setters
Added states enum so magic numbers aren't placed everywhere with comments having to explain them
Added a class that contains nbt tag strings, to remove hardcoded strings. Currently only for teleport pipes
Added a class from my TeleportPipeRemade mod (was made to learn how everything for teleport pipes) currently contains translation key strings, to remove hardcoded strings. Very limited to one key in teleport pipe rn
…lass. Now teleport side is set in the GUI, and updates when they click the button in real time.

Updated the gui to include a teleport side button as well as increased the blank background to suit fit this
Enabled coloring of:
- Advance Wood Pipe
- Gravity Feed Pipe
- Item Switch Pipe
- Fluid Switch Pipe
Added back classic teleport pipe recipe
Fixed the GUI of Advanced Wood pipes
Bump version to 6.0.0.9
Fixed teleport fluid pipes duping fluid that went into it. Though this issue is appears to down to buildcraft api's PipeEventFluid.OnMoveToCentre event.fluid and it's amount not always being accurate
Fixed teleport fluid pipes duping fluid that went into it. Though this issue is appears to down to buildcraft api's PipeEventFluid.OnMoveToCentre event.fluid and it's amount not always being accurate
Bumped version
@multiplemonomials

Copy link
Copy Markdown
Collaborator

Question: Were you getting a bunch of asset download errors when running gradle commands? Because I think I figured out how to fix that

@Munchy907

Copy link
Copy Markdown
Author

It said it failed to download the assets one time, but I might of accidently fixed that issue when solving my IDE not being able to import from minecraft source, but at the same time it would build properly. Out of curiosity, how did you fix the asset download error? Will also help in the future if I come across it

@multiplemonomials

Copy link
Copy Markdown
Collaborator

That was fixed by switching to the fork of ForgeGradle!

itemsTeleportPipeItem = PipeCreator.createPipeItem(itemsTeleportPipeDef);

// add assembly recipe for Item Teleport Pipe
/* // add assembly recipe for Item Teleport Pipe (Deprecated)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait why are we deprecating this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I swapped back to the old recipe of 2 diamond gears, as I was unable to get the recipe to showup in jei. Works for normal buildcraft, but not add pipes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah I have seen that, the recipe does work but doesn't show in JEI. Maybe could we open a bug for this and them come back to it later?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped back and added buildcraft compat for jei support

Comment thread build.gradle Outdated
Comment thread build.gradle Outdated

dependencies {
compile 'BuildCraft:buildcraft:7.99.24.1:dev'
implementation('BuildCraft:buildcraft:7.99.24.1:dev')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like buildcraft actually just released the 8.0 release, should we try and update?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, i've been flipping between buildcraft pre 8.0 release, 8.0 release, and buildcraft remastered. Will update to 8.0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to 8.0

Comment thread src/main/java/buildcraft/additionalpipes/AdditionalPipes.java Outdated
super(GuiTeleportPipe.this.mainGui, OVERLAY_COLOR, true);
this.title = TranslationKeys.TELEPORT_LEDGER_OWNERSHIP;

appendText(() -> ((!Objects.equals(pipe.getOwnerName(), ""))? pipe.getOwnerName() : "no-one"), 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should "no-one" be a localized string here?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can at least help with the Japanese localization, the Japanese would be "だれも"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was taken from buildcrafts ownership ledger, but modified to work with pipes. But yea, seems like it's an oversight and should be localized string. Will fix that

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added the Japanese localization as well



//Fix Stall
if (stackPartThisSide.stack.isEmpty()) {

@multiplemonomials multiplemonomials Sep 27, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking through the logic, I am really confused how this if statement could ever be true.

  • stackPartThisSide.stack has an item count equal to Math.min(getItemsLeftThisSide(), entry.stack.getCount())
  • getItemsLeftThisSide() must be greater than 0 or the check on line 97 would fail and then we would move to another side that accepts at least 1 item
  • entry.stack.getCount() must be greater than 0 or the check on line 95 would fail
  • So, the count of the new ItemEntry must be greater than 0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not me, I forked from https://github.com/Rishum-P/AdditionalPipesBC Which is what is used in tekkit 2 & tekkit smp. I'll mention him with this message in the tekkit 2 discord

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this around three years ago to resolve an issue where Tekkit 2 servers were stalling due to the distribution pipe. It was an issue that affected basically every server at the time and led to the pipe being banned on practically every server. Eventually, malicious players began to exploit this to crash servers intentionally.

It was a long time ago, so I would have to review how it worked again and why the check is needed. When I added it, I was able to step through with debugging and hit it, but I'll get back to you on it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Tekkit 2 a 1.12 pack? Or did it use an older additionalpipes version

@Rishum-P Rishum-P Sep 30, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes Minecraft 1.12.2. I'll grab reproduction steps tonight. If I remember it was trivial to do.

@Rishum-P Rishum-P Sep 30, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's extremely easy to reproduce. All you need to do is break the pipe that the distribution pipe is transferring into while items are in transit, with no alternative route for the items to follow (i.e., other colours set to 0). You trigger a server stall (generated with sampler's stall reporting) with:

	at net.minecraft.item.ItemStack.forgeInit(ItemStack.java:1291)
	at net.minecraft.item.ItemStack.<init>(ItemStack.java:129)
	at net.minecraft.item.ItemStack.func_77946_l(ItemStack.java:454)
	at buildcraft.additionalpipes.pipes.PipeBehaviorDistribution.splitStacks(PipeBehaviorDistribution.java:102)
	at java.lang.invoke.LambdaForm$DMH/608392736.invokeVirtual_LL_V(LambdaForm$DMH)
	at java.lang.invoke.LambdaForm$BMH/1634695718.reinvoke(LambdaForm$BMH)
	at java.lang.invoke.LambdaForm$MH/843164542.invoke_MT(LambdaForm$MH)
	at buildcraft.transport.pipe.PipeEventBus$LocalHandler.handleEvent(PipeEventBus.java:178)
	at buildcraft.transport.pipe.PipeEventBus.fireEvent(PipeEventBus.java:116)
	at buildcraft.transport.tile.TilePipeHolder.fireEvent(TilePipeHolder.java:524)
	at buildcraft.transport.pipe.flow.PipeFlowItems.onItemReachCenter(PipeFlowItems.java:313)
	at buildcraft.transport.pipe.flow.PipeFlowItems.onTick(PipeFlowItems.java:271)
	at buildcraft.transport.pipe.Pipe.onTick(Pipe.java:250)
	at buildcraft.transport.tile.TilePipeHolder.func_73660_a(TilePipeHolder.java:261)
	at net.minecraft.world.World.func_72939_s(World.java:2125)
	at net.minecraft.world.WorldServer.func_72939_s(WorldServer.java:742)
	at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:978)
	at net.minecraft.server.dedicated.DedicatedServer.func_71190_q(DedicatedServer.java:475)
	at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:828)
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:687)
	at java.lang.Thread.run(Thread.java:750)

It also occurs in single-player mode, but it crashes the client.

I have attached an example video here: https://streamable.com/ymzu3e (at the end the server has stalled and won't recover till its timeout time is hit and process killed)

There may be better ways to fix it than what i did, however, it was a quick fix at the time.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh gotcha, thanks for the detailed example! I will look into this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK sorry for the delay but finally had time to give this some attention. Created this branch with my own take on fixing this issue: #206

The main difference is that with my branch, the un-routable item will be dropped on the ground instead of getting deleted.

canConnect = newCanConnect;
pipe.getHolder().scheduleNetworkUpdate(PipeMessageReceiver.BEHAVIOUR);
//pipe.getHolder().scheduleNetworkUpdate(PipeMessageReceiver.BEHAVIOUR);
pipe.markForUpdate();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why was this changed? Also can we comment out the old code?

@Munchy907 Munchy907 Sep 28, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot remember the technical reasoning. But old version would not give a block update meaning it would show the pipes either connected when it should be disconnected or vice versa.

I did comment out the old code?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*sorry, remove the commented code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Changed version to 6.0.0.9
Change http to https for have links
Added back assembly table recipe, removed old recipe
Updated jei to latest stable build
Added Buildcraft Compat
Removed commented out old code in PipeBehaviorSwitch
Add localized string for no-one, which currently is only supported in en_us & ja_jp
@xJon

xJon commented Oct 4, 2025

Copy link
Copy Markdown

Any chance this can be compatible with both BC mainstream (v8.0.0) and BC Remastered?

@Munchy907

Copy link
Copy Markdown
Author

We don't use anything that's exclusively in BC 8. So I see no reason why it wouldn't be compatible. Just to make sure, I added it to tekkit smp and checked and worked without issue

@Munchy907

Copy link
Copy Markdown
Author

Too many changes at once. Will redo each fix as it's own pull request

@Munchy907 Munchy907 closed this Dec 31, 2025
@xJon

xJon commented Mar 21, 2026

Copy link
Copy Markdown

@Munchy907, any idea whether this bug was resolved? xJon/Tekkit-SMP#60

@Munchy907

Copy link
Copy Markdown
Author

@Munchy907, any idea whether this bug was resolved? xJon/Tekkit-SMP#60

It was not in the bug fixes. big reason I missed it was I thought teleport tethers are uncraftable as it had WIP next to it. The bug is a quick fix, and will create a pull request with it when I get back in few days. I tried sorting it now, but forge 1.12.2 doesn't like apple silicon macs and just crashes when I try run it.

The issue itself is caused by BlockTeleportTether.java not calling ModelLoader.setCustomModelResourceLocation

@xJon

xJon commented Mar 26, 2026

Copy link
Copy Markdown

@Munchy907 Thank you! If helpful, I heard that the Cleanroom modding template supports developing on Apple Silicon.

@Munchy907

Copy link
Copy Markdown
Author

@xJon all sorted, thanks for bringing the bug to our attention. I've just got to fix the minor bug found in #206 then @multiplemonomials should be able to make a new release in a few days.

Also, thanks a lot for the cleanroom link! Sorted the issue out

@xJon

xJon commented Mar 27, 2026

Copy link
Copy Markdown

Thank you @Munchy907 & @multiplemonomials for keeping 1.12.2 mods alive and well 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants