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

Add: Ground sprite constants for partially cleared tiles #195

Merged
merged 2 commits into from Mar 27, 2021

Conversation

rexxars
Copy link
Contributor

@rexxars rexxars commented Mar 24, 2021

This PR adds two new ground sprite constants for "partially cleared" tiles, eg the ones that will gradually appear with more grass over time after bulldozing a tile to the fully cleared state.

I wanted to reuse these tiles for a "swamp" setting that I'm building, and figured it was more of an oversight than a feature that they were not part of the constants. Correct me if I'm wrong!

planetmaker
planetmaker previously requested changes Mar 24, 2021
Copy link
Contributor

@planetmaker planetmaker left a comment

If you compare the constant names to that of snow, a few lines further down, this PR is inconsistent:
there are 4 snowy stages, the fully snowed being GROUNDSPRITE_SNOW_4_4 which is duped as GROUNDSPRITE_SNOW.

Consequently with 3 clearing stages, these omitted constants should be
GROUNDSPRITE_CLEARED_1_3
GROUNDSPRITE_CLEARED_2_3
and
GROUNDSPRITE_CLEARED_3_3 as dupe to GROUNDSPRITE_CLEARED

(or should the latter be 1_3 and the other ones shifted one up? I don't know the order which is cleared 'more'. The one shown immediately after clearing IMHO should be the first, thus 1_3)

@rexxars
Copy link
Contributor Author

@rexxars rexxars commented Mar 24, 2021

There's also GROUNDSPRITE_DESERT_1_2 but no GROUNDSPRITE_DESERT_2_2, which also seems inconsistent with the snow one. Should I add one for that too, to be more consistent?

@rexxars
Copy link
Contributor Author

@rexxars rexxars commented Mar 24, 2021

Which one of these do you prefer, @planetmaker ?

a:

image

b:

image

@frosch123
Copy link
Member

@frosch123 frosch123 commented Mar 24, 2021

My preferences:

name number status my preference
CLEARED 3924 exists
GRASS_1_3 3943 new Strong preference: GRASS_x_y is better than CLEARED_x_y.
GRASS_2_3 3962 new Strong preference: GRASS_x_y is better than CLEARED_x_y.
GRASS_3_3 3981 new Weak preference: add for consistency.
GRASS 3981 new Weak preference: "NORMAL" is weird.
NORMAL 3981 exists
SNOW_1_4 4493 exists
SNOW_2_4 4512 exists
SNOW_3_4 4531 exists
SNOW_4_4 4550 exists
SNOW 4550 exists
DESERT_1_2 4512 exists
DESERT_2_2 4550 new Weak preference: add for consistency.
DESERT 4550 exists

@rexxars rexxars force-pushed the feat/cleared-ground-sprites branch from 3b0228a to 6ef517e Compare Mar 24, 2021
@rexxars
Copy link
Contributor Author

@rexxars rexxars commented Mar 24, 2021

Yeah that makes much more sense, @frosch123!
I've changed the constants in line with your suggestion and added the GRASS and DESERT_2_2 for consistency.

@rexxars rexxars requested a review from planetmaker Mar 25, 2021
@LordAro LordAro dismissed planetmaker’s stale review Mar 27, 2021

dismiss out of date

@LordAro LordAro merged commit 5f25e4d into OpenTTD:master Mar 27, 2021
21 checks passed
@rexxars rexxars deleted the feat/cleared-ground-sprites branch Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants