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

Jump to bottom

Long format model life property for vehicles #3044

Closed
DorpsGek opened this issue Jul 20, 2009 · 8 comments
Closed

Long format model life property for vehicles #3044

DorpsGek opened this issue Jul 20, 2009 · 8 comments
Labels
component: NewGRF This issue is related to NewGRFs flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

Terkhen opened the ticket and wrote:

There is a long format introduction date for vehicles, but the model life property is still limited to a maximum of 254 years. I have been working to implement a long format model life property for vehicles that allows up to 65534 years. Since model life values greater than 255 would conflict with the current way of determining if a vehicle never expires, it is implemented in a different way. The EngineInfo struct now has a never_expire field. This field is set to true in the following cases:

- When a value of 0xFF is found at a model life property.

- When a value of 0xFFFF is found at a long format model life property.

- When initializing base engines out of the original engine data range.

- For all standard wagons.

This new value replaces the (base_life == 0xFF) condition when checking early retirement or if the engine is at the phase 2 of its life.

The long format model life property is word sized and has the following property numbers:

Trains: 0x2B
Road vehicles: 0x21
Airplanes: 0x1C
Ships: 0x1C

Since I don't know how the original property numbers were decided, I just used the first available position at each vehicle.

Attached is a diff file containing the source changes, a win32 binary and the two NewGRF files I used for testing this patch.

Attachments

Reported version: trunk
Operating system: All

This issue was imported from FlySpray: https://bugs.openttd.org/task/3044
@DorpsGek
Copy link
Member Author

Belugas wrote:

Why do you set the never_end value to true in the engine's constructor?
That eludes me a bit.
Note that you might have a very good reason. Just wondering...

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6363

@DorpsGek
Copy link
Member Author

frosch wrote:

The constructor thingie is ok.

But:

  1. Using bigger numbers will overflow duration_phase2. So e.g. limit it to 5000 years. (I don't think bigger numbers are of any interest)
  2. Adding never_retire adds redundancy. Instead add e.g. static const Engine::BASELIFE_INFINITE = 0xFFFF, and convert the 0xFF when the old property is set.
This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6364

@DorpsGek
Copy link
Member Author

Terkhen wrote:

The never_expire in the constructor was there to replace the old assignment to 0xFF, but as frosch said it was duplicating info. I have implemented both corrections. The long model life property now allows to set a number from 0 to 5000, with anything greater than 5000 being set to 0xFFFF.

EDIT: (Incorrect version, see new comment below)

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6377

@DorpsGek
Copy link
Member Author

Terkhen wrote:

It seems I uploaded an intermediate version by mistake. I am sorry, here is the real patch:

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6378

@DorpsGek
Copy link
Member Author

Belugas wrote:

const int Engine::BASELIFE_MAXIMUM = 5000; <---- suggestion

Clamp(grf_load_word(&buf), MIN_YEAR, Engine::BASELIFE_MAXIMUM); <---- another suggestion

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6379

@DorpsGek
Copy link
Member Author

Terkhen wrote:

Thanks for the suggestion; I have implemented it. Besides the old base life maximum (0xFF), I think that there's no more magic numbers remaining in the patch code.

What behaviour when numbers greater than the maximum base life is preferable?. Right now values greater than 5000 are clamped to 5000, being 0xFFFF the only value that creates a never expiring vehicle. If implementing that any value greater than 5000 creates a never expiring vehicle is preferred, I will change it back.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6380

@DorpsGek
Copy link
Member Author

Terkhen wrote:

I have done two GRFs that are more interesting for testing this patch. They can be found here:

http://www.tt-forums.net/viewtopic.php?p=806329# p806329

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044#comment6400

@DorpsGek
Copy link
Member Author

andythenorth closed the ticket.

Reason for closing: Won't implement

Mass closure of patch tickets with no commentary for >5 years. Goal is to reduce patch queue as an experiment to see if it aids faster reviewing and rejection/acceptance (it may not). If this offends you and the patch is maintained and compiles with current trunk, discuss with andythenorth in irc. (andythenorth has no ability to review patches but can re-open tickets).

This comment was imported from FlySpray: https://bugs.openttd.org/task/3044

@DorpsGek DorpsGek added flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) component: NewGRF This issue is related to NewGRFs wontfix patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: NewGRF This issue is related to NewGRFs flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@DorpsGek