Skip to content

Add: Slope-aware and roadtype-specific one-way sprites - #10282

Merged
2TallTyler merged 1 commit into
OpenTTD:masterfrom
FLHerne:flh-roadtypes-minimal
Dec 26, 2022
Merged

2TallTyler merged 1 commit into
OpenTTD:masterfrom
FLHerne:flh-roadtypes-minimal

Conversation

@FLHerne

@FLHerne FLHerne commented Dec 25, 2022

Copy link
Copy Markdown
Contributor

Motivation / Problem

As in #10238:

  • The baseset only includes one-way road indicators for flat ground. These look bad when placed on slopes.
  • NewGRF road types can't provide their own type-specific indicators. The baseset ones may be inappropriate for some road types (e.g. painted markings on dirt roads are silly) @LC-Zorg.

Description

Action5 type 0x09 is extended to accept a set of 18 sprites.
The first 6 are unchanged from before this patch, followed by similar groups for north-facing and then south-facing slopes.

For compatibility with older basesets and NewGRFs, if an Action5 provides only sprites for flat tiles these will be used on slopes as in previous versions.

Action3 type 0x0B, for roadtypes only, may optionally provide a set of 18 sprites as above for use with that roadtype.

OpenTTD/OpenGFX#82
OpenTTD/nml#274

Limitations

This doesn't yet allow the baseset itself to provide indicators for sloped tiles, because I haven't yet understood how to add sprites to the baseset without breaking compatibility. I intend to do that before merging.
This is done now.
Using flat tiles for slopes, if no sloped tiles are provided, for compatibility might be confusing if for some reason a GRF wanted to override only the flat tiles. I can't think of a reason to do that.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

@glx22

glx22 commented Dec 25, 2022

Copy link
Copy Markdown
Contributor

Basesets use action5, so handling more sprites here should be doable, if only first 6 are provided use openttd included ones for the missing.

@FLHerne

FLHerne commented Dec 25, 2022

Copy link
Copy Markdown
Contributor Author

I know it's possible, I just haven't yet figured out the details of how it's done. Will look again when it's not 3am on Christmas Day...

@glx22

glx22 commented Dec 25, 2022

Copy link
Copy Markdown
Contributor

https://github.com/OpenTTD/OpenTTD/blob/master/src/newgrf.cpp#L6160
Looks like changing ONEWAY_SPRITE_COUNT should be enough.

It's already handled like OpenTTD GUI block (and we increased it many times).

@FLHerne
FLHerne force-pushed the flh-roadtypes-minimal branch from 2bc18ba to fd6b72d Compare December 25, 2022 12:25
@FLHerne FLHerne changed the title WIP: Feature: Roadtypes can provide custom one-way indicators Feature: Slope-aware and roadtype-specific one-way sprites Dec 25, 2022
FLHerne added a commit to FLHerne/OpenGFX that referenced this pull request Dec 25, 2022
For OpenTTD/OpenTTD#10282.
To-do: draw less ugly ones. Maybe poke a decent pixel artist.
@FLHerne

FLHerne commented Dec 25, 2022

Copy link
Copy Markdown
Contributor Author

Action5 is supported now; this is ready for review.

Comment thread src/newgrf.cpp Outdated
Comment thread src/table/sprites.h
@2TallTyler

Copy link
Copy Markdown
Member

I think the failed commit check is an actions failure and not a problem with your PR. Hope it'll run properly next time you push it.

@michicc

michicc commented Dec 25, 2022

Copy link
Copy Markdown
Member

This doesn't yet allow the baseset itself to provide indicators for sloped tiles, because I haven't yet understood how to add sprites to the baseset without breaking compatibility. I intend to do that before merging. This is done now.

Ideally, the one-way road graphics from openttd.grf would be updated, too. One could argue if the graphics should be in openttd.grf or in orig_extra.grf.
Right now they are in openttd.grf, which means any baseset that does not provide one-way arrows will get the ones from there. Adding them there would preserve this, while needing code to do the legacy behaviour when a baseset only sets the old amount. Adding to orig_extra.grf would avoid this, but results in a different look if a baseset uses the game-supplied arrow sprites.

@FLHerne FLHerne changed the title Feature: Slope-aware and roadtype-specific one-way sprites Add: Slope-aware and roadtype-specific one-way sprites Dec 25, 2022
@PeterN

PeterN commented Dec 25, 2022

Copy link
Copy Markdown
Member

Something is not right with it. I reran the CI but it didn't resolve it.

@FLHerne
FLHerne force-pushed the flh-roadtypes-minimal branch 2 times, most recently from ca2669e to 493ce94 Compare December 26, 2022 17:30
@FLHerne

FLHerne commented Dec 26, 2022

Copy link
Copy Markdown
Contributor Author

Added sloped graphics to openttd.grf. I got a bit lazy with the 'impassable' sprites because they're useless so no-one will ever see them; gave them better offsets but didn't redraw the sprite.

These will only be visible if no one-way sprites at all are provided by a baseset or GRF; i.e. when using the TTD baseset.
If a baseset or GRF provides flat one-way sprites they'll be used on slopes per Action5 compat; this looks less bad IMO than using a completely mismatched style especially for zBase.

The CI issue was caused by my branch being based on a rather old checkout of OTTD, rebasing cleared it up.

Per OpenTTD#10238, the baseset only provided one-way sprites for flat tiles,
 matching the default road type. These look bad on slopes or on
 some custom roadtypes.

Action5 type 0x09 is extended to accept a set of 18 sprites.
The first 6 are unchanged from before this patch, followed by
 similar groups for north-facing and then south-facing slopes.

For compatibility with older basesets and NewGRFs, if an Action5
 provides only sprites for flat tiles these will be used on slopes
 as in previous versions.

Action3 type 0x0B, for roadtypes only, may optionally provide a
 set of 18 sprites as above for use with that roadtype.

Sprites for sloped tiles are added to openttd.grf, used when
 no one-way sprites are provided by the baseset (i.e. when using
 the original TTD graphics).
@FLHerne
FLHerne force-pushed the flh-roadtypes-minimal branch from 493ce94 to 5f77d3c Compare December 26, 2022 19:32
@2TallTyler
2TallTyler merged commit 6caed5f into OpenTTD:master Dec 26, 2022
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.

5 participants