The International Simutrans Forum

 

Author Topic: Meaning of brown pile display of the factory info window  (Read 346 times)

0 Members and 1 Guest are viewing this topic.

Offline Ranran

  • Devotee
  • *
  • Posts: 1282
  • Languages: ja
Meaning of brown pile display of the factory info window
« on: June 14, 2020, 05:29:51 AM »
I'm wondering how to handle this display in the new GUI.  ???
I haven't changed this decision function is_active_lieferziel(), but it behaves differently than I expected.

What I expect is if the two factories are properly connected by a line or convoy with the correct goods category.
Actually it doesn't seem to be.

People playing on the server will often see this route disappearing even though the route exists.
For that I checked if it was connected many times. And the player finds this pile untrustworthy.  ::-\


Examples of other abnormalities that can be confirmed
Open this saved game
1) Check out the bakery near Sparrowpike Central Stop. It shows a mountain that is connected to the windmill, but only the passenger line runs at that bus stop.  ???
I have never set up a freight line at that bus stop.


2) Check out the colliery and coal marchant connected by rail. This is the trap I used to fall in.  :o
Although there is a cargo attribute, the cargo is not carried because the capacity is 0. But it is fooled by the brown pile. (´・ω・`)


What does this brown pile represent?

Offline jamespetts

  • Simutrans-Extended project coordinator
  • Administrator
  • *
  • Posts: 20341
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: Meaning of brown pile display of the factory info window
« Reply #1 on: August 02, 2020, 12:26:24 PM »
I have looked at this briefly. The code in question for this is as follows:

Code: [Select]
bool fabrik_t::is_active_lieferziel( koord k ) const
{
assert( lieferziele.is_contained(k) );
return 0 < ( ( 1 << lieferziele.index_of(k) ) & lieferziele_active_last_month );
}

"lieferziel" is German for "delivery target" according to Babelfish.

This code is unchanged from Standard, as can be observed here.

Its only use, both in Standard and Extended, is in the GUI - it does not have any effect on any simulation parameters. All that it does in Extended is determine whether to display this symbol.

The code is very odd: it uses bitwise logic in what should be a basic GUI method that is not at all computationally intensive and does not use any bitfields.

What it seems to check is whether the index of the passed co-ordinate, when bit-shifted one to the left and combined using a bitwise & with the variable lieferziele_active_last_month, is greater than zero.

In turn, the variable lieferziele_active_last_month is only used in this particular GUI function, although it has been loaded and saved since Standard version 112002. There is no code comment explaining this variable.

This variable, in turn, is set in only one place (aside from some places where it is reset to 0), and that is void fabrik_t::verteile_waren(const uint32 product).

The line is:

Code: [Select]
// add as active destination
lieferziele_active_last_month |= (1 << lieferziele.index_of(best_ware.get_zielpos()));

This appears to be set if any goods are sent to their destinations. Again, there is bitshifting of the index of the target in the vector, the purpose of which I do not understand at all.

In short, I suggest that this system shows nothing useful that I can discern and, unless a Standard developer who understands this can explain what this is actually for, I suggest that this logic be replaced with something more meaningful: this is only used in the UI and does not affect any simulation logic.

Online Freahk

  • Devotee
  • *
  • Posts: 1354
  • Languages: DE, EN
Re: Meaning of brown pile display of the factory info window
« Reply #2 on: August 02, 2020, 05:57:12 PM »
Okay, that seems quite odd in the first place. Let's see what's going on:
Code: [Select]
return 0 < ( ( 1 << lieferziele.index_of(k) ) & lieferziele_active_last_month );
1. shift the one by the retrieved index to the left.
  That means 0b1<< 3 = 0b1000 = 8
  It's not 1<< 3 = 1<<0b11 = 0b110 = 6 That's not correct! Forget about this line!

In any case, this seems to be very dangerous! 1 is an int, i.e. it's signed.Left-shifting signeds is well-defined, as long as it's not shifted to or beyond the sign bit.
So if we can ensure our retrieved index is never larger than - whatever the number of bits in our target environment - we are fine. Otherwise, anything can happen.

2. &-combine the result from step 1 with lieferziele_active_last_month, which is according to its name, the active delivery targets of the last month.
That means, the 1 we set in the previous step will remain, if there is a 1 in lieferziele_active_last_month, otherwise all bits will be 0.

3. return true if that one bit was retained, otherwise false.


Code: [Select]
lieferziele_active_last_month |= (1 << lieferziele.index_of(best_ware.get_zielpos()));The same considerations about undefined signed shifting behavior applies here again.
Here we shift the one by index to the left again, that means the bit at that position indicates that the delivery target was active last month.
Now we |-comine that information into the lieferziele_active_last_month bitfield.
That means, we set the bit of the lieferziele_active_last_month  bitfield at the location of the obtained index to 1.

Apart from the undefined behavior thing and bad code readability, this seems fine to me. We should assert that we do never run into undefined behavior here.
« Last Edit: August 02, 2020, 06:26:42 PM by Freahk »

Offline jamespetts

  • Simutrans-Extended project coordinator
  • Administrator
  • *
  • Posts: 20341
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: Meaning of brown pile display of the factory info window
« Reply #3 on: August 02, 2020, 07:19:32 PM »
Another problem here might be if we have more than 32 connected industries.

Online Freahk

  • Devotee
  • *
  • Posts: 1354
  • Languages: DE, EN
Re: Meaning of brown pile display of the factory info window
« Reply #4 on: August 03, 2020, 11:03:58 AM »
Another problem here might be if we have more than 32 connected industries.
That's exactly what the undefined behavior is about ;)
Though, to be preciese, 32 is already too large (as that would shift the one to the sign bit, which is undefined behavior) and to be very preciese, it depends on the target systems size of int, it might as well be 16 or 64 bits on some target architechtures.
« Last Edit: August 03, 2020, 03:21:32 PM by Freahk »

Offline jamespetts

  • Simutrans-Extended project coordinator
  • Administrator
  • *
  • Posts: 20341
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: Meaning of brown pile display of the factory info window
« Reply #5 on: August 03, 2020, 11:20:11 AM »
I am not even sure that the intended behaviour of this method is particularly useful - it is still not clear to me what it is supposed to be doing.