search

codetaylor / pyrotech-1.12

An early game mod with new primitive devices, combustion machines, smelting mechanics, storage options, tools, torches, advancements, and absolutely zero GUIs -- with exception to the substantially complete, mostly illustrated, and charred guidebook.
https://pyrotech.readthedocs.io/en/latest/
Other
52 stars 20 forks source link

Different fluid capability on blocks? #420

Closed Primitive-Human closed 9 months ago

Primitive-Human commented 1 year ago

Intro

I like to integrate Pyrotech's blocks into my multiblocks using MultiBlocked where I can use these as inputs and outputs for my more primitive machines. Except I've come across a problem regarding (fluid) capabilities.

Issue Description

The several placed fluid containers in Pyrotech seem to have a different implementation of the fluid capability, which leads to other mods not recognizing these blocks as proper fluid containers. And this is a problem when another mod only accepts blocks through the capability system, like MultiBlocked.

What Happens

Using the Screen with Fluid Screen module from RFTools, I try to detect the amount of fluid in Pyrotech placed fluid containers (Barrel, stone/refractory tank, collectors, soaking pot etc), and found that most of these do not properly register as a fluid container. I make this statement based on the RFTools Fluid Screen module being able to recognize, or not recognize a proper fluid container, as well as the fluid bar in The One Probe showing up.

I've added the following screenshots to explain further what I mean.

Screenshot 1: On the floor I have placed all placeable fluid containers that Pyrotech offers. Behind them on the wall, I have creative RFTools screens with fluid modules, each individually linked to a Pyrotech fluid container. The top row is for the blocks in the back row, while the bottom row is for the blocks in the front row.

2023-04-11_21 19 33

https://i.imgur.com/FHxbr2d.png

Moreover, on the left side of my screenshot, you can see the One Probe fluid bar overlay, showing the contents of the soaking pot, as well as the fluid content being showed on its dedicated fluid screen behind it.

Screenshot 2: Compared to the soaking pot, another fluid container like the barrel, does not have the proper TOP fluid bar; instead it shows this separate info bar regarding the fluid contents.

2023-04-11_21 19 42

https://i.imgur.com/fry86wf.png

And the dedicated RFTools fluid screen for the barrel does not show the contents of the barrel at all, despite it also having 1000 mB of water like every other block shown here.

In both screenshots, you can see which Pyrotech do have a properly implemented fluid capability system, and which ones did not.

What You Expect to Happen

That all placed fluid containers register properly with the fluid capability system. So that I can utilize these blocks in my own custom multiblocks that utilizes inputs and outputs based on the Forge capability system.

Script

No scripts used

Crash Log

No errors or crashes or other abnormalities were observed in the log.

Affected Versions

codetaylor commented 1 year ago

Pyrotech fluid blocks use the default fluid capability provided by Forge, just like most everything.

The Pyrotech fluid blocks use different sides for their fluid access and some are only exposed via the configs. For example, IIRC, the barrel uses the top as it's fluid side. Maybe this interferes with how RFTools accesses the capability, I don't know their code. It would need to access the capability passing the correct side and not null for the side.

codetaylor commented 1 year ago

Compared to the soaking pot, another fluid container like the barrel, does not have the proper TOP fluid bar; instead it shows this separate info bar regarding the fluid contents.

Correct. This is because of a custom ToP provider. It has nothing to do with the underlying fluid capability. https://github.com/codetaylor/pyrotech-1.12/blob/master/src/main/java/com/codetaylor/mc/pyrotech/modules/tech/basic/plugin/top/provider/BarrelProvider.java#L73

The barrel requires the automation flag to be set in the config for the fluid capabilities to be exposed: https://github.com/codetaylor/pyrotech-1.12/blob/master/src/main/java/com/codetaylor/mc/pyrotech/modules/tech/basic/tile/TileBarrel.java#L317

Primitive-Human commented 1 year ago

Thanks for the response, I'll try fiddling with the configs to see if that allows me to integrate these into my multiblocks.

Primitive-Human commented 1 year ago

Didn't have to fiddle with the configs too much, not at all really since I had them on default settings.

In the multiblocks with Pyrotech blocks, I had to change the IO side face in the Multiblocked controller so the fluids could be properly inserted or extracted.

One issue persisted though; I can properly use stone/refractory tanks as fluid outputs, but not as fluid inputs. I've tried changing the IO side to top/sides/bottom and none seem to allow pulling fluids for recipes.

2023-04-12_23 42 26

The other fluid IO blocks seem to work for fluid inputs, if I set their IO facing correctly.

Primitive-Human commented 9 months ago

I'm coming back to this issue in combination with #405 because I've been comparing the NBT data on your tanks with those in other mods like Embers Rekindled in an effort to use Hot or Not+ for an extra challenge working with hot liquids.

I'll start with the Embers tank (Vessel), which keeps its content when broken and stores its NBT exactly like the Forge fluid format:

2023-10-12_14 26 19

2023-10-12_14 28 25

In screenshot 1 and 2, the format is {FluidName:"fluidname",Amount:1000}

But in the Pyrotech Stone and Refractory tank, there's much more additional data that doesn't seem to be necessary.

2023-10-12_14 26 12

As a result, the Pyrotech refractory tank does not work with Hot or Not, whereas the Refractory bucket does.

2023-10-12_14 32 17

So, I'll ask if the NBT data on your tank blocks can be culled when broken, so only the relevant NBT data is stored.

codetaylor commented 9 months ago

So, compatibility with the Hot or Not+ mod doesn't have anything to do with the NBT data on the tank.

Hot or Not looks for a fluid capability on the item which the Embers Vessel has and the Pyrotech buckets have.

The Pyrotech tank items do not have a fluid capability.

Today I've added a tank item as well as a fluid capability on that item. This should solve the mod compatibility issue, but I haven't tested it.

As a result of adding the fluid capability, I did clean up the item-serialized NBT to make it easier to implement the fluid capability. Here's what the NBT looks like now:

image

I also found and fixed an issue with multiblock tank fluid settling.

@Primitive-Human If you could please test this interim build for compatibility with Hot or Not+ I would appreciate it. Thank you.

pyrotech-1.12.2-1.6.8-3-g6d20103f.zip

Primitive-Human commented 9 months ago

Thanks for the interim build.

In my quick tests, the refractory tank, and the stone tank after enabling hot fluids, work with HotorNot+ as expected. They also function as buckets now which is great.

The ability to stack tanks got broken however. Empty tanks taken from JEI which have no NBT tag do not connect to each other. All of their "connection" blockstate defaults to "none", regardless of their position in a stack.

And the issue with multiblock tank fluid settling doesn't seem to be fixed by this; the simple multiblock in the screenshot takes water and produces iron ingots with it in the chest; but no matter which input side I select for the stone tank, fluid isn't used and the machine doesn't run. 2023-10-13_10 52 11

codetaylor commented 9 months ago

In my quick tests, the refractory tank, and the stone tank after enabling hot fluids, work with HotorNot+ as expected.

Great!

They also function as buckets now which is great.

They should only function as buckets inside of machines that accept buckets in slots. They should not be able to collect fluids from the world.

The ability to stack tanks got broken however. Empty tanks taken from JEI which have no NBT tag do not connect to each other. All of their "connection" blockstate defaults to "none", regardless of their position in a stack.

You're not wrong. I tested this yesterday and it worked fine. I'm looking at it now.

Yep, I was testing with pre-existing tanks which had NBT. Testing tanks without NBT revealed a flaw in the logic. Fixed.

And the issue with multiblock tank fluid settling doesn't seem to be fixed by this; the simple multiblock in the screenshot takes water and produces iron ingots with it in the chest; but no matter which input side I select for the stone tank, fluid isn't used and the machine doesn't run.

Ok, so fluid settling doesn't have anything to do with the tank's interaction with that mod. Fluid settling refers to fluids from a top tank flowing down and settling into bottom tanks.

I don't know why the tank doesn't interact properly with that mod.

The tanks provide a fluid capability for all sides.

Fixed stacking: pyrotech-1.12.2-1.6.8-4-g1bd3dd8d.zip

codetaylor commented 9 months ago

... but no matter which input side I select for the stone tank, fluid isn't used and the machine doesn't run.

It's possible that the compatibility issue is caused by the tank's fluid capability returning an empty IFluidTankProperties array, as you can see here.

If the mod is interrogating the properties of the fluid capability for retrieval, then it would likely not work.

I've implemented the tank properties properly in the build below.

@Primitive-Human Please test this build with your multiblock. If this doesn't solve the problem, let me know what mod that is (is Multiblocked).

Edit: This is quite possibly the case as it looks like Multiblocked does interrogate the fluid tank properties for recipe input.

Proper implementation of public IFluidTankProperties[] getTankProperties() in tank fluid capability: pyrotech-1.12.2-1.6.8-6-g57d14300.zip

Primitive-Human commented 9 months ago

Back with an update:

Ah, I misunderstood what you meant with settling, but indeed, the fluids moving from top to bottom didn't work in the first interim build. Thanks for the explanation.

I've tested your latest interim build (1.6.8-6) with the testing multiblock and recorded the results. I used free recording software for this, so the frame rate is rather poor, but it gets the point across.

https://github.com/codetaylor/pyrotech-1.12/assets/81650773/5b81fccd-3340-44c9-b507-6585101bb98c

The tanks are recognized as inputs now.

As a larger test, here I used 3 stacks of settling tanks as fluid inputs to create dragon breath from your potion fluids.

https://github.com/codetaylor/pyrotech-1.12/assets/81650773/161c4470-60e7-4dbf-b58b-f19cbd392b29

While I do receive some errors whenever creating this multiblock, I don't believe this is on your part but MBD. If you're interested anyway, here's the error code.

[23:02:09] [Server thread/FATAL] [minecraft/MinecraftServer]: Error executing task
java.util.concurrent.ExecutionException: net.minecraft.util.ReportedException: Exception while updating neighbours
    at java.util.concurrent.FutureTask.report(Unknown Source) ~[?:1.8.0_351]
    at java.util.concurrent.FutureTask.get(Unknown Source) ~[?:1.8.0_351]
    at net.minecraft.util.Util.func_181617_a(SourceFile:47) [h.class:?]
    at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:723) [MinecraftServer.class:?]
    at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:668) [MinecraftServer.class:?]
    at net.minecraft.server.integrated.IntegratedServer.func_71217_p(IntegratedServer.java:185) [chd.class:?]
    at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:526) [MinecraftServer.class:?]
    at java.lang.Thread.run(Unknown Source) [?:1.8.0_351]
Caused by: net.minecraft.util.ReportedException: Exception while updating neighbours
    at net.minecraft.world.World.func_190524_a(World.java:572) ~[amu.class:?]
    at net.minecraft.world.World.func_175685_c(World.java:494) ~[amu.class:?]
    at net.minecraft.world.World.func_175722_b(World.java:440) ~[amu.class:?]
    at net.minecraft.world.World.markAndNotifyBlock(World.java:381) ~[amu.class:?]
    at net.minecraftforge.common.ForgeHooks.onPlaceItemIntoWorld(ForgeHooks.java:954) ~[ForgeHooks.class:14.23.5.2860]
    at net.minecraft.item.ItemStack.func_179546_a(ItemStack.java:186) ~[aip.class:?]
    at net.minecraft.server.management.PlayerInteractionManager.func_187251_a(PlayerInteractionManager.java:481) ~[or.class:?]
    at net.minecraft.network.NetHandlerPlayServer.func_184337_a(NetHandlerPlayServer.java:741) ~[pa.class:?]
    at net.minecraft.network.play.client.CPacketPlayerTryUseItemOnBlock.func_148833_a(SourceFile:55) ~[ma.class:?]
    at net.minecraft.network.play.client.CPacketPlayerTryUseItemOnBlock.func_148833_a(SourceFile:11) ~[ma.class:?]
    at net.minecraft.network.PacketThreadUtil$1.run(SourceFile:13) ~[hv$1.class:?]
    at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) ~[?:1.8.0_351]
    at java.util.concurrent.FutureTask.run(Unknown Source) ~[?:1.8.0_351]
    at net.minecraft.util.Util.func_181617_a(SourceFile:46) ~[h.class:?]
    ... 5 more
Caused by: java.lang.ClassCastException: com.cleanroommc.multiblocked.api.tile.ControllerTileEntity cannot be cast to com.codetaylor.mc.pyrotech.modules.storage.tile.spi.TileTankBase
    at com.codetaylor.mc.pyrotech.modules.storage.tile.spi.TileTankBase.updateTankGroups(TileTankBase.java:203) ~[TileTankBase.class:?]
    at com.codetaylor.mc.pyrotech.modules.storage.block.spi.BlockTankBase.updateTankGroups(BlockTankBase.java:125) ~[BlockTankBase.class:?]
    at com.codetaylor.mc.pyrotech.modules.storage.block.spi.BlockTankBase.func_189540_a(BlockTankBase.java:68) ~[BlockTankBase.class:?]
    at net.minecraft.block.state.BlockStateContainer$StateImplementation.func_189546_a(BlockStateContainer.java:481) ~[awu$a.class:?]
    at net.minecraft.world.World.func_190524_a(World.java:551) ~[amu.class:?]
    at net.minecraft.world.World.func_175685_c(World.java:494) ~[amu.class:?]
    at net.minecraft.world.World.func_175722_b(World.java:440) ~[amu.class:?]
    at net.minecraft.world.World.markAndNotifyBlock(World.java:381) ~[amu.class:?]
    at net.minecraftforge.common.ForgeHooks.onPlaceItemIntoWorld(ForgeHooks.java:954) ~[ForgeHooks.class:14.23.5.2860]
    at net.minecraft.item.ItemStack.func_179546_a(ItemStack.java:186) ~[aip.class:?]
    at net.minecraft.server.management.PlayerInteractionManager.func_187251_a(PlayerInteractionManager.java:481) ~[or.class:?]
    at net.minecraft.network.NetHandlerPlayServer.func_184337_a(NetHandlerPlayServer.java:741) ~[pa.class:?]
    at net.minecraft.network.play.client.CPacketPlayerTryUseItemOnBlock.func_148833_a(SourceFile:55) ~[ma.class:?]
    at net.minecraft.network.play.client.CPacketPlayerTryUseItemOnBlock.func_148833_a(SourceFile:11) ~[ma.class:?]
    at net.minecraft.network.PacketThreadUtil$1.run(SourceFile:13) ~[hv$1.class:?]
    at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) ~[?:1.8.0_351]
    at java.util.concurrent.FutureTask.run(Unknown Source) ~[?:1.8.0_351]
    at net.minecraft.util.Util.func_181617_a(SourceFile:46) ~[h.class:?]
    ... 5 more

I'll go experiment around a bit more, but the main issue of the tanks not being recognized as inputs has been fixed for which I'm grateful. Now I'll try to look why this error is popping up.

codetaylor commented 9 months ago

While I do receive some errors whenever creating this multiblock, I don't believe this is on your part but MBD.

No, that looks like my end.

codetaylor commented 9 months ago

New interim build:

pyrotech-1.12.2-1.6.8-10-g0128add1.zip

Primitive-Human commented 9 months ago

Update using the 3 liquids multiblock:

Error reported earlier has stopped appearing; no new issues occurred.

codetaylor commented 9 months ago

Great! I'll push an update in the next day or two.

codetaylor commented 9 months ago

Fixed in 1.12.2-1.6.9