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

Fix #6407: Show snowy ground sprites for depots #7671

Merged
merged 1 commit into from Oct 8, 2019

Conversation

@stormcone
Copy link
Contributor

stormcone commented Jul 27, 2019

This is a quick fix by @KeldorKatarn:
KeldorKatarn/OpenTTD_PatchPack@65e656b

It has the drawback that snow is draw to the inside the depots as well, as the removed comment suggests.

snow_in_depot

This is a quick fix by @KeldorKatarn:
KeldorKatarn/OpenTTD_PatchPack@65e656b

It has the drawback that snow is draw to the inside the depots as well, as the removed comment suggests.
@LordAro

This comment has been minimized.

Copy link
Member

LordAro commented Jul 27, 2019

I fear I have to agree with the original comment - it does look weird

Original comment dates back to r1 - https://github.com/OpenTTD/OpenTTD/blame/ebc18e3957ceaf2c3fdd208a3ad6590875caf8de/rail_cmd.c#L1529

@Eddi-z

This comment has been minimized.

Copy link
Contributor

Eddi-z commented Jul 27, 2019

Could be made a railtype flag, so only NewGRF railtypes who care (and are programmed to handle it) show the climate-specific sprite?

@jcoletto77

This comment has been minimized.

Copy link

jcoletto77 commented Jul 27, 2019

That inconvenient can be overcome by the NewGRF author, if he supplies an overlay for the in-depot ground. In other words, the "snowy" depot sprite should be drawn so that it includes dark ground that will cover the snow.

@planetmaker

This comment has been minimized.

Copy link
Contributor

planetmaker commented Oct 7, 2019

Could be made a railtype flag, so only NewGRF railtypes who care (and are programmed to handle it) show the climate-specific sprite?

It's a bit the question what is the lesser evil: snow inside the depot or no snow around the depot. Both looks weired. IMHO the snow inside (maybe attributed to wind) is less visible than no snow around the depot (attributed to heat from the shed) - but YMMV.

As to NewGRFs... yes, a flag could work and save lengthy groundtype decision trees. But with this patch it wouldn't be needed either as it could be simplified to "check snow" instead of all land types possible.

@LordAro
LordAro approved these changes Oct 7, 2019
Copy link
Member

LordAro left a comment

Changed my mind, definitely looks less weird than the alternative

@planetmaker planetmaker merged commit f1712a5 into OpenTTD:master Oct 8, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190727.2 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.8) Linux linux-amd64-clang-3.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@stormcone stormcone deleted the stormcone:fix-6407 branch Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.