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 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 Nov 19, 2019
@andythenorth
Copy link
Contributor

@andythenorth 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.

Copy link
Member

@LordAro LordAro left a comment

LGTM

@glx22
Copy link
Contributor

@glx22 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...
@FLHerne FLHerne force-pushed the dedup-tracktype-2 branch from a478abc to adc4f2f Nov 24, 2019
glx22
glx22 approved these changes Nov 24, 2019
Copy link
Contributor

@glx22 glx22 left a comment

seems OK

@LordAro LordAro merged commit e8c07ae into OpenTTD:master Nov 24, 2019
12 checks passed
@FLHerne FLHerne deleted the dedup-tracktype-2 branch Apr 2, 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants