[patch] Trolleybus tab for road depot

[VS]

Hello,
here is an attempt at working code : It compiles, links, runs and doesn't crash, so I assume it's time for review.

What it does - in road depot (and only there), electric vehicles get their own tab. Articulated parts can be forced to display there by adding "engine_type=electric", similarly to forcing passenger trainsets onto their tab by adding zero passenger capacity for locos and tenders.

Code notes:

• The trolleybus status has highest priority, even over passengers.
  
• I had to add new function for tab name to all depot_t descendants. No idea if there is a good workaround.
  
• There is a problem: the current logic for deciding which tab is active (where I replaced ?: by switch) does not work when some tabs are missing not at the end. This is the reason why passenger tab is always added - it can't be missing or the depot breaks! So I added some public information to tabs themselves to identify them (that elegantly removes the need to assume they are in any specific order). It is one int called "tag" in gui_komponente_t... that's 1:1 with what Delphi has; a 32bit field for any gui stuff, free to be used by programmers for anything, regardless of the actual gui class - button, canvas, window etc.
  

EDIT: fixed, added new patch file and so on.

[prissi]

I am not very keen on the electric tab. Especially since cars with engine_type=electric could not be appended to a non-electric engine. But let's hear more poeple on this.

Currently it looks like a special tab for trolley busses (even with this name) and this I want to avoid: Any stuff should be a general as possible. Thus it should work well with electric engines too.

Also I do not thing you need to add extra amount of memory to a zillion structures, when you just want to change the gui_tab structure. This is not very OOP imho. And any programmer for a portable program should  avoid using int, since it can be of any size from 8 to 64 bit (although mostly from 16 to 64 bit for processors still around).

[VS]

A little clarification - the new tab is only for road+electric, since other combinations don't make sense or are not different. Air and water - no electrification. All rail vehicles already need building the track, so there isn't much of a difference. Only with road vehicles it is different because you have roads "for free" in cities so there is no maintenance; the need for catenary changes that: trolleybus lines in reality need more traffic to be economical, too. Also trolleybuses better accelerate and can't drive faster than some 80 km/h because of the wires. So I felt they are almost another category, compared to buses.

Of course this is subjective... Otherwise I agree strongly that electric tab everywhere does not make sense. If you don't want special tabs for one kind of depot, it's clear.

I thought int was globally redefined to sint32 - it turns out it wasn't (so what is the point of including simtypes.h only somewhere?).

What would be an OOP approach? Subclass the tab to depot_tab?

[prissi]

First the electrification argument applies to roads too: No electrification no trolley busses.

And an electric tab might be useful for pak128 and pak.japan128 with the many DMU and EMU, which would be seperated (giving proper definitions.)

[VS]

Uhm, I think we don't understand each other. Or you understand me but I don't.

Regardless: this was just a test if I can write something useful. If you feel it's wrong, no need to talk about it; this already served for learning purposes

[prissi]

Nono, is so far just you and me. Since I usually have to support stuff included in the truck, I just want to have those things well discussed. Just give it a little longer wait.

[VS]

Thanks for adding it!

But there is something else... not of the crashing kind, but it's a bug nevertheless. The electric tab is always shown on road. I'm looking into it.

EDIT: Of course typecasting to weg_t instead of schiene_t works, but the tile is asked only for rail! I knew rail had to be special in some way again Fixed.

[prissi]

The safest way is to use the depots waytype for lookup ... with trams way(0) would be the wrong way. Anyway, fixed.

[VS]

Oh, right... I didn't notice that. :( Sorry for giving you buggy code again, let's hope it doesn't become a tradition.

[prissi]

That is why I do code review; unfourtunately only very few look at mine :(

[VS]

But so far you did very well, so why?

[prissi]

I wrote very bad code at the beginning, with nearly zero knowledge of OOP, especially much stuff od simplay was horrible but also some other parts are bad. Most of them I cleaned up meanwhile, but still, I think I could learn a lot. tron did some critics of my code though, although we had quite different progrmming styles.

[VS]

Heh. You don't want to know how Shades look under the hood, no kidding. I tried to refactor it but the whole thing is like writing the more important part from zero. So zero progress. Same issue, start with no knowledge of OOP but lots of ideas and quick fingers to transfer them into bad code that works.

[VS]

Tram depot now needs some fancy text for new tab, too!

(And after that some way to find how to add only needed tabs)

[IgorEliezer]

I come here just to thank for new feature. I discovered it after I tested an add-on developed by Wender, a PT-member.



http://forum.simutrans.com/index.php?topic=836.0

A question, please: is "TrolleyBus_tab" string translatable?

[prissi]

In principle yes, I just need to upload it (and a few others).

[VS]

I was thinking about the impact of this change on UI and making another tab visible everywhere. It's good only in road depot, otherwise not so good... would you mind if this new tab could be switched on/off per waytype from config file?

[prissi]

Why? It is very useful for pak.japan128 for instance, where many vehicles are lectirc EMUs I do like it. If there is not fitting unit, it will not be shown anyway.

[VS]

Maybe you are right. It's just that I heard some people complain about this...

[prissi]

So far nobody in this forum ...

[VS]

True