Transport Tycoon Forums

The place to talk about Transport Tycoon
It is currently Fri May 24, 2013 8:24 am

All times are UTC




Post new topic Reply to topic  [ 20 posts ] 
Author Message
PostPosted: Wed Apr 11, 2012 12:42 am 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
Hi,

Here's the patch for the feature suggestion made here:
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:
(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


Attachments:
cargosplit_patch_r24265.diff [12.58 KiB]
Downloaded 31 times


Last edited by Eldarion on Mon May 21, 2012 11:36 pm, edited 3 times in total.
Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 1:21 am 
Offline
Transport Coordinator
Transport Coordinator
User avatar

Joined: Wed Dec 10, 2008 4:08 pm
Posts: 367
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 :cry:

Use svn.

Check afterload.cpp for compatibility issues.

I use notepad++ to check my patches.

Regards

_________________
¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨ Wiki ¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 11:42 am 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
HackaLittleBit wrote:
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.

HackaLittleBit wrote:
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.

HackaLittleBit wrote:
I was not able to apply your patch :cry:

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).

HackaLittleBit wrote:
Use svn.

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


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 1:07 pm 
Offline
Transport Coordinator
Transport Coordinator
User avatar

Joined: Wed Dec 10, 2008 4:08 pm
Posts: 367
Eldarion wrote:
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 :wink:

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

I will apply your patch manually now. :?

Regards

_________________
¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨ Wiki ¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 2:45 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
Ok, try this. Generated it with svn.

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


Last edited by Eldarion on Wed Apr 11, 2012 8:07 pm, edited 1 time in total.

Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 3:08 pm 
Offline
Transport Coordinator
Transport Coordinator
User avatar

Joined: Wed Dec 10, 2008 4:08 pm
Posts: 367
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[i] < 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.cpp:1094:63: 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

_________________
¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨ Wiki ¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 3:15 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
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.


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 6:18 pm 
Offline
OpenTTD Developer
OpenTTD Developer

Joined: Sun Sep 09, 2007 5:03 am
Posts: 2456
Location: home
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.


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 8:05 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
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


Last edited by Eldarion on Sat Apr 14, 2012 9:34 am, edited 2 times in total.

Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 8:10 pm 
Offline
Tycoon
Tycoon
User avatar

Joined: Mon May 21, 2007 11:47 am
Posts: 6117
Location: The Netherlands
You need to have a version control system (e.g. SVN) installed on Windows as well.

_________________
FooBar's Tram Tracks | TransRapid Track Set | Metro Track Set | OpenGFX base graphics set | FIRS Industry Replacement Set
Dutch Tram Set | Dutch Trainset 2 | Dutch Road Furniture


Top
 Profile  
 
PostPosted: Wed Apr 11, 2012 8:32 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
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.


Top
 Profile  
 
PostPosted: Thu Apr 12, 2012 6:54 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
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.


Top
 Profile  
 
PostPosted: Fri Apr 13, 2012 12:34 am 
Offline
Transport Coordinator
Transport Coordinator
User avatar

Joined: Wed Dec 10, 2008 4:08 pm
Posts: 367
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.

_________________
¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨ Wiki ¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨


Top
 Profile  
 
PostPosted: Fri Apr 13, 2012 8:56 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
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.


Attachments:
cargosplit_r24111_svn.diff [12.74 KiB]
Downloaded 38 times
Top
 Profile  
 
PostPosted: Fri Apr 13, 2012 10:27 pm 
Offline
Transport Coordinator
Transport Coordinator
User avatar

Joined: Wed Dec 10, 2008 4:08 pm
Posts: 367
Compiling ok!

Need some time to test however.

Eldarion wrote:
I've spent way too much time on this already

Most of us do here. :wink:

_________________
¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨ Wiki ¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨¨*:•:.,¸¸,.:•:*¨


Top
 Profile  
 
PostPosted: Sat Apr 14, 2012 9:32 am 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
That's good :) (that it's compiling I mean, not that you spend too much time on OTTD :P)

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.


Top
 Profile  
 
PostPosted: Sun Apr 22, 2012 10:30 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
So what do you think? Is it working alright? Any chance of including it in the trunk?


Top
 Profile  
 
PostPosted: Mon May 21, 2012 11:41 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
Updated patch to work with r24265.


Attachments:
cargosplit_patch_r24265.diff [12.58 KiB]
Downloaded 48 times
Top
 Profile  
 
PostPosted: Fri May 25, 2012 7:13 pm 
Offline
OpenTTD Developer
OpenTTD Developer

Joined: Sun Sep 09, 2007 5:03 am
Posts: 2456
Location: home
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.


Top
 Profile  
 
PostPosted: Sun May 27, 2012 8:43 pm 
Offline
Engineer
Engineer

Joined: Wed Jan 18, 2012 8:56 pm
Posts: 28
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


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 20 posts ] 

All times are UTC


Who is online

Users browsing this forum: Google Feedfetcher and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  

Powered by phpBB © 2000-2013 phpBB Group

Copyright © Owen Rudge/The Transport Tycoon Forums 2001-2013.
Hosted by Zernebok Hosting.