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

Fix: Correct limit of available road/tramtypes. #110

Merged
merged 1 commit into from Apr 27, 2020

Conversation

FLHerne
Copy link
Contributor

@FLHerne FLHerne commented Apr 27, 2020

According to the NFO spec, there can be up to 63 each of road/tramtypes.

OpenTTD also requires that the combined total of road and tram types be
no more than 63, and that a label used for a roadtype cannot be used for
a tramtype or vice versa.

This directly contradicts the NFO specification, and nmlc will not
check for, nor enforce, these additional limitations.

According to the NFO spec, there can be up to 63 each of road/tramtypes.

OpenTTD also requires that the *combined* total of road and tram types be
 no more than 63, and that a label used for a roadtype cannot be used for
 a tramtype or vice versa.

This directly contradicts the NFO specification, and nmlc will not
 check for, nor enforce, these additional limitations.
@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Apr 27, 2020

Addresses the nmlc part of #109

@FLHerne
Copy link
Contributor Author

@FLHerne FLHerne commented Apr 27, 2020

Based on @frosch123's recommendations on IRC.

Paging @andythenorth and @PeterN in case they have different ideas.

@andythenorth andythenorth merged commit 76da056 into OpenTTD:master Apr 27, 2020
15 checks passed
@FLHerne
Copy link
Contributor Author

@FLHerne FLHerne commented Apr 27, 2020

IMO it was a mistake to merge this without further discussion; we still need to decide if nmlc or OTTD needs 'fixing' for the further issues.

@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Apr 28, 2020

Ok, I'm +1 it's probably worth getting NRT right after so long and so much work by so many people :)

Full declaration, there's some burnout around NRT, me and probably other people too.
It's a very long time since I started a patch to simply remove catenary from tramway with a flag. That grew rather quickly into NRT.

Although it's wholly irrelevant to the PR, this ticket is as good a place as any to put the NRT history, which includes.

  • using GitHub and git (badly, with lots of merges) when the main project was in SVN
  • having the binaries built by the official compile farm, which was good and bad (made it trivially
    available to players for testing, but also set expectations that it HAD to be be included in trunk)
  • the clown shoes NML patch I wrote, again in Git (with bad merges), not the NML mercurial repo
  • the happy background noise of peanuts being thrown that the spec is all wrong
  • the write, rewrite, rebase, review, rewrite, rebase, review cycle for which I persuaded multiple people to 'please help' with the damn thing :)
  • the sense of obligation from quite a lot of newgrf authors putting quite a lot of work into writing NRT newgrfs
  • the desire to ship something big and prove that vanilla OpenTTD isn't dead
  • the realisation that there's nowhere to hide from doing the docs, oof

So yeah, it's been emotional :)

I don't regret NRT overall, but I definitely have regrets, and would kind of like it to go away as my problem, I guess that's not how it works :)

@Wolfolo
Copy link

@Wolfolo Wolfolo commented Apr 28, 2020

I'm +1 for fixing NRT if needed (I don't know a bit of NML, so I can't talk about that), I would have followed precisely the specs if I didn't get confused about the constant changes, opinions, IRC talk, busy with work, my inability to get things right and the difficulty to understand that it was no more my (and Andy's) little project.
I'm also +1 to talk about what fix it need if OTTD-side, just poke me.

@FLHerne FLHerne deleted the fix-roadtypes-count 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants