Traffic lights (r17455)

[ChillCore]

Hello Roujin and everybody else,

Attached an update to trunk of the patch against r17455.
I needed to bump it to try and add it to another patch later on and I might as well post the separate updated patch I have ...

Nothing changed in functionality (except for the code to make it work with recent trunk) but I did change the way the sprites are loaded into the game because I was getting questionmarks instead of trafficlights ...

Roujin, could you please adjust table/sprites.h, that is if you want to do so.
The patch works and sprites are loaded correctly but with the code as is now the file will need updating everytime sprites get added to openttd.grf, they are attached to the end of it instead of the way it was before.
I might change it later myself but I wanted to post this version before I mess it up again.
Doing it correctly is something I still have to learn ...

Devs, is there something in particular that is holding this patch away from trunk? Besides extensive testing and the fact that the sprites are not added to the openttd.grf(*).

(*)
I could try to do that if there is a chance of this patch getting to trunk, if not I think it is better to keep them separate to avoid having to distribute a modified version of openttd.grf, which might create all kind of troubles (eg. people putting it in their shared folder).




EDIT:
I made a misstake while bumping, new patch a few posts down ...

[Dwight_K._Schrute]

Nice!

But what's the deal with npf.cpp and yapf_road.cpp. If I checkout r22093 they are not included. I could download r17455 of those two files but would that help?

[Eddi]

they likely got moved around. like src/pathfinder/npf or so

this is likely a mistake in the updating process.

[Sensation Lover]

they likely got moved around. like src/pathfinder/npf or so

this is likely a mistake in the updating process.

So when i try to apply the diff file, what do i have to do with that files that causes the errors?

[ChillCore]

Thank you for reporting Dwight_K._Schrute, my bad.

As Eddi mentioned those files got moved to the pathfinder/ folder.
When files are patched while bumping, they are not removed so I got no errors when compiling.
I correctly removed two other files while adjusting code and forgot about these two afterwards.

Don't bother dowloading and adding old files as it will not work correctly anyway, they should no longer be referenced by the new source and pathfinding penalties for trafficlights will not work even if you add and patch those old files ...
Just aply those two chunks to pathfinder/npf/npf.cpp and pathfinder/yapf/yapf_road.cpp or apply the corrected patch that is attached here.

ps:
If you prefer to apply the new patch do not forget to remove the new files first or they will be applied twice.

[Roujin]

Hey, great to see some of you are still interested in these patches I made some time back
As you may have noticed I've pretty much shifted my attention away from OpenTTD towards other projects nowadays (rarely posting in these forums, just lurking mostly).

Roujin, could you please adjust table/sprites.h, that is if you want to do so.

Umm, I'm not sure what you mean with that. You changed the order of some things there, but it does work, so isn't it fine? What should I want to change?

Devs, is there something in particular that is holding this patch away from trunk? Besides extensive testing and the fact that the sprites are not added to the openttd.grf(*).

Of course first at least one dev must be convinced that this is a valuable addition to OpenTTD gameplay and be interested enough and have enough time to review the patch.
Then he must after review agree with both the implementation of things, and the coding style. I'm sure there would be some stuff disagreed with..

Some things I found myself while doing a quick scan through the .diff:
*STR_ERROR_DRIVE_THROUGH_ON_TOWN_ROAD should be renamed to something generic. The string (at least in english and back when I wrote the patch) doesn't contain drive through anywhere, so I reused it for the TLs instead of making a new one.
*The debug sprite (yellow tile outline) and the respective code to draw it should be taken out, as It's rather unnecessary. Moreover, the debug category "tl" I invented there and the few debug messages should probably be removed. Or extended with additional debug messages that actually make sense and justify that whole new category (I doubt it).
*SetBit(_me[t].m7, 4) -- is this still free? Are devs willing to sacrifice that bit in the map array for traffic lights? If not how else should it be stored?
*ClearAllTrafficLights() -- This is called when trafficlights setting is set from true to false midgame. It looks a bit as if it could be broken for multiplayer (only executed on server), not sure though. Should be checked.

*I feel the "towns build traffic lights during road construction" setting does not make too much sense without the "random road construction" patch I made originally as part of this patch, but split off later. Because without that, towns will only ever have road construction if a player funds it. So either take that feature out or also add the random road construction back into the patch. Unfortunately, this adds new problems: road constructions happen on all roads roads near/inside a town, also player owned ones. With the random road construction + traffic lights patches, players complain that towns will do road construction and place traffic lights on players' "highways" that happen to be too close to the town...
In vanilla OpenTTD, nobody complains about that fact, because (I reckon) the fund road construction option is really really rarely used.
Problem is that it's not trivial to fix. Allow towns only to do road construction on self-owned roads? Bad idea because towns grow into roads placed by players, but never "annex" those roads, so such a mechanic would be needed first. This on the other hand could lead to new griefing possibilities in multiplayer.
So all in all this is opening a can of worms...
(note that my knowledge about implementation of e.g. the roadworks in OpenTTD is from back when I did these patches, so stuff may be different now than how I remember it)

* I don't know if devs agree with my idea and/or its implementation of the traffic light consists (adjacent tiles with traffic lights placed on them are merged to one big crossing) at all. I thought I was pretty clever with how I implemented them back then However looking at it now I'm not fully satisfied, there may be better ways of implementing them.

[edit some more just came to my mind:]
* devs would maybe want newgrf control over more than the sprites (which would come with properly including them in openttd.grf), say tl phases controllable via newgrf callbacks, or such.
* Maybe take out the hardcoded offsets for left-and-right roadside, specify the sprites relative to tile normally. Double the amount of sprites to have left-side-driving and right-side-driving variants (e.g. for half-arc-type TLs that bend over the road, the current offset scheme won't do).

Well that's all I can think of right now.
Of course that's all just my opinion. Devs of course might have a completely different one on which things must be changed for trunk (if any of them considers the whole idea as interesting for trunk, at all). Just giving my own guess.

Unfortunately I'm not really interested in picking up work on this patch again, but if anyone else is (well ChillCore, you're already at it it seems), be my guest

[Zephyris]

I think I'll just jump in and express my support for any continued development of this patch, I think it can make the game more interesting, fun and, dare I say it, realistic!

[ChillCore]

*SetBit(_me[t].m7, 4) -- is this still free? Are devs willing to sacrifice that bit in the map array for traffic lights? If not how else should it be stored?

According to the files in the docs folder, yes. However I have noticed that sometimes when loading old games/maps (eg.Cindini Map) trafficlights are added to roads rather randomly.
I fixed that in afterload.cpp ... I have not yet noticed this behaviour when loading games made after the patch is included.

*ClearAllTrafficLights() -- This is called when trafficlights setting is set from true to false midgame. It looks a bit as if it could be broken for multiplayer (only executed on server), not sure though. Should be checked.

I can not test multiplayer games. I will disable changing the settings for clients in a later version.

*I feel the "towns build traffic lights during road construction" setting does not make too much sense without the "random road construction" patch I made originally as part of this patch, but split off later. Because without that, towns will only ever have road construction if a player funds it. So either take that feature out or also add the random road construction back into the patch. Unfortunately, this adds new problems: road constructions happen on all roads roads near/inside a town, also player owned ones. With the random road construction + traffic lights patches, players complain that towns will do road construction and place traffic lights on players' "highways" that happen to be too close to the town...
In vanilla OpenTTD, nobody complains about that fact, because (I reckon) the fund road construction option is really really rarely used.
Problem is that it's not trivial to fix. Allow towns only to do road construction on self-owned roads? Bad idea because towns grow into roads placed by players, but never "annex" those roads, so such a mechanic would be needed first. This on the other hand could lead to new griefing possibilities in multiplayer.
So all in all this is opening a can of worms...
(note that my knowledge about implementation of e.g. the roadworks in OpenTTD is from back when I did these patches, so stuff may be different now than how I remember it)

Something to think about indeed ... I left it in for the moment.

* I don't know if devs agree with my idea and/or its implementation of the traffic light consists (adjacent tiles with traffic lights placed on them are merged to one big crossing) at all. I thought I was pretty clever with how I implemented them back then However looking at it now I'm not fully satisfied, there may be better ways of implementing them.

I like this feature of the patch, so much in fact that I have extended it to allow for 3 by 3 roadgrids to be synchronised too.
To not upset the Devs , I have also included an option where one can select how far trafficlights look to be synchronised. Minimum setting is 0 so it can be disabled completely.

* devs would maybe want newgrf control over more than the sprites (which would come with properly including them in openttd.grf), say tl phases controllable via newgrf callbacks, or such.

The goal is to have the sprites in the source ... but will that still allow to select the type of signals one wants to use? Devs could pick the version they prefer and add those?
Anyway I am not comfortable with all those grf callbacks (yet) so I will not be adding that code, if needed, for the moment.

* Maybe take out the hardcoded offsets for left-and-right roadside, specify the sprites relative to tile normally. Double the amount of sprites to have left-side-driving and right-side-driving variants (e.g. for half-arc-type TLs that bend over the road, the current offset scheme won't do).

Not comfortable with that neither ... I could use some help here ... Is Someone present?

Unfortunately I'm not really interested in picking up work on this patch again, but if anyone else is (well ChillCore, you're already at it it seems), be my guest

No problem. I had to recompile your grf(s) with DOS sprites for the patch to work correctly with current trunk.
Would you mind linking to them from the first post and is it OK if I continue using your thread? Or would you rather have me creating a new one to avoid confusion?
Final question: I have not added a lisence to the grfs because I could not find which one you want(ed) to use. Would it be OK for me to add the GPL lisence, same version as OpenTTD?

Thank you for the many usefull coments Roujin, below is what I have changed so far.

I think I'll just jump in and express my support for any continued development of this patch, I think it can make the game more interesting, fun and, dare I say it, realistic!

Enoy the attached version. Do grab the recompiled GRF(s) you will need them to avoid pinky sprites.



Changes:
- Added a new file -> trafficlight.h (I have put the arrays here that are used by the patch).
- Added headers to the new files -> mentioning that OpenTTD's source is GPL.
- Little bit of coding style in the documentation.
- Trafficlight can now look 4 tiles away for synchronising themselves (square).
- Added option to select the distance trafficlights look for synchronising themselves (min is 0 -> no synchronising), max is 4 -> for 3 by 3 road grids))
- Added two more error messages (building trafficlights is disabled and building them on town roads is disabled).
- STR_ERROR_DRIVE_THROUGH_ON_TOWN_ROAD -> STR_ERROR_TRAFFIC_LIGHTS_NOT_ALLOWED_ON_TOWN_ROADS
- Removed the debugging code.
- Recompiled the GRF(s) with DOS sprites and removed debugging sprite.
- The sprites are re-introduced in the old way so most likely when sprites are added to openttd.grf there is no need for adjustments.
- Savegamecode adjusted to ini files where needed.
- TL_SV instead of 200 in savegamecode.
- When loading older savegames loop over the entire map and remove all trafficlights that ay be present (I do not know i this bug was present in a version before I bumped the patch)

[Jackson21]

How Do I Install This In The Game Do I Just Put In The Data Folder

[Level Crossing]

It's a patch that you have to compile into the source code.
http://wiki.openttd.org/Category:Compiling_OpenTTD

[kamnet]

Just an idea - would it be possible for an additional config where if a light is placed in an area which is "rural" (say, under 2000 people) that the lights are replaced with stop signs?

[Eddi]

that kinda totally defeats the point of having traffic lights, if no actual lights are placed.

[kamnet]

Not really. Most areas that have little to no population or traffic do not normally invest in stop lights, but stop signs. As the population grows, the area gets more traffic, then the stop signs are replaced with stop lights.

To me, it's not so much about the actual light, but the fact that traffic has to obey the signs, whatever they may be.

[Lockwood]

I'd lean more towards putting Diagram 602 (Give Way) signs on those junctions, effectively putting a priority on the road being joined.

[Westo454]

How do you install this?

[Level Crossing]

How do you install this?

Most people will point you to this.

You will need to compile the source code (of a very old revision of OTTD) for this to work.

[Quast65]

Are there any newer versions of this patch around (or how can one make one)?

I've just figured out how to patch and compile and I would like to use this patch in a later version of OTTD.

[kamnet]

AFAIK, ChillCore has been the only person making updates, which have only been applied in his patch pack.

[ChillCore]

Are there any newer versions of this patch around

AFAIK, ChillCore has been the only person making updates, which have only been applied in his patch pack.

I did not have a separate version after the one I posted and I did not have the need to update it neither as my patchpack is still based on a revision very close to the latest patch posted above.
Anyway I made one and have attached it below.
The game runs and trafficlights can be built but testing if everything still works correctly is is for you.

(or how can one make one)?

I've just figured out how to patch and compile and I would like to use this patch in a later version of OTTD.

The easiest way is applying the "old" version to the correct revision, then bumping your source while solving all conflicts.
Then you compile while solving errors that still may be present; errors (often codechanges in trunk) that the bumping process does not tell you about are common.

You could try doing it yourself and then comparing your diff with mine if you get stuck in the middle.

[Quast65]

Thnx for making this! It nicely compiles (I'll test it out a bit later)

However, how does "bumping" work? As I have another patch I would like to apply (signals in tunnels), that is for another version (r24031), that I might want to try to "bump".

Is it even recommended to apply multiple patches on one version?

Sorry for asking this, but having just found out how the compiling works has made me curious.