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

Feature: Show rail/road/tram NewGRF name in Land Area Information window #8794

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

perezdidac
Copy link
Contributor

@perezdidac perezdidac commented Mar 2, 2021

Motivation / Problem

Currently there is no way of retrieving NewGRF name for a rail, road or tram track.

See #8779

Description

This code change introduces extra fields in the Land Area Information window to display the NewGRF names for rail, road and tram tracks, as well as for stations. It works for railroad crossings and overlapping road and tram tracks.

image

Limitations

Not that I am aware of.

Checklist for review

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

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@LordAro
Copy link
Member

@LordAro LordAro commented Mar 3, 2021

Looks fine on the whole. Few warnings for you to take a look at - grffile is not null

It also concerns me that you've apparently taken a photo of your screen ;)

@perezdidac
Copy link
Contributor Author

@perezdidac perezdidac commented Mar 3, 2021

Looks fine on the whole. Few warnings for you to take a look at - grffile is not null

It also concerns me that you've apparently taken a photo of your screen ;)

Took the photo because screenshots are not working in the most recent 1.11 beta ;-)
Thanks for the comment, will work on those warnings.

Copy link
Member

@LordAro LordAro left a comment

Can I be mildly unreasonable and ask that the (initial) TrackDesc changes be in a separate commit? Then adding the GRF name is clean and easy to read & review

src/tile_cmd.h Outdated Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
@LordAro
Copy link
Member

@LordAro LordAro commented Mar 14, 2021

Not really what I meant. I'd rather an actual constructor:
TrackDesc(StringID type, uint16 speed, GRFFile *grffile);

(and you can also have a "default" constructor that "zero-initialises" the members too)

@perezdidac perezdidac force-pushed the misc branch 2 times, most recently from df7ae42 to ec698c5 Compare Mar 19, 2021
src/tile_cmd.h Outdated Show resolved Hide resolved
@perezdidac perezdidac force-pushed the misc branch 2 times, most recently from b2eccae to 4142097 Compare Mar 21, 2021
src/newgrf_config.cpp Outdated Show resolved Hide resolved
@perezdidac
Copy link
Contributor Author

@perezdidac perezdidac commented Apr 21, 2021

FYI this is ready for review at this point :)

@@ -2808,8 +2808,7 @@ static bool ClickTile_Track(TileIndex tile)
static void GetTileDesc_Track(TileIndex tile, TileDesc *td)
{
const RailtypeInfo *rti = GetRailTypeInfo(GetRailType(tile));
td->rail_speed = rti->max_speed;
td->railtype = rti->strings.name;
td->rail_desc = TrackDesc(rti->strings.name, rti->max_speed, rti->grffile[0]);
Copy link
Member

@michicc michicc Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The console command dump_info railtypes (that probably no one knows about) uses rti->grffile[RTSG_GROUND] (i.e. index 2) for GRF info output.

(Also applies to all other instances where track GRF is queried).

@@ -2098,14 +2098,12 @@ static void GetTileDesc_Road(TileIndex tile, TileDesc *td)
RoadType tram_rt = GetRoadTypeTram(tile);
if (road_rt != INVALID_ROADTYPE) {
const RoadTypeInfo *rti = GetRoadTypeInfo(road_rt);
td->roadtype = rti->strings.name;
td->road_speed = rti->max_speed / 2;
td->road_desc = TrackDesc(rti->strings.name, rti->max_speed / 2, rti->grffile[0]);
Copy link
Member

@michicc michicc Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above for road, ROTSG_GROUND (2).

(Also applies to all other instances where road/tram GRF is queried).

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

3 participants