[Dominik] [Mod related] fluid mixing assertion failure triggered by upgrade of Assembler Fluid Passthrough mod.

[slippycheeze]

I updated the Assembler Fluid Passthrough mod today, and a map that was run with the older version now triggers an assertion failure:
81.048 Error FluidBox.cpp:599: this->fluid.getID() == this->getSystemFluidID() was not true

This is pretty clearly a "halt and catch fire" situation: something in the mod update causes the map to mix fluids, where it did not previously, and the game aborts when faced with an invalid situation. Please find attached the map and log file.

Note: you can toggle on and off the Assembler Fluid Passthrough mod; turning it off causes the map to work, and back on will trigger it again reproducibly for me. So, whatever the issue is, it isn't an upgrade, but rather a "this situation is now broken" problem.

I have not reduced the set of mods involved, but my prediction is that you need two of them only: Assembler Fluid Passthrough, and Geothermal. The later supplies the "geothermal heat exchanger" entity that I'm pretty sure (for lack of any competition for the role) is the machine that ends up with intermixed fluids. They are in 4 rows of 10 tightly packed - in contact - machines, and have both water and "geothermally heated water" in them.

You are unlikely to actually run the fluids without the advanced pipe mod, though, because ... well, that is what they run through from their source.


Anyway, like I said, this is clearly a "halt and catch fire" situation: the fluid system asserts because fluids that should never mix, do, and the change to the mod is what triggers that mixing by changing something about how fluid systems connect such that, after the upgrade, they do cross-connect invalid fluids.

Reported on advice here, and I'll be pointing the mod author to this post also.

[EnigmaticAussie]

I have not reduced the set of mods involved, but my prediction is that you need two of them only: Assembler Fluid Passthrough, and Geothermal.

I believe the offending machine is actually part of the Space Exploration pack.

I am investigating this.

[EnigmaticAussie]

I have not reduced the set of mods involved, but my prediction is that you need two of them only: Assembler Fluid Passthrough, and Geothermal.

I believe the offending machine is actually part of the Space Exploration pack.

I am investigating this.

I was wrong - you were right.

I've added compatibility for geothermal in version 0.0.8.

Edit: I would be happy for this to be closed as not-a-bug, but rather a bad edge case of Assembler-Pipe-Passthrough and some updates I've done to it recently.
The game doesn't like it too much if you add a fluidboxe to the same position as an already full one.

[Dominik]

You have by far broken the record for the most modded save I ever got in a bug report. 177 mods.

I will check it out. But tell me this thing.
Do you think I should leave it as is, because the assert does not bother a player but instead is useful to find a bug in the mod
or
I should add some migration that would remove the conflict on loading?

[Dominik]

Actually I might not be able to check it out because the mods are not compatible now because of beam graphics and I have no idea what I can disable while still retaining the bug....

[EnigmaticAussie]

Minimal mods required (I think):

Assembler-Pipe-Passthrough_0.0.5
Geothermal
PickerPipeTools

It will break most things play wise, but should give you a crash out.

As far as what to do with it...
I guess it shouldn't be possible to have multiple input/output/input-output fluidbox connections, in the same position, at data stage.

[slippycheeze]

You have by far broken the record for the most modded save I ever got in a bug report. 177 mods.

I will check it out. But tell me this thing.
Do you think I should leave it as is, because the assert does not bother a player but instead is useful to find a bug in the mod
or
I should add some migration that would remove the conflict on loading?

As a player my view is: the assertion is great, and absolutely fine. The mod change highlighted a problem, and I'd rather you crash out than corrupt my save, or something horrible. (...and I have good backups. most players will have none, and a corrupt save will be the end the save for them.)

The error reporting mechanism is not. I got a crash-to-desktop with the crash dialog *under* the game. It looked like the game had frozen, I tried to revive it, and it was only through the "preview all the windows" feature of the taskbar that I discovered the assertion dialog existed.

Instead, if possible, make the assertion more like a lua scripting violation. don't crash to desktop, but do abort the save and return to main menu, with an error dialog. Send stuff in the background, or ask about sending, or just tell the user what to do. ("post to forums at ${url} and attach ${files} from ${directory}")

Minimal mods required (I think):
Assembler-Pipe-Passthrough_0.0.5
Geothermal
PickerPipeTools
It will break most things play wise, but should give you a crash out.

Can't fully confirm, but the fluid runs through [url=https://mods.factorio.com/mod/underground-pipe-pack]Advanced Fluid Handling[url] pipes underground. All they do is connect "water" and "geothermally heated water" ("with uranium byproduct" in this case, but I doubt that matters) to the "geothermal heat exchangers"

I'm pretty sure you don't need the picker pipe tools - though you may for clamped pipes - and can substitute the input paths from the AFH undergrounds to regular pipes and a fluid source.

Packing two or more geothermal heat exchangers next to each other per the save is probably necessary. I doubt much else is.

I'm saying that because the geothermal heat exchangers are the only assemblers of any description on the map, full stop. This crash came up before I got to building out anything else than the geothermal power infrastructure.

[Dominik]

The assembler passthrough mod is not on the save's mod list. So what exactly should I do with it?

[EnigmaticAussie]

It was on the list before the save crashed and cheeze overwrote it.

But all you should need to do is add it, and load the save.
Make sure you have version 0.0.5.

I'll see if I can put together a quick test mod to replicate the issue without all the bloat.

[EnigmaticAussie]

Not quite the crash to desktop that we had before.
So there's something else at play.

Load the save, you'll see that the assembler on the left makes express belts.
Quit.
Enable EA-test-mod.
Load save, the fluidboxes get extra pipe_connections and the lube pipeline turns into heavy oil.

Like I say, it's not *quite* what's happened for Cheeze, but this also shouldn't happen.

[Dominik]

Thanks for the effort and help.
What is it exactly that is happening there, what does the mod do? I see some overlapping connections and weird stuff
I see that the behavior is correct, given the circumstances. I suppose that heavy fluid from the assemblers is connected to the pipe full of lube. The lube is cleared from the pipe on migration as that is the easiest solution to fluid mixing. Then it gets filled by the heavy fluid once the game starts from the assemblers.

I have run the original save with
Assembler-Pipe-Passthrough_0.0.5 (with and without)
Geothermal
PickerPipeTools
Underground pipe pack
and still don't see anything weird. Saved it with paththgourh, loaded without, and still ok, other way too.
So what else can I do?

[EnigmaticAussie]

sigh

This will be partially my fault.
Assembler-Pipe-Passthrough should be bersion 0.0.6.

Attached is a minimal save, and the correct version of APP.

Load the save without APP, everything is ok.
Enable APP_0.0.6
Load save
CTD.
???
Profit!

[Dominik]

Thank you, now I have got the crash and fluid mixing. Now the fun starts...
I won't be here next couple days so if I don't fix it today, it will take longer. Sorry for that.

[Dominik]

Now I see what you did there. Output fluidbox with 9 connections, up to 3 on one spot. Altogether 14 connections on one assembler. At the worst place, 5 connections and 3 different resources. I would expect issues there
That of course causes fluid mixing. The migration catches that, tries to fix it by removing the assembler recipe - which until now always worked - but in this case that causes new fluid mixing as the connection to the assembler from the outside reconnect to pretty much random connections of the many available.

I don't think there is a generic solution for all such cases, considering all the things that mods are allowed to do. But for this case I think I will just prevent having any overlapping connections within one entity as a check in the prototype, which would give a warning into the log if that happens. But I will only get to the next week.
I also see that the automatic reports of the bug stopped coming once you have updated the mod so it is not urgent.

[EnigmaticAussie]

But for this case I think I will just prevent having any overlapping connections within one entity as a check in the prototype, which would give a warning into the log if that happens.

I assume this means that it won't crash out, but rather just have a

Code: Select all

Warning ModManager.cpp: ... multiple connections at the same position ... blah blah

?

One thing I tried to do, was to figure out if and when the connections would overlap, however, that requires testing every single recipe that has fluids as input and/or output.

Normally this recipe change would mean all 3 outputs would be petroleum gas, but we added a new feature so that a recipe can specify a specific fluidbox to use.

On that note, with the proposed changes to the way recipes can "set an output" fluidbox would that resolve some of these tests/issues?

[Dominik]

Normally with assemblers/chemical plants the inputs are opposite of the outputs so your extension would always create the overlap. But then with refinery it is different... I think that unless you can read the connection layout and create the new connections into empty spots, the whole thing is not going to work. I don't do modding so I can't say how hard that is. Neither I know how the new changes will affect this.
Yes, I mean warnings like that - that is the best I can do, aside letting it to crash - which people here say should not be allowed to happen.

[EnigmaticAussie]

I don't mind the hard crash - but I can understand the logic behind not letting it happen.
Especially seeing as essentially it's a mod problem, not a core problem.

I'll have a think about some possible implementations of fixes for this and get back to you.

[Dominik]

You might want to wait with a solution as there might be some changes to how recipes configure fluidboxes.

[Dominik]

I have added the check, so creating a fluidbox overlap within a crafter should be no more possible (will throw an exception).