Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advanced Fences: (with patch) #6315

Closed
DorpsGek opened this issue Jun 8, 2015 · 17 comments
Closed

Advanced Fences: (with patch) #6315

DorpsGek opened this issue Jun 8, 2015 · 17 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay stale Stale issues

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Jun 8, 2015

pi1985 opened the ticket and wrote:

Is it a bug or a feature? In grass tiles rails drawn with fences, but in snowed or desert tiles without them.
This patch solve this. Also it add bit ADVANCED_FENCES for use for drawing 32 spritres: 16 for grass and desert tiles and 16 for snowed ones (8 tiles for front and 8 for back of the fence).
Used trunk r27261.

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6315
@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 8, 2015

frosch wrote:

Adding a flag for also drawing fences on snow is fine.
But the snowy fence should not use new offsets. The GRF can check itself for snow and various other things.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13956

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 9, 2015

George wrote:

Do you mean graphics? Snowed graphics may be, for example, 1 px higher.
Or what offsets did you mean?
https://dev.openttdcoop.org/projects/sprite-tests/repository/revisions/5962aed2dc5e


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13957

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 9, 2015

George wrote:

How do you intend a GRF should provide different graphics it snow?


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13958

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 9, 2015

pi1985 wrote:

You maybe mean that this sprites should be in different sprites group (RTSG_FENCES_FRONT, RTSG_FENCES_BACK for example) with same indexes?
And GRF should detect snowed tile and return different base image (if this is possible at all)?


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13959

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 9, 2015

frosch wrote:

Railtype GRFs draw snowy tracks, underlays and depots using the "terrain_type" variable. For example swedish rails: https://dev.openttdcoop.org/projects/swedishrails/repository/entry/src/railtypes.pnml# L19

Drawing fences should be no different, so there is no need for RFO_XXX_SNOW_YYY. It only needs a flag for considering drawing them in the first place.

I notice, the patch also adds another thing, which you call XXX_TWO_YYY. This looks like you add fences for more orientations. Enabling this could also be done by extending GetCustomRailSprite to return the number of sprites in the set (group->GetNumResults). Compare it with vehicles, which can also provide either 4 or 8 sprites. It does not need a flag for enabling. (Though if it would need one, it should be separate from "draw on snow".)


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13960

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 9, 2015

George wrote:

I do not see tile height variable here http://newgrf-specs.tt-wiki.net/wiki/NML:Railtypes# Railtype_variables, like nearby_tile_height for houses


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13961

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 9, 2015

George wrote:

Also, I can't understand how can GRF recognize the railway side (XXX_TWO_YYY)
I suppose you could provide some explaining based on https://dev.openttdcoop.org/projects/sprite-tests/repository/revisions/b2a3cc8bfea7 for example
because I used townzone instead of terrain_type and I can't make it work

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13962

@DorpsGek
Copy link
Member Author

planetmaker wrote:

You don't need height information. terrain_type gives you the indication by means of TILETYPE_SNOW on when to draw snow and when not.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13964

@DorpsGek
Copy link
Member Author

George wrote:

I need it, because I want to provide several levels of snow - "a little snow", "some snow", "much snow" and "fully snowed"


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13965

@DorpsGek
Copy link
Member Author

pi1985 wrote:

Patch update.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment13966

@DorpsGek
Copy link
Member Author

frosch wrote:

The 16 sprite version has been implemented in r27343.

The fences-on-snow needs more work:
- Currently the code to decide which fences to draw is duplicated between DrawTrackDetails and TileLoop_Track in the patch.
- Fences are placed instantly above snow, but not below snow.
- Fences do not work on half-snow tiles.

Probably this needs reshuffling of the RailGroundType enum. There are 5 different ground types, with up to 4 fence combinations. However, not all combinations are possible, so 4 bits would still suffice:
0 barren, no fence
1 grass, no fence
2 grass, fence 1
3 grass, fence 2
4 grass, fence 1 and 2
5 snow/desert, no fence
6 snow/deset, fence 1
7 snow/deset, fence 2
8 snow/deset, fence 1 and2
9 half-snow, no fence
10 half-snow, fence
11 water, no fence
12 water, fence
13-15 unused


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment14019

@DorpsGek
Copy link
Member Author

pi1985 wrote:

Here is remake fences in snow patch.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment14068

@DorpsGek
Copy link
Member Author

pi1985 wrote:

Here is remake fences in snow patch.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment14070

@DorpsGek
Copy link
Member Author

George wrote:

Any progress on the adding the patch to the trunk?


This comment was imported from FlySpray: https://bugs.openttd.org/task/6315#comment14098

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
@frosch123 frosch123 removed the Core label Apr 14, 2018
@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth
Copy link
Contributor

Thanks for this. There's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. Since OpenTTD moved to GitHub, we use pull requests rather than patches, as they are a much more productive workflow.

I'm planning to close this soon (in 7 days), as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun.

If you would like to continue with this patch, the best way would be to move the patch to your own GitHub fork, update it for the current OpenTTD master, and then create a pull request. For more information, please see our CONTRIBUTING.md.

We are also happy to discuss directly on the issue, or in #openttd irc, including help to get this into a pull request. Thanks for your contribution!

@andythenorth
Copy link
Contributor

No longer stale, won't close currently.

@stale
Copy link

stale bot commented Mar 12, 2019

This issue has been automatically marked as stale because it has not had any activity in the last two months.
If you believe the issue is still relevant, please test on the latest nightly and report back.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale Stale issues label Mar 12, 2019
@stale stale bot closed this as completed Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay stale Stale issues
Projects
None yet
Development

No branches or pull requests

3 participants