[Dominik][0.17.58] Fail after fluid system override. this->fluid.getID() == this->getSystemFluidID() was not true

[boskid]

Steps to reproduce: watch replay.

replay

fluid-mixing-2.zip

(161.77 KiB) Downloaded 141 times

Or reproduce by hand:

1. put two assemblers, one to unload crude oil, one to unload petroleum (any two different fluids will work)
2. put undeground pipes that are internally cut by additional underground pipe - this prevents connecting underground pipes
3. put pumps from crude oil unload and into petroleum load (they are not connected to assemblers because undergrounds are cut)
4. connect pumps with regular pipes
5. remove underground pipe cutters
6. feed crude oil assembler with crude oil barrels - this fills pipes
7. rotate pump in line with petroleum assembler - this forces underground to connect internally with petroleum assembler
8. (optional): /c game.check_consistency() should pass (edit: at this point map is not save-loadable even if consistency passed)
9. feed petroleum assembler with petroleum barrels - this changes whole pipe system to be petroleum
10. /c game.check_consistency() will crash
    log

    Code: Select all

       0.001 2019-07-24 15:18:48; Factorio 0.17.58 (build 46193, win64, steam)

    Code: Select all

    c:\cygwin64\tmp\factorio-build-yuejtk\src\fluid\fluidbox.cpp (599): FluidBox::checkConsistency
    c:\cygwin64\tmp\factorio-build-yuejtk\src\map\map.cpp (2026): Map::checkConsistency
    c:\cygwin64\tmp\factorio-build-yuejtk\src\script\luagamescript.cpp (6050): LuaGameScript::postUpdate
    c:\cygwin64\tmp\factorio-build-yuejtk\src\scenario\scenario.cpp (1118): Scenario::postUpdate
    c:\cygwin64\tmp\factorio-build-yuejtk\src\mainloop.cpp (1153): MainLoop::gameUpdateStep
    c:\cygwin64\tmp\factorio-build-yuejtk\src\mainloop.cpp (1021): MainLoop::gameUpdateLoop
    c:\cygwin64\tmp\factorio-build-yuejtk\src\util\workerthread.cpp (42): WorkerThread::loop

    Code: Select all

     133.825 Error FluidBox.cpp:599: this->fluid.getID() == this->getSystemFluidID() was not true

    factorio-current.log

    (17.17 KiB) Downloaded 174 times

Severity:

1. may be performed on multiplayer server without help of editor (not tested, however no step requires editor)
2. may cause desyncs because before last consistency check, map is not save-loadable. After save-load, petroleum assembler will not receive proper recipe and so joining player will get different game state and there should be desync (not tested, but above save file when loaded will be different from what can be seen at end of replay)

-- edit:
Better reproduction:

1. Open 73600-reproduction.zip (just in case: there is replay)
   save

   73600-reproduction.zip

   (154.64 KiB) Downloaded 154 times

   

   73600-reproduction-overview.png (160.7 KiB) Viewed 3615 times

2. Hover over pump and press "R" two times - this makes right underground pipe to connect internally (after this step, save is not save-loadable because right assembler will loose its recipe on load)
3. Feed right assembler with petroleum barrel - this puts game at inconsistent state
4. /c game.check_consistency() will crash

[boskid]

Confirmed desync (found some empty multiplayer server and tested by performing above actions, disconnecting, connecting and setting recipe for assembler that lost recipe):

desync

desync-report-2019-07-24_16-24-47.zip

(44.03 MiB) Downloaded 224 times

-- edit:
i hope server owner will not be mad at me.. map keeps desyncing when reconnecting and doing something with affected assembler. I cannot connect and fix this issue

--
Factorio alchemistry - how to change full tank of one fluid into another using this bug? It looks that when assembler finishes recipe (with this fluid mixing), it changes type on whole fluid system

73600-fight.gif (2.53 MiB) Viewed 3574 times

[boskid]

[0.17.59], issue still exists

replay + bp

73600-reproduction-0_17_59.zip

(172.58 KiB) Downloaded 153 times

0eNrll9+OoyAUxt+Fa5iIou30VTabBvXUIaNoACfbbfruC3b6Z1oKzGbvtjeN4vc7h3M+TvSA6n6GSQlp0OaARDNKjTY/DkiLTvLe3TP7CdAGCQMDwkjywV1pMyreATFcvqMjRkK28Att6BFHpVxrGOpeyI4MvHkTEkh+g8iPPzECaYQRcEpludhv5TzUoGyMmyR4806E1KCMXcFoGrWVjdLFtijCMNrbv8LSW6GgOa0xl+QdNE+HlmeoB1NcMELuhLRLpHkDbZ7nxpadn57dajDG1kW7ZxQM4wdsZ7vW20Sg3boy2iWjZvDFZumxy7+NveO9BoxOt0/d+YzZqLkFMoqe1Fwp6G3UZpydq2iG0TC2S+sN6YEvKd0a5hMxgVFjD/NAOq6/h7Gm8ZWkvHZ1rm1jlxo8loO+nHv6Uvow1QUDvTWREg0BCarbW5vYOux4A4/U7Bb61X8Y1fNuB2qrxW9wO7v8PMFXyc70u73yMNfJVmH/iVNek6tcJs8Ul0dimct/PQwoDc/bpzOB3rkH29iNWMY4DJPbwX37fNGv43Sahyl05O7i5T7adapOLpNHWnFm+dTsi5qYkXTKGqX1HFl/CXzQMhmap++zSobSdOjqm0ZgfvSDD7xn0JfAOta9aonob95rTFw+F+dZTMwCYppmOr84j4nzgDjR7plXzGJiGohcRsRZQFtFtKG4q4g2VK2YvUJtirkr4I8iZq6AMQvqf49+5u4ifsyLPBWZJyOLRCRNJrJEIvMT7YfB8imxuflowejDvl8sD6yKjK4LmlUVOx7/ANnWQtk=

Copy blueprint

-- edit:
Small question/task list:
- Why rotating pump allowed different systems to merge? Is this intermediate state when first fluid boxes are disconnected - this made undeground pipe to internally connect - then pump fluid boxes were connected and merged without check of same fluids?
- Why consistency check did not catch that?
- How can save-load be made stable in this case as to avoid desync?
- Why recipe when finished was able to change fluid system content type? This seems to be perfect place for assertion. Is this because of assignment-in-assertion-instead-of-equality-check?
- Why fluid system ID on storage tank shows ID==0?

[Dominik]

This simply looks like the pump rotation misses one check. Should be a quick fix. Thanks for catching it. This blocked underground connection is a rather edge case that does not occur very often (what you do is exactly how I test it too) so the bugs take longer to find.
Consistency check does not really catch anything, because it is not normally run. It is quite expensive. It is only run when loading a game with changed data (game version, mods, data) or I think in some cases around multiplayer but I am not sure about that.
Why it changes the fluid - probably because it is just faster that way. This situation should not happen at all so there is no reason to wrap ones head around it.
fluid system # is just an informal id. 0 is totally legit.

[boskid]

Consistency check does not really catch anything, because it is not normally run. It is quite expensive. It is only run when loading a game with changed data (game version, mods, data) or I think in some cases around multiplayer but I am not sure about that.

With consistency check here i mean: after rotating pump, if i execute

Code: Select all

/c game.check_consistency()

(where i know that consistency check is "heavy") it shows that consistency passed, but game is in state that is not stable save-load (one assembler will loose recipe and this is why there is desync).

[Dominik]

So the fix was as simple as I thought (one line) because I have already fixed this bug before for assemblers so all was ready. Bad part was that this bug works with many other entities too so I had to fix them all.
By the way, you don't need to put any fluids in to get the conflict, you just need to connect two different "filters". I use different sides of a boiler.
You are right with the consistency check, I have added it.
Thanks for help!