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

Avoid duplicating rail/road/tramtype table code #60

Merged
merged 1 commit into from Nov 24, 2019

Conversation

FLHerne
Copy link
Contributor

@FLHerne FLHerne commented Nov 19, 2019

All this virtually-identical code was copied in triplicate.

Should be no functional changes; regression tests pass.

@FLHerne
Copy link
Contributor Author

FLHerne commented Nov 19, 2019

Variable names table_prop_id and cond_tracktype_not_defined could do with review, I wasn't really sure what to call them.

(I think the current names were suggested by someone else already)

@FLHerne FLHerne force-pushed the dedup-tracktype-2 branch 2 times, most recently from e3f8470 to 851e166 Compare November 19, 2019 18:51
@andythenorth
Copy link
Contributor

andythenorth commented Nov 19, 2019

Tests pass for me.
NML example_roadtype_and_tramtype.nml builds.
Iron Horse builds.

Someone else should read the diff ideally.

LordAro
LordAro previously approved these changes Nov 23, 2019
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

LGTM

@glx22
Copy link
Contributor

glx22 commented Nov 24, 2019

Since checks are enabled, a rebase to master is required to include workflow file.

nml/ast/tracktypetable.py Outdated Show resolved Hide resolved
All this virtually-identical code was copied in triplicate...
Copy link
Contributor

@glx22 glx22 left a comment

Choose a reason for hiding this comment

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

seems OK

@LordAro LordAro merged commit e8c07ae into OpenTTD:master Nov 24, 2019
@FLHerne FLHerne deleted the dedup-tracktype-2 branch April 2, 2021 22:18
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