Splitting cargo between industries at a single station

[Eldarion]

Hi,

Here's the patch for the feature suggestion made here:
http://www.tt-forums.net/viewtopic.php?f=32&t=58842


The way it works:

1) Find the following ratio for all industries at the station that accept the cargo being unloaded:

Code: Select all

(amount of this cargo already waiting at the industry) / (last month total production of the industry)

2) Divide the unloaded cargo between the industries according to the calculated ratios.
3) If there is an accepting industry that didn't produce anything last month and hasn't got this cargo waiting it will receive one unit of this cargo. This will allow it to start up if there is sufficient supply.

The total production includes all cargos produced by the industry.
The feature can be enabled from Advanced Settings -> Stations -> Cargo Handling.


Issue: for some savegames (but not all) you get an error "Broken savegame - invalid chunk size" when you try to load them with this patch compiled into openttd. Would like help on this. Couldn't find anything useful from google. Does this have something to do with the setting added to Advanced Settings window?

edit: You can load patched savegames with an unpatched openttd.

edit: updated patch file to work with r24265

[HackaLittleBit]

Just some short notes.

Coding style is explained here

Check empty spaces and tabs.

Patches are supplied with extension diff or patch.

I was not able to apply your patch

Use svn.

Check afterload.cpp for compatibility issues.

I use notepad++ to check my patches.

Regards

[Eldarion]

Check empty spaces and tabs.

There was some weird issue with generating the diff file where it wanted to replace loads of empty lines that I didn't touch with another set of just empty lines so I skipped all whitespace differences. This didn't happen with a release prior to r24100 (don't remember the exact number, maybe r24098). Just tell your editor to re-indent and the code should look fine. At least it does in Xcode.

Patches are supplied with extension diff or patch.

Will keep that in mind. I generally don't use any extension for them, but as this forum doesn't allow files without extensions I just added .txt as it is a text file.

I was not able to apply your patch

What errors did it give you? I used GNU diff to create the file and GNU patch to verify it works. Both on unix system (MacOS X).

Use svn.

Do you mean use svn to create the diff file? Will have to look into that.

[HackaLittleBit]

What errors did it give you?

I use tortoise svn.
It simply did not want to digest your patch.

Trunk uses svn

Next serious crash of my system I will start using Git.
SVN GIT HG is personal preference

see bottom of Hirundo's first posting to get a general idea of presenting patches.

I will apply your patch manually now.

Regards

[Eldarion]

Ok, try this. Generated it with svn.

edit: removed patch file. See another post below for an update.

[HackaLittleBit]

Ok managed to apply

Quick look through patch

ln 73 should be uint ret = 0; // space = space
ln 75 for (uint i = 0; i < arrLength; i++) for space bracket and no space after bracket
ln 76 if (arr < arr[ret]) ret = i; for space bracket and no space after bracket

I saw more .


Got compile error

[SRC] Compiling dock_gui.cpp
[SRC] Compiling economy.cpp
d:/development/ottd/vtile/src/economy.cpp: In function 'uint DeliverGoodsToIndus
try(const Station*, CargoID, uint, IndustryID)':
d:/development/ottd/vtile/src/economy.cpp63: error: 'ceil' was not declare
d in this scope
make[1]: *** [economy.o] Error 1
make[1]: Leaving directory `/o/vtile/objs/release'
make: *** [all] Error 1

Checking now what it is
EDIT1: I think is stead of ceil you can use max


Regards

[Eldarion]

The 'ret=0' was a typo. Was just going through the others according to the coding style guidelines. My spacing habits are slightly different as you noticed.

The compile error is interesting. I tried to make sure that all the math functions I used are actually declared/included in the openttd project and not taken from somewhere on my computer, but I guess I failed on the 'ceil' function. It's a standard cmath function, I just didn't want to include a whole library for only one function.

[Alberth]

We don't do floating point math in openttd.
Reason is that floating point is inherently instable between different processor architectures, causing rounding differences between different machines, and thus they are potential desync causes.

[Eldarion]

I've just compiled it both on Mac and on Windows and didn't get any errors. Here's the updated patch with the coding style corrected.

Also another question. When I compile and run on Mac, the version r24111M (or whichever release i'm using) is shown on the title bar. Under windows norev000 is shown instead. On mac the configure script generates the needed rev file that sets the revision number, but how do you do it on windows? I've got very little experience with Visual Studio or compiling under windows.

edit: use the diff file provided in a later post

[FooBar]

You need to have a version control system (e.g. SVN) installed on Windows as well.

[Eldarion]

Ok, that could be the reason.

Regarding the save game issue. I looked through afterload.cpp. There's nothing about invalid chunk size there. The only place is saveload.cpp and there's nothing there that would give me and immediate clue. I don't really see anyway how my patch would alter the savegames apart from the added setting, but that shouldn't mess up only some of the savegames.

[Eldarion]

Does anyone have suggestions on the savegame issue? HackaLittleBit, did you manage to compile in the end? I set up Visual Studio and the project according to OTTD Wiki so we should have similar settings and I didn't have any problems compiling and one of my friends ran it on Win7.

[HackaLittleBit]

I tried to compile for second time.
Same error, ottd does not want to swallow ceil
try finding other solution.
See post of alberth about double.

[Eldarion]

Ok, I re-wrote the code to avoid floating point arithmetics. Try this patch and see what happens. It better work this time. I've spent way too much time on this already. This time i've left the debug calls also in the code.

[HackaLittleBit]

Compiling ok!

Need some time to test however.

I've spent way too much time on this already

Most of us do here.

[Eldarion]

That's good (that it's compiling I mean, not that you spend too much time on OTTD )

While re-writing the code I changed the logic a little bit also to reduce the amount of calculations needed. As it seems most (if not all) cargo packets have 5 or less pieces of cargo in them that by itself is a fairly small amount. Therefore I divided the logic in two. If there are more than 5 pieces in one packet the logic given in post #1 still applies. But if there are 5 or less pieces all of them get immediately delivered to the industry with the lowest ratio as there's no point in dividing 5 units of cargo into even smaller chunks.

[Eldarion]

So what do you think? Is it working alright? Any chance of including it in the trunk?

[Eldarion]

Updated patch to work with r24265.

[Alberth]

A quick glance at your patch was enough to find many code style problems. It might be a good idea to review your code against the OpenTTD code style.

A more worrying problem is that your patch takes loads and loads of time just to deliver cargo to industries, judging by the source code.
Allocating+free-ing dynamic arrays just for a single delivery, looping over the same array at least twice (once for the smallest and once for the biggest value) which can be trivially combined, a super-inefficient way of finding and eliminating maxed out industries, no simple case for delivery to a single industry (which would probably cover about 80-90% of the deliveries).

The code also seems a bit long for what it does, but I cannot put the finger on it why that's the case.

[Eldarion]

Thanks for the feedback. I did review the code earlier on to match the coding style guidelines given in the wiki. Regarding efficiency, I'll have a think about it and try to do something when I find some spare time