[Patch] Bridge Pillars Fix

[uni657]

Hello, this patch does just that - fixes bridge pillars, as can be seen on the picture. Patch requires bridge_pillars_fix.grf, which can be downloaded below. Better set it to load static.

On the left side are old pillars, on the right side fixed.

1s.png (45.79 KiB) Viewed 7614 times

Edit: Patch is mostly in trunk.

[uni657]

...

[maquinista]

I like the idea.

I have a question: the background pillars are drawn if the height of the bridge is more than two (and the two first are not drawn).

It is possible to add a flag in the NFO code to draw these to pillars always? I want to create a monorail bridge with a single beam and some supports, but It doesn't look good.

I mean this problem, my idea is to create a bridge with a single monorail beam and supports (I need a new beam between supports, but I have abandoned this project because It couldn't be realized). This bridge is like the TTO monorail bridge, but with some supports:
https://www.tt-forums.net/viewtopic.php?p=869005#p869005


NOTE: this problem is the same in 8 bpp without extra zoom levels.

[uni657]

Yes, it's possible. There are two ways to do it, set patch to always draw all sprites for back pillars of all bridges (I think it won't hurt anyone), or use a flag for specific bridge (2nd flag can be used for this). Which solution is the best depends on how many bridges will use this feature.

For now just comment 313th line in the diff file, or if you wish I can send you modified diff file or binary.

• /* skip 2 rows for back facing pillars hidden by bridge, they are unseen anyway */
  + //int z_back = z_bridge - TILE_HEIGHT * 2;

[maquinista]

Yes, it's possible. There are two ways to do it, set patch to always draw all sprites for back pillars of all bridges (I think it won't hurt anyone), or use a flag for specific bridge (2nd flag can be used for this). Which solution is the best depends on how many bridges will use this feature.

For now just comment 313th line in the diff file, or if you wish I can send you modified diff file or binary.

• /* skip 2 rows for back facing pillars hidden by bridge, they are unseen anyway */
  + //int z_back = z_bridge - TILE_HEIGHT * 2;

Thanks.
This could be used in my bridge and in new bridges which doesn't have a concrete platform, like truss and trestle bridges.
http://www.trainzone.co.nz/log-bridge.htm
http://www.trainweb.org/nzgr/RockyCreek ... tion_b.JPG

The flag could be added in property 0E:
http://wiki.ttdpatch.net/tiki-index.php ... on0Bridges

[uni657]

The flag could be added in property 0E:

Sorry, I meant it, but something has distracted me. To be correct Action 0 for bridges, property 0E bit 2. Please note, this is only for my patch and not official, that means it's unsupported by OpenTTD / TTDPatch / NewGRF.

[SirkoZ]

Beautiful patch (literally), however - wouldn't it be better (at least in the beginning) that no new .GRF file would be needed for it to work (just using default GRF elements)?

Aligned bridge pillars using existing graphics are lacking since way back when the current solution was included in trunk as a fix for even worse misalignment... :/

[uni657]

Beautiful patch (literally), however - wouldn't it be better (at least in the beginning) that no new .GRF file would be needed for it to work (just using default GRF elements)?

Thank you. Idea was that this .GRF file will be included in OpenTTD.GRF overwriting those unused pillar sprites. By the way, are they really unused? I could not find any references for SPR_PILLARS_BASE. There are 24 sprites (1577-1600) in all 3 files (OpenTTD.GRF, OpenTTDD.GRF, OpenTTDW.GRF). Is there a way to check it for sure? As for the question, yes, it's possible, but half of the TTD bridges will look like an aqueduct or any other NewGRF bridge.

[Hirundo]

By the way, are they really unused? I could not find any references for SPR_PILLARS_BASE..

There are many hard-coded sprite numbers in the code. My guess would be table/bridge_land.h

Alternatively, you could load a grf that replaces these sprites with large black boxes and see what happens.

[uni657]

There are many hard-coded sprite numbers in the code. My guess would be table/bridge_land.h

Thanks for the advices, but they are not used for bridges, that I checked, either they are pillars for something else or just unused.

Alternatively, you could load a grf that replaces these sprites with large black boxes and see what happens.

This is not very error-prone solution as I'll need to create every possible object in the game to check that.

[planetmaker]

It's a nice idea to tackle this and a common glitch and I wish you find a good way to fix this. Your results indeed look quite a bit better than currently.

Check if OpenGFX is used is made in hackish way and I don't know how to do it better.
Some OpenGFX and NewGRF bridges are not fixed or even look worse.

But currently it seems to me that you follow a fundamentally flawed approach: OpenTTD's code MUST NOT depend on the use of the base set at all and anywhere (except where the base set is loaded / selected).

If you want to introduce additional sprites which shall be used by base sets, you add them to the base grf (openttd.grf / ogfxe_extra.grf), making them available via action5 - which probably means adding an additional, new block there. If you then subsequently have problems with newgrfs looking ugly, you didn't solve your problem in a backward compatible way. IMHO it'd be quite bad to break existing bridge newgrfs - though there may be cases where advancement justifies that approach. You probably don't want to fix a particular bridge but you want to take the pillar sprite and modify the pillar drawing routine, so that it fixes every bridge whereever it draws a pillar.

This said: the patch also should not contain an option to enable or disable it. Either this will be a good fix - then it should be present always - or not. It just bloats the patch needlessly.

[Zephyris]

If this patch is going to have a chance at being included in trunk then I think you definately need to split it into bits for the devs to look at separately:
* Improvement of bridge pillar drawing (i.e. full height double supports)
* New graphics (via Action5) for cantilever bridge supports
* Full height back pillar drawing
Bite sized chunks of code make the devs happy

[uni657]

But currently it seems to me that you follow a fundamentally flawed approach: OpenTTD's code MUST NOT depend on the use of the base set at all and anywhere (except where the base set is loaded / selected).
...
This said: the patch also should not contain an option to enable or disable it. Either this will be a good fix - then it should be present always - or not. It just bloats the patch needlessly.
...
If you want to introduce additional sprites which shall be used by base sets, you add them to the base grf (openttd.grf / ogfxe_extra.grf), making them available via action5 - which probably means adding an additional, new block there.

As I stated in the "highlight's" topic this is because it's a test version of the patch. Of course, all of it will be removed (or moved to appropriate grf file) in later version.

making them available via action5 - which probably means adding an additional, new block there.

Currently it's works that way.

If you then subsequently have problems with newgrfs looking ugly, you didn't solve your problem in a backward compatible way. IMHO it'd be quite bad to break existing bridge newgrfs - though there may be cases where advancement justifies that approach. You probably don't want to fix a particular bridge but you want to take the pillar sprite and modify the pillar drawing routine, so that it fixes every bridge whereever it draws a pillar.

I'm currently finding how this backward compatibility looks like in case of NewGRF pillars. For now it seems that there are two ways, leave as the patch currently does (fixing all bridges that are done TTD way; moving glitches to other side of the bridge or doing nothing at all for bridges done different way; breaking or "fixing twice" if author tried to fix it's bridge through NewGRF) or don't touch NewGRF bridges at all, so they stay as they are. I think that many "alive" (maintained by the author) NewGRFs will be updated after the patch gets to the trunk (if it gets, that is), remains only backward compatibility with "dead" NewGRFs.

If this patch is going to have a chance at being included in trunk then I think you definately need to split it into bits for the devs to look at separately:
* Improvement of bridge pillar drawing (i.e. full height double supports)
* New graphics (via Action5) for cantilever bridge supports
* Full height back pillar drawing
Bite sized chunks of code make the devs happy

Bad thing is that the core of this patch is very heavy (even compared to the old DrawBridgePillars function), but additional options are quite small:
Action 5: 1 line of code; full height back pillar drawing: half line of code.

[planetmaker]

making them available via action5 - which probably means adding an additional, new block there.

Currently it's works that way.

Then it should also work with an appropriately updated OpenGFX: Adding an action5 entry means adding sprites to the extra grf - which is either openttd.grf (which you supposedly did as it comes along with OpenTTD as the TTD grfs cannot be changed) or ogfxe_extra.grf (which comes with OpenGFX and should not be your but my worry to update - though I wouldn't mind at all, if you supplied the necessary sprites there, too).

[frosch]

Is there any difference between the pillar sprites in the bridge_pillars_fix.grf and the sprites in the original/ogfx .grfs after cutting them in half?

If not I would like to point you to the "SubSprite"-drawing mechanism which is also used for drawing half groundtiles on halftile foundations without adding any new sprites and thus keeping compatibility to all grfs. At least that is the solution I always was heading for for bridgepillars, though I never got to actually coding it

[uni657]

Is there any difference between the pillar sprites in the bridge_pillars_fix.grf and the sprites in the original/ogfx .grfs after cutting them in half?

No, (except 2 sprites for tubular bridges, and 6 sprites for cantilever bridges).

If not I would like to point you to the "SubSprite"-drawing mechanism which is also used for drawing half groundtiles on halftile foundations without adding any new sprites and thus keeping compatibility to all grfs. At least that is the solution I always was heading for for bridgepillars, though I never got to actually coding it

Thanks, I didn't know it is possible to draw only part of the sprite.

Modified the patch, so it uses subsprites instead of the sprites from the grf file, removed fixes for the TTD tubular and cantilever bridges, also removed checks for the NewGRF and OpenGFX, so first stage of the patch is ready, now it only fixes the height of the pillars.

The diff is in the first post.

OpenTTD's code MUST NOT depend on the use of the base set at all and anywhere (except where the base set is loaded / selected).

You are right, but what to do with broken TTD bridges? Breaking them in OpenGFX is not an option. Fix them at load? Is it possible? (cantilever bridge needs first pillar under the bridge drawn with default offset and all other with modified; tubular bridge needs different sprite or drawing second pillar with offset)

[planetmaker]

I don't quite have time to read through the bridge drawing code of OpenTTD...

But both base sets use the exact same code to decide upon the pillar sprites being used. Only the sprites themselves look differently though. But you should 'just' be able to re-use those. Every bridge type knows where to get its pillars from.... and a quick look tells me that each bridge has its individual pillar sprites in the base grf, all in all 11 different sprite ranges (the tubular bridges share sprites and are just re-coloured):
1573 - 1580,
1601 - 1602,
1621 - 1624,
1647 - 1648,
1661 - 1664,
1813 - 1820,
1827 - 1828,
1841 - 1842,
1861 - 1864,
1887 - 1888,
1901 - 1904,
So you'll have to treat each bridge separately as it has its own pillars, even if the default base set should have the same graphics in different places; the associated sprite numbers are known and hard-coded, you just need to use them here. But you could need quite a generic way as there are some bridge types which obviously have four pillar sprites and others which have only two.

Looking at it now, as every bridge has anyway nicely its own pillars, I don't quite see where you need to define new pillar sprites as you could re-use the existing ones, no need to bring your own. And then it'd even work smoothly for any newgrf. Further, if I understand frosch correctly you can draw parts of an existing sprite allowing even more fine-grained control over the height by using a sub-sprite drawing routine (no idea what that implies though).

In short: I think you'll only need to fix the sprite _drawing_ - not the sprites themselves.

An overview over the bridge sprites is found in the bottom part of http://dev.openttdcoop.org/projects/ope ... nfra08.pcx

[uni657]

I don't quite have time to read through the bridge drawing code of OpenTTD...

This is not needed, because the question is how (if it's posible) to do it at load time, which is the preferable way, than in real time like this patch did.

In short: I think you'll only need to fix the sprite _drawing_ - not the sprites themselves.

Both ways give us same result: we need to get from 1 to 2, drawing sprite 3 two times or sprite 4 once. More complicated case is with cantilever bridge, there sprite 8 is drawn three times, but with different offset, so if we can make sprite 9 from sprites 7 and 8, and then correct offset for sprite 8, we will get compatible base sets and no checks for OpenGFX or NewGRF will be needed.

pic1.png (7.62 KiB) Viewed 7089 times

Further, if I understand frosch correctly you can draw parts of an existing sprite allowing even more fine-grained control over the height by using a sub-sprite drawing routine (no idea what that implies though).

It just cuts part from the sprite which is then drawn, sub-sprite is just container for another offset. Changing sprite's offset is also possible from OpenTTD, what I didn't know too.

Edit: Separated patch to 5 parts, they are in 1st and 2nd posts.

[frosch]

I used the advantages of a version control system to explore the usage of SPR_PILLARS_BASE.
It's usage was removed at r4430 between 0.4 and 0.5. Sadly we forgot to remove the unused sprites from openttd.grf during the grf restructuring at 0.6.

Attached images show the pillars as of 0.4 and 0.5 (which is basically the same as 1.0).

The fun conclusion is: It was better in 0.4 The better looking solution was removed in favour of TTDP NewGrf compatibility.
0.4 draws less pillars, with correct length, but not completely symmetric wrt. north and south end.

[uni657]

frosch
Thanks.

This patch in it's current version (v2) don't uses the grf file and also don't changes any sprite, that way feature that changes cantilever bridge pillar sprites to that drawn by me is discarded.