Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Fluid Hatch fluid handling. #1544

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from
Open

Conversation

draknyte1
Copy link
Collaborator

You shouldn't be able to pull from input hatches.
You shouldn't be able to push into output hatches.

@bartimaeusnek
Copy link
Contributor

I think pulling from input hatches makes sense...

@draknyte1
Copy link
Collaborator Author

It’s like single blocks, no pull.
Shouldn’t be given the flexibility when you advance past them.
Mistakes cost, ask anyone with an assembler full of tin.

@leagris
Copy link
Contributor

leagris commented Nov 15, 2019

@draknyte1 Can you please add an option switch for this?
If I know if I was given the option, I would prefer it off.

@draknyte1
Copy link
Collaborator Author

It’s a bug, not a feature. So no.

@leagris
Copy link
Contributor

leagris commented Nov 15, 2019

Does the plunger works at least to empty an input hatch?

@draknyte1
Copy link
Collaborator Author

Yes, because that’s intended.
GT++ also has a tool for removing the fluid and keeping it.

Same behaviour as single blocks, don’t know why inputs for multis should differ in logic.

@leagris
Copy link
Contributor

leagris commented Nov 15, 2019

Because GregT did not want to implement sided fluid IO, so he had to block extraction of input fluid or you would not be able to extract fluid output without also extracting fluid input.
This is the reason he added a plunger.
But he left the hatches bidirectional because Input and Output hatch are distinct blocks and it would not cause an issue.
So, no it was not a bug but a feature.

@draknyte1
Copy link
Collaborator Author

Because GregT

That was over five years ago.
What Greg says in relation to GT5u doesn’t matter when it’s not his project.

@leagris
Copy link
Contributor

leagris commented Nov 16, 2019

Sorry @draknyte1 I did not intent to hurt or offend in any way. You remind me your courage maintaining such an old project as a lone standing hero. I vow you respect for this. Would you accept some PR Patch to add an option switch to this change. Not that I see no issue with blocking fluid insertion into an output hatch; but an option switch could help preserve existing worlds logistical setups when inserted fluids are extracted after usage. I'd contribute the patch rather than deal with the change, at least until I can feel comfortable or not. Just let me know if I can work on it and relieve you of doing extra work just to please a few people resistant to change like me ^^.

@MauveCloud
Copy link
Collaborator

MauveCloud commented Dec 25, 2019

I might be a bit late to mention this, but as it stands, I think this might break the functionality of several multiblocks, because they use tHatch.fill on the output hatches, and tHatch.drain on the input hatches. Examples here:
https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/src/main/java/gregtech/common/tileentities/machines/multi/GT_MetaTileEntity_DistillationTower.java#L214
https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/src/main/java/gregtech/api/metatileentity/implementations/GT_MetaTileEntity_MultiBlockBase.java#L587

Edit: I tested this myself with a local build, and multiblocks still seem able to use their input hatches, but the output hatches remain empty, even though no pipes or other machines are attached, and that's definitely a bug, since it renders the Distillation Tower, Oil Cracking Unit, Oil Drilling Rig, Large Heat Exchanger, and Fusion Reactor pretty much useless.

FWIW, filling/emptying universal fluid cells (and presumably other fluid containers) via the gui is still possible in both hatch types.

@MauveCloud
Copy link
Collaborator

Actually, thinking about this more, the intent of this PR is a little ambiguous. Do you mean to A. prevent pumping fluid out of input hatches and into output hatches, or B. prevent filling containers in input hatches and emptying them in output hatches? (or both?)

Currently it only does A, with the alarming side effect of preventing multiblocks from outputting fluids, but I'm having trouble thinking of a scenario where a player would want to pipe fluid out of an input hatch or into an output hatch instead of just using an extra pipe piece.

If you meant for it to do B (to prevent the input/output hatches from being used as cheaper alternatives to fluid canners), there are separate functions to override for that (doesFillContainers() and doesEmptyContainers())

@draknyte1
Copy link
Collaborator Author

The intent is stated rather clearly.
Like single blocks, you shouldn’t have the luxury of pumping fluids out of input hatches. Mistakes should cost you plunger usage.
Output hatches shouldn’t be allowed to be filled by any mechanic beyond the multiblock itself.

If I didn’t change the correct code, then it’s due revision. Simple web edit without checking was all I wanted so as to trigger a CI build. I’ve yet to get around to testing it as I’ve been busy.

@StragaSevera
Copy link

Well, this will make using Large Chemical Reaction a pain in the ass. Please, add a config option to disable this behaviour, it's really uncool.

@MauveCloud
Copy link
Collaborator

Are you able to go into more detail of what makes this so frustrating or "uncool"?

As far as "existing worlds logistical setups", this hasn't been merged yet, so there should be time for those setups to be re-designed to account for this.

@MauveCloud
Copy link
Collaborator

MauveCloud commented Oct 19, 2020

Since draknyte1 apparently hasn't done anything further on this, I tried to prepare my own alternate fix by overriding the directional drain and fill methods (as specified by the Forge IFluidHandler interface). However, I was only partially successful - output hatches could no longer be filled from outside sources (while still working in multiblocks, since they call a separate version of the fill method that lacks the ForgeDirection argument), but it did not prevent input hatches from being pumped out, and I haven't figured out why. Edit: turns out I had forgotten about the second version of the drain method that only specifies an amount, but not a fluid type.

@leagris @StragaSevera I will try asking again: could either of you provide more details as to why you want an option to revert this to original behavior? Simply saying it's frustrating, "uncool", or will break existing logistical setups (which are apparently exploiting a bug) is not very convincing. If you can explain why such setups are trying to pull fluids from the input hatches (rather than just from the output hatches), you might have a better chance of changing my mind.

@StragaSevera
Copy link

StragaSevera commented Oct 19, 2020

Well, because in my eyes it is not a bug, but a feature. Can you present some details about why you think it IS a bug?

Firstly, in reality, you can empty any container, if the fluid was not mixed with something else.

Secondly, the setups I can make currently are beautiful and compact, because they do not require to put the powerline to the fluid canner. The setups I will be forced to do to fill or empty capsules will require to power the fluid canner, whis is both an overkill and, in many cases, will require complex pipework or force me to use not Greg pipes, but pipes from other mods.

Therefore, I conclude that it is not a bug, but a feature of hatches.

@MauveCloud
Copy link
Collaborator

I believe draknyte1 already explained why it's a bug - single block machines don't allow extracting the fluid from the internal input tank or inserting fluid into the internal output tank from outside (the fluid canner is a weird exception where the input tank is also an output tank), so why should multiblocks get a special exception?

Also, there might be a small misunderstanding here - my change would have only affected automated pumping from adjacent blocks. As far as emptying and filling capsules, even if that were to be partially restricted (which draknyte1 has not explicitly said is desired), emptying them into an input hatch's tank (via the GUI) would still be allowed, as would filling them from an output hatch's tank. While I'm at it, as far as powering a fluid canner, remember that you can do that with a battery - filling/emptying a 1000L container only takes 16 EU at 1 EU/t with a basic fluid canner.

@MauveCloud
Copy link
Collaborator

@StragaSevera @leagris @bartimaeusnek @draknyte1 I get the impression you're set to only be notified when you're mentioned, not just when issues you've commented on have other comments added. I've made my own PR that doesn't break multiblock fluid production (see #1572 ), but I want to make sure we're all on the same page here as far as what's wanted, so let me break this down into cases:

  1. Manually adding fluid to an input hatch in the gui with cells or other containers: fine. This is a normal usage scenario, so it is still allowed, and I'm guessing draknyte1 is okay with this as well (if not, I hope he can explain why).

  2. Manually extracting fluid from an input hatch with containers: I can understand wanting this when the input hatch is used to collect the output from a single-block machine, rather than being part of a multi-block. Would you be okay with only forbidding this when the input hatch is part of a multi-block machine, given draknyte1's comment about "mistakes should cost plunger usage"? This is currently allowed in all cases, even with my PR.

  3. Manually adding fluid to an output hatch with containers: this doesn't make much sense if the output hatch is part of a multiblock, but does if used to feed a single-block machine, as a less-automatable version of a fluid canner that doesn't require power. This is currently allowed in all cases, even with my PR.

  4. Manually extracting fluid from an output hatch with containers: this is a normal usage scenario, so it is still allowed.

  5. Automatically adding fluid to an input hatch with pipes (or from an adjacent machine's fluid auto-output): normal usage scenario, but should only work through the "input side".

  6. Automatically extracting fluid from an input hatch to an adjacent block with pump covers or similar: this makes no sense to me, and is one of the main things blocked by this PR and mine. If you're objecting to this, please explain why your setup requires this functionality.

  7. Automatically adding fluid to an output hatch from an adjacent block: This is the other thing blocked by this PR and mine, but I can think of one reason for wanting this, which is that with current functionality (i.e. not including this PR or mine) a chain of LV output hatches could function as a non-branching fluid pipe with crazy fast throughput - 16000 L/tick or 320000 L/sec, which is faster than a large high pressure fluid pipe, and it wouldn't have a temperature limit. On the other hand, one could do the same thing with a series of basic fluid canners - I confirmed that their fluid auto-output works even if they have no power.

  8. Automatically extracting fluid from an output hatch from an adjacent block: this is a normal usage scenario, but should only be allowed through the "output side".

@leagris
Copy link
Contributor

leagris commented Oct 26, 2020

The fluid canner, the plunger were all invented to work-around bugs or IC2 features. Don't reverse the truth:

The canner was invented because IC2 made cells single-use, so the normal IC2 API mechanic did not allow re-use of cells. So GregT implemented a fluid canner that does this.

Then GregT created GT5 and did force sided output but unsided input, so he could not allow removing fluid from the input tank with tile to tile interaction fluid transfers. He designed very simple GUI (too simple) and he could not place cell filling and emptying. So he had to make very complex recipes map t deal with cells of fluid and fluid directly, causing multiple recipe duplicates. Then in a blink of questionable motivation, he added the overly complicated plunger to purge input tank whereas wrenching out the machine is always a simpler and less costly option. Because he expected to make the tilemachine NBT data to be persisted on harvest but failed the code. So machine inventory is not saved to the item in harvest and the plunger is made useless.

As you can see, it is all a pile of workarounds, unforeseen consequences, broken implementation and after-thoughts.

So instead of adding more code in the broken direction:

  • Fix single-block machines and GUI to allow free input output of fluids.
  • Fix machines to only have to deal with a single copy of a recipe using fluids regardless of contained or raw fluid.
  • Fix remove the now useless plunger.
  • Fix remove the now useless fluid canner.
  • Fix machines to keep their inventories, power, state on harvest.

@MauveCloud
Copy link
Collaborator

MauveCloud commented Oct 26, 2020

I think you're getting off topic here, so maybe this can be moved to a forum post or something (Edit: see https://forum.industrial-craft.net/thread/16364-gt5u-machine-fluid-handling/ if you want to take me up on this), but here is my understanding of things (which might be wrong in some cases, but if I am "reversing" anything, it is not deliberate):

  1. IC2 cells (without GT) were made from 1/3 of a tin, and intended to be single-use. Universal Fluid Cells are re-usable, but store fluid information in NBT data, which makes them more difficult to use in recipes. Edit: okay, a little research reveals UFCs are fairly new; I've only been playing IC2 since 1.7.10, so I hadn't realized they were that new. However, I'm not entirely certain re-usability was the only reason for changing the behavior and recipe of the other cells.
  2. The primary purpose of input and output hatches are for handling fluid input and ouput in multiblock machines. The ability to use them outside multiblocks is a happy coincidence.
  3. I like the idea of giving single block machines a built-in way to empty and fill fluid containers, with extra gui slots next to their input and output tanks, but I think that might be beyond my skill to add, and it would be a separate feature request.
  4. The fluid canner does have a few recipes that aren't just filling or emptying standard fluid containers - namely filling batteries and coolant cells. Thus it wouldn't be entirely useless after adding the above.

Edit:

Then GregT created GT5 and did force sided output but unsided input, so he could not allow removing fluid from the input tank with tile to tile interaction fluid transfers.

As far as I can tell, the output of single block machines is not "forced sided" - only the auto-output is sided - a pump cover (or something similar from another mod) can be attached on any side to extract fluid from the output tank of a single block machine. Admittedly, the gui isn't as fancy as Thermal Expansion or Ender IO, which allow configuring multiple input/output sides, but I challenge you to show me any other mod that allows "removing fluid from the input tank with tile to tile interaction fluid transfers".

Fix machines to only have to deal with a single copy of a recipe using fluids regardless of contained or raw fluid.

I think there would be a small complication there regarding how many empty fluid containers are returned, though I can think of a couple of ways to deal with that: 1. calculate that automatically in processing, rather than as part of the recipe, which might not even show fluid containers anymore (when viewing in NEI), 2. make recipes that involve multiple input fluids (which as far as I can tell only apply to the chemical reactor) be multiblock only - and add a "primitive" version of the multiblock that can be crafted more easily, but is limited somehow (e.g. slower, only certain recipes) until the resources needed to make the more advanced version become available.

Fix machines to keep their inventories, power, state on harvest.

Power and state I can understand, and fluid inventory at least, but item inventory? That seems a bit less important, since one can easily remove items by hand before harvesting the machine (or just collect the dropped items), unless you're dealing with something like a super buffer or quantum chest, in which case it might be overpowered to allow them to keep their inventory when harvested. Also, it might be a good idea to add a way to clear state to allow stacking of previously-used machines for adding to the processing array.

@StragaSevera
Copy link

I'll try to say what I want, rather what change it would make.

I want to be able to make an automated capsule-emptier or capsule-filler that can be operated from one side, without needing to inject electricity, change battery, do all of this stuff. I'm making a long production chain with 20+ mechs, a mech outputs liquid in a capsule, I want to empty this capsule and push it back to the mech.

There is literally zero reasons why I cannot do this. The fact that some mechs output resources in capsules is just a chore, and you want to make this chore even harder without any gameplay benefit? Do I need to give examples of setups that will become harder because I now need to somehow put the electricity in the canner? I think all people playing this mod can give examples.

@MauveCloud
Copy link
Collaborator

@leagris @StragaSevera Thank you for finally clarifying your usage scenarios. Now I understand better why you insisted this was a feature instead of a bug. I had been thinking of this in terms of using the fluid hatches as part of multiblocks, and based on draknyte1's comments, I think he was as well. I have actually used fluid hatches somewhat like you describe, though I'd forgotten about it until a day or two ago, and I have no problem with keeping that functionality intact.

On the other hand, if you have enough power to run a production chain of 20+ machines like you describe, the extra power to run a fluid canner is a drop in the bucket (pun partially intended) by comparison, so I'm having trouble understanding why you're as upset as you seem to be about that option.

@StragaSevera
Copy link

Yes, I don't care about multiblocks, I care about single block capsule emptier or filler.

And the problem is not energy per se, it's the need to pass energy through the cable, therefore using space that could be used by real machines. It is a chore to pass the electricity cable around in compact schemes to power a fluid canner, and I would prefer if I would still be able to just use the hatch for emptying/filling capsules.

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.

5 participants