Liquid container branch

Not logged inOpenClonk Forum

Forum Home Help Search Watchlist Register Login

Topic Development / Scenario & Object Development / Liquid container branch

1 2 3

Post

Date 2016-01-30 15:32

[topic branch moved]

By Marky

Date 2016-01-25 22:02

The next thing that I noticed is that there is a "tank" library with a strange interface that does not seem to be in use. I'd like to rewrite it, so that it is more in line with the barrel interface. Since it is a structure library it could offer pipe connections, like the pump, so that you can store liquids in it. Maybe this is not necessary at all, but I did not find out how to pump oil into the steam engine for example. Collecting oil barrels from an oil puddle seems a little strange.

By Maikel Date 2016-01-25 22:29

Good point, maybe we can have the oil material in OC finally together with a test for liquid containers.

By Marky

Date 2016-01-28 22:41

I am working on the test now.

By Marky

Date 2016-01-29 23:40

I got to a point where the barrel stuff is now mostly in a library. See history of this branch. Could someone please have a look at this? For example I have no strong opinion on which functions in the library should be runtime-overloadable.
The next step is offering the drain and source menu interactions from the pump in Library_Tank. I see the pump as a "tank", too, but it immediately transfers the stored liquid with its timer function.

By Maikel Date 2016-01-30 15:06

It is hard to get an overview with all those squashed commits, the branch has a strange name. I'd suggest to rename the branch to liquid-container.

Then the tests fail and give some errors for me. Otherwise it looks good, I'd suggest to move all liquid related libraries into the folder Libraries.ocd/LiquidControl.ocd.

Maybe rename SetLiquidName to SetLiquidType. Also seeing the pump as an "immediate" tank might lead to problems, are you sure this is the right way.

What I'd like to see at the end is: oil field->pipe->pump->pipe->steamengine =>power.

By Marky

Date 2016-01-31 22:32

The commits are actually not squashed yet. I like commits to be small, so that the changes are easily reviewable. The commits marked with "Squash:" are to be squashed yet. I'll rename the branch and functions as you suggested.

By Maikel Date 2016-01-30 15:32

This topic actually deserves its own thread. Also what do you think about having concrete stored in barrels as well? Can the library handle that?

By Marky

Date 2016-01-31 22:26

It should be able to handle that, because the material is stored as a string, so it can hold any liquid or material that you can think of, actually.

By Sven2

Date 2016-01-31 22:55

In Libraries.ocd/LiquidContainer.ocd/Script.c, the call to Construction should call _inherited(...) because objects may include other libraries that also need the callback.

By Marky

Date 2016-02-03 22:28

I got to the point where connecting pipes to pumps and steam engines is possible. Now the awkward thing is the following: If I connect to the steam engine first, then the pipe is green, because it counts as a source pipe for the steam engine. If I connect it to a pump, then the pump will have two green lines, because the thing that goes to the steam engine is a drain for the pump.

As a solution I always color the line from the steam engine red. However, what about connecting pumps in line (this would be stupid though, because with the current situation they all need power)? Or other connections between containers and pumps. Maybe we should have the first part of the line be red, then green where it reaches the other object, and vice versa?

By Maikel Date 2016-02-03 22:43

I think we should try to avoid complication here. Don't make the pump a liquid container. Only allow connections from the pump to a liquid container.

Or what do you want to achieve? I imagine a use-case where the pump pumps oil into the steam engine.

By Marky

Date 2016-02-03 22:48 Edited 2016-02-03 22:57

I did not have a specific use-case in mind. Of course, pump to steam-engine is a use case. I did not want to limit the possible connections, but it probably makes sense. I wanted to be able to connect any liquid containers to each other with pipes. I will still need that code for the classic project, but I will simplify it as you suggested for the normal game.

By Marky

Date 2016-02-04 22:56

The pump can pump stuff to liquid containers now. Can you explain why you were against making it a liquid container? At the moment the pump and the container share nearly the same functions, and both store liquid.

By Zapper

Date 2016-02-04 23:00

His intention probably was that players can not pump stuff into a second pump

By Marky

Date 2016-02-04 23:04

This can still be prohibited by not letting the pipe connect to a second pump.

By Zapper

Date 2016-02-04 23:38

Well, yeah. I don't know how your stuff works internally. I was just saying that Maikel probably didn't mean "Don't make the pump inherit from Libraries_LiquidContainer" but rather "Don't make pumps be able to connect to other pumps".

PS: And I'd find it weird if pumps actually contained liquid

By Marky

Date 2016-02-05 18:38

Actually in the master branch implementation the pipe can contain liquid - that happens when the drain clogs and you run out of energy to continue pumping for example.

By Zapper

Date 2016-02-05 19:39

What was the reason that the pump does not simply "teleport" liquid without storing it anymore?

By Sven2

Date 2016-02-05 22:05

I don't know what the original reasons were. But the problem I see with direct pumping is that ExtractMaterial returns the extracted material and has then already done its job. I.e. the liquid is removed from the input. If the following InsertMaterial fails, there's no reliable way to put it back.

So if you wanted to avoid the storage, you would have to do an extra check before pumping, which makes the script more complicated.

Also, I think it's not totally unrealistic for the pump to store a few pixels of liquid internally as being "in the pump".

By Zapper

Date 2016-02-05 22:08

Ah, okay. That makes sense

By Marky

Date 2016-02-08 21:06

I am done with the liquid container branch now. Is force push supported in the repository? If so, I'd push the branch to the repository and rebase it on the head later. Anyway, my local version can be found here.

The branch has the following stuff:
- you can connect pipes to the pump and/or steam engine
- there is oil
- the pump delivers oil to the steam engine, which burns it
- the steam engine can also be fueled with oil barrels
- tests for the connection logic and liquid container interface are in Tests.ocf\LiquidContainer.ocs, added two more tests in PowerSystem.ocs

Could someone please test the changes in actual gameplay? Furthermore, oil does not yet burn.

By Maikel Date 2016-02-08 21:18

I will test in the coming days, for now you can leave it on github, only a few people have force push rights iirc.

By Marky

Date 2016-02-08 21:32

Thanks! There may be encoding errors in the StringTbl.. files.

By Zapper

Date 2016-02-08 21:34

>Is force push supported in the repository?

What? Why would you want to do that.

By Marky

Date 2016-02-08 21:37

For example, I have my branch, push it to my forked repository. Then I pull, rebase my local branch on top of the repository, and force-push the branch to my forked remote repository again. At least that is what I do with single commits.

By Zapper

Date 2016-02-08 21:49

Then you should do that locally and not in the remote repository and push only the rebased history.
But as far as I am concerned, keeping the branch history won't hurt.

By Maikel Date 2016-02-08 21:58

I think it is best to keep it on github and rebase on master in regular intervals. This makes merging later a lot easier. This also makes switching branches easier as I don't have to recompile every time.

Some fast remarks:
* Don't comment out lines just replace them.
* The last test in the power system is an infinite loop, so your tests should go before.
* Remove the flagpole in test 21.
* SetProperty("Value", 10); -> this.Value = 10;
* Script warning (needs to be failsafe):
[21:55:39] WARNING: Translate: no translation for string "MaterialLava"
[21:55:39] by: Object(31)->UpdateLiquidContainer() (/home/maikel/openclonk/repos/build/planet/Objects.ocd/Items.ocd/Tools.ocd/Barrel.ocd/Script.c:121)
[21:55:39] by: Object(31)->ChangeLiquidFillLevel(100) (/home/maikel/openclonk/repos/build/planet/Objects.ocd/Libraries.ocd/LiquidContainer.ocd/Script.c:99)
[21:55:39] by: Object(31)->SetLiquidFillLevel(100) (/home/maikel/openclonk/repos/build/planet/Objects.ocd/Libraries.ocd/LiquidContainer.ocd/Script.c:86)
[21:55:39] by: Object(31)->SetLiquidContainer("Lava",100) (/home/maikel/openclonk/repos/build/planet/Objects.ocd/Libraries.ocd/LiquidContainer.ocd/Script.c:190)
[21:55:39] by: Global->Test8_Execute() (/home/maikel/openclonk/repos/planet/Tests.ocf/LiquidContainer.ocs/Script.c:410)
[21:55:39] by: FxIntTestControlTimer(0,{Name = "IntTestControl", launched = false, plr = 0, result = 1, testnr = 8}) (/home/maikel/openclonk/repos/planet/Tests.ocf/LiquidContainer.ocs/Script.c:115)
* We need an intuitive interface where you can see how much liquid is held in a tank.
* What is the difference between the tank and liquid container library?
* What kind of other unit tests do you have in mind?

Otherwise it looks very good and promising so far (at least the power tests).

By Marky

Date 2016-02-08 22:16

* The warnings in the unit test probably cannot be avoided, maybe by overloading the barrel for the test or not translating the string if the barrel does not support the liquid: I fill lava into it, it is looking for the string "MaterialLava" in its StringTblXX.txt, but obviously it cannot find that, because the barrel cannot carry lava. How else can I test a liquid that the barrel is not supposed to hold?
* The commented lines have been removed in a later commit
* I did not know about the infinite loop. Why do you want to design it that way? I thought the test has one purpose: proove that you can get a working power network with a pump that generates energy by pumping downward, one that consumes energy by pumping upward, and a third source that provides the power difference. I tried to make the test such that it is not an infinite loop, but still tests that aspect.
* The SetProperty-stuff is some kind of relic from the barrel. The whole value calculation there is seriously broken in my opinion, but it does not matter because the barrel cannot be sold anyway.
* Difference between tank and container: The container can be anything that can hold liquids, such as a barrel, bottle. The tank is a structure that can hold liquids (as such it inherits from the liquid container), but it also allows to connect pipes to it, by using the pipe or in the interaction menu.
* As for the interface: Do you mean some kind of GUI component?
* Other unit tests: No idea yet. Maybe have the pump transfer liquids with different pipe connections, but that is already handled in the liquid container unit test (probably not all of the combinations though).

By Sven2

Date 2016-02-08 22:59 Edited 2016-02-08 23:07

> * The warnings in the unit test probably cannot be avoided, maybe by overloading the barrel for the test or not translating the string if the barrel does not support the liquid: I fill lava into it, it is looking for the string "MaterialLava" in its StringTblXX.txt, but obviously it cannot find that, because the barrel cannot carry lava. How else can I test a liquid that the barrel is not supposed to hold?

For example, you can just make them locals:

local BarrelNames = {
  Lava = "$LavaBarrel$",
  Water = "$WaterBarrel$",
};

and then, in a function:

func GetNameForBarrel(string material)
{
  var name = BarrelNames[material]; // Name known?
  if (!name) name = "$FilledBarrel$" // Otherwise, return a standard name
  return name;
}

Of course we could also add a failsafe-parameter for translate. But I don't like that too much, because it makes code paths dependent on the loaded language pack.

By Maikel Date 2016-02-08 23:09

>* I did not know about the infinite loop. Why do you want to design it that way? I thought the test has one purpose: proove that you can get a working power network with a pump that generates energy by pumping downward, one that consumes energy by pumping upward, and a third source that provides the power difference. I tried to make the test such that it is not an infinite loop, but still tests that aspect.

That is okay for now, I'll change stuff afterwards if I want something different.

>* As for the interface: Do you mean some kind of GUI component?

Yes, how do I know how many oil my steam engine hold, and how many it can hold.

By Maikel Date 2016-02-08 22:03

Next error when testing in the playground:

[22:02:11] ERROR: '->': no function "GetBarrelMaterial" in object "Object(5136)".
[22:02:11] by: Object(5136)->CanBeStackedWith(Object(5128)) (/home/maikel/openclonk/repos/build/planet/Objects.ocd/Items.ocd/Tools.ocd/Barrel.ocd/Script.c:205)
[22:02:11] by: Object(5146)->FxIntRefreshContentsMenuTimer(Object(5146),{Name = "IntRefreshContentsMenu", last_inventory = [], menu_index = 0, obj = Object(5160), slot = 0},1) (/home/maikel/openclonk/repos/build/planet/Objects.ocd/HUD.ocd/ObjectInteractionMenu.ocd/Script.c:1004)

By Marky

Date 2016-02-14 18:26

This was fixed, thanks to Sven2's suggestion. Can we merge this into the master branch? Maybe after the bugfix release.

By Maikel Date 2016-02-14 20:13

I want to have a more detailed look at it before. Can you give a link again to the most actual branch at your github?

By Marky

Date 2016-02-14 22:23

I do not have an interface for displaying the stored material yet. Maybe I'll use Zapper's suggestion and have it contain an actual liquid object, that has the potential to simplify things a little bit.

By Sven2

Date 2016-02-14 22:32

It could be one object per material type, which has the amount of material as a property (like "stack count" for e.g. arrows). That way we get the simplicity of having objects that you can e.g. search for when looking for construction materials and also give us proper translation strings, etc.. But we still don't create thousands of objects if you fill a power plant with a screen full of oil.

By Marky

Date 2016-02-16 21:39

The easiest way so far was including the stackable library, but it seems not to be that good of a solution in the long run. Your proposal appears to be better. A makeshift version can be found here: https://github.com/gitMarky/openclonk/tree/liquid_container

This will not work as construction material, though. My suggestions from there on:
- make liquid object save the contained volume
- make liquid object enter liquid containers only
- make steam engine a liquid container for the "fuel" liquid/material that already exists, instead of an oil container, and have it convert oil to fuel at a 1:1 ratio
- modify behavior of steam engine so that liquids are pumped into it as long as the fuel count is below a certain value, so the pump works as long as the steam engine is burning fuel
? modify barrels so that they have an extra inventory slot (where the liquid object will be displayed)
? make liquid objects C4D_Object instead of C4D_StaticBack and have an actual graphics, not just a picture
? make liquid objects convert to respective material if they are not contained in a liquid container, or particles in case of non-material liquids (fuel)

By Zapper

Date 2016-02-08 22:10

How do you imagine production to work (loam e.g.)?

I am really still playing with the idea of a "water" object that would just be contained in the objects :x

By Sven2

Date 2016-02-08 23:15

GetBarrelIntakeY is defined but is not used? I think the constant value 3 is used instead.

Also I think it should be a property instead of a callback (because it's called on a timer and it would also be consistent with ApertureOffset), but I have no strong opinion about this.

By Maikel Date 2016-02-10 22:51 Edited 2016-02-10 22:53

I don't know if this has been already discussed, but I imagine the following use case:

* We have a new structure, the tank, which can contain liquids of a certain type and only that type.
* The pumps source is in some liquid and the drain connected to the tank, the pump fills the tank.

So now when the tank is full the pump stops working presumably. Questions:

* Can I connect multiple tanks to one pump? How do we determine which one will be filled.
* Can I connect multiple tanks to each other (enhancing the total capacity)? [My opinion: don't allow this]
* What happens if the pump tries to pump the wrong material?
* Can I do Pump->Tank->Pump->Steam Engine, or Pump->Tank->Steam Engine (arrows indicate liquid flow)?

You see there are lots of design issues to resolve if we complicate matters. For now though I propose to add a Tank structure with place holder graphics to which the pump directly connects. This can then be filled and can store barrels which can be filled.

Edit: Sorry for being so negative/precise/obnoxious here, but I know from the power system that we should be really careful about the design and test thoroughly before adding it to master. I appreciate the effort you put in here!

By Marky

Date 2016-02-10 23:04

At the moment the objects save only one pipe per connection type: source / drain / neutral
The pump can have source and drain, but not neutral; the steam engine can only have neutral (a connected drain from the pump counts as neutral in this case).

So, in the current implementation:

>* Can I connect multiple tanks to one pump? How do we determine which one will be filled.

No.

>* Can I connect multiple tanks to each other (enhancing the total capacity)? [My opinion: don't allow this]

No.

>* What happens if the pump tries to pump the wrong material?

Its the same as if the pump were clogged.

>* Can I do Pump->Tank->Pump->Steam Engine, or Pump->Tank->Steam Engine (arrows indicate liquid flow)?

If you designed a tank object that can have an input and output pipe, then you could have the first version, but not the second (since the tank does not know how to transfer liquid to another object, and the steam engine does not know how to request liquid from another object. Only the pump can transfer stuff.

By Maikel Date 2016-02-10 23:14

That sound simple and consistent. One question, the pump can pump liquids out of the steam engine?

By Marky

Date 2016-02-10 23:18

No, because you can only connect a drain pipe to it. What still bugs me is the thing that the pump quickly fills the steam engine (I made it so that only one barrel worth of oil is contained, so that the pump does not pour (then unretrievable) oil into the steam engine without reason) and then has a slow pumping animation. I'd rather have it insert material and the engine takes some small amount out again as it is working.

By Sven2

Date 2016-02-10 23:53

> * We have a new structure, the tank, which can contain liquids of a certain type and only that type.

Would that magically be able to compress all liquids? Because otherwise I don't see a difference between a liquid tank and just using an area in the landscape that you have prepared with wall kits, etc. Also, it would have to be a huge structure!

Or would it be for oil only, the idea being that it provides protection from enemy firestones?

By Luchs

Date 2016-02-10 23:58

>Also, it would have to be a huge structure!

It would obviously be huge towards the back of the screen.

By Marky

Date 2016-02-11 20:38

We do not have that structure actually, I just used a library with that name, and it existed before I started doing this. So far I see no purpose for a new specific tank structure. However, the pump and the steam engine use the (new) interface from that library for connecting pipes now.

By Marky

Date 2016-02-20 21:20

Added the new logic with liquid items to the branch. Unfortunately it is a little more convoluted now than before. The code can be found here: https://github.com/gitMarky/openclonk/tree/liquid_container

I'll take a few days off from doing this, to get a clear view on it.

By Marky

Date 2016-02-28 22:16

I got the liquid items to work, but in my opinion they are a step back. Liquid containers now contain a liquid object (water, oil, laval acid, fuel, as well as a generic renamable liquid). This liquid can enter liquid containers only, and disperses when not inside a liquid container for 1 frame. Dispersion means: The object gets removed, it can spray material pixels or particles equivalent to the liquid amount, though.
This has advantages and drawbacks though:
+ liquid objects are one object, similar to a "stackable" object
+ the barrel does not need an implementation for emptying, it just ejects the liquid object which then sprays its contents
+ the steam engine can contain "fuel" now, displays the amount of fuel as contained liquid object "fuel"
- the code of the liquid container is not as intuitive anymore, because it has to contain code for handling a liquid object, instead of handling liquid type and volume: The liquid object contains that information, and the container has to save that object and delegates calls to that object. However, the logic for creating new objects and partially transferring liquid from one liquid object to the container is convoluted, even though I tried to keep it as straight forward as possible
- the producer library does not benefit from the liquid objects: a liquid container can have only one type of liquid, so you have several options: a) allow several liquids in a producer (one could limit this, but why would you put 100x water and 50x lava in a bakery for example. This is the same as putting water or lava barrels into it), b) allow one type of liquid per producer only (which seriously limits producers that produce objects that rely on multiple liquids, such as oil and/or water)
- the producer library does not benefit from the liquid objects: only the steam engine can have a pipe connection at the moment.
- the producer library does not benefit from the liquid objects: it has to distinguish between liquid containers ("IsLiquidContainer" - barrels, can partially remove liquid from it), "real" liquid objects ("IsLiquid" - water, liquid can be partially removed), and "pseudo" liquid objects ("IsLiquid" - ice, does not really contain liquid and gets removed entirely)

Would be nice to get some feedback on this. The latest version is available on my fork, branch "liquid_container"

By Zapper

Date 2016-02-28 23:01

>liquid objects are one object, similar to a "stackable" object

Yes, they should use the stackable-interface then.

>a liquid container can have only one type of liquid, so you have several options

At the moment, producers can take only the objects they need for production (you can't put loam into a chemical lab). I guess that would be the same there? (And wouldn't need any changes?)

> the producer library does not benefit from the liquid objects: it has to distinguish between liquid containers ("IsLiquidContainer" - barrels, can partially remove liquid from it), "real" liquid objects ("IsLiquid" - water, liquid can be partially removed), and "pseudo" liquid objects ("IsLiquid" - ice, does not really contain liquid and gets removed entirely)

Why does the producer library need new code? It just contains the liquids, right? So it should work if they are just IsStackable objects. And if the producer does not work with IsStackable right now, it should be made to work - because we have many exceptions everywhere for the stackable stuff anyway.

Using the stackable interface for liquids should reduce the required code quite a bit. Or did I misunderstand anything?

By Marky

Date 2016-02-29 05:49

>Yes, they should use the stackable-interface then.

That's what they did, but it prooved to be not that useful: Objects that include Library_Stackable can be stack up to 999 objects. For liquids that is not enough, because one barrel contains 300 items already, and I do not want to have 3x999xWater in a container. Having another "non-limited, but not infinite" mode for stackable objects seemed to be strange, too.

>At the moment, producers can take only the objects they need for production (you can't put loam into a chemical lab). I guess that would be the same there? (And wouldn't need any changes?)

Lets say the chemical lab needs water for one product, and oil for another product. You can put oil into it, but then you would not be able to put any water into it (as a reserve) until the oil is used up. Once could make an exception, so that producers can contain any amount of liquids at the same time, but that would effectively make all producers a liquid storage that can hold e.g. 100+ barrels worth of water each. On the other hand, one could actually put 100+ barrels inside the producer at the moment, so no actual change.

>Or did I misunderstand anything?

There are three kinds object objects for liquid at the moment:
* Liquid containers (Barrel), marked by "IsLiquidContainer". These contain liquid and should not be removed by the producer, just emptied.
* Liquid objects (Liquid_Water), has a function "IsLiquid" that returns water. These also "contain/are" liquid, and should not be removed by the producer, just emptied. I.e. if there is 90 water in the producer and it needs 60, then 30 should remain.
* Other objects (Ice) that also have a function "IsLiquid", but the producer just counts how many water is contained in such an item and removes the entire object.

So in the end I need to distinguish these items, or make them use the same interface. This is somewhat hindered by the fact that the producer handles fuel and liquid completely different from normal components. If that were not the case, then the code might become less convoluted indeed.

Still the actual questions that we must ask are these:
- what is the benefit of having, say 100 water items in the kitchen, instead of a barrel that contains 100 water?
- would barrel empty into the kitchen if you transfer a water barrel to the kitchen, leaving the empty barrel in the Clonk?
- if not, then why include the water item? It would only ever enter the kitchen if there were a pipe attached to it, which is not possible at the moment.

Topic Development / Scenario & Object Development / Liquid container branch

1 2 3

Post