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

Change: Synchronize randomness in vehicle introduction… #7147

Merged
merged 1 commit into from Mar 2, 2019

Conversation

Eddi-z
Copy link
Contributor

@Eddi-z Eddi-z commented Jan 30, 2019

...so all vehicles introduced simultaneously will stay at the same date

we could add a NewGRF flag somewhere to toggle old vs. new behaviour, but for trains misc_flags (prop 27) is already full. the others have a few "reserved, do not use" bits.

@LordAro
Copy link
Member

LordAro commented Jan 31, 2019

I don't quite follow what the purpose of this is. Why the change?

@andythenorth
Copy link
Contributor

I don't quite follow what the purpose of this is. Why the change?

There are (for trains) newgrf vehicles that make no sense without other vehicles. E.g. Some engines need specific wagons, but current randomisation can't force them to be introduced simultaneously.

Not a showstopper, but common enough that many train grf authors seem to run into it.

@Eddi-z
Copy link
Contributor Author

Eddi-z commented Jan 31, 2019

A side effect of this is that intro dates won't be rerandomized when using "resetengines" or "reload_newgrfs" commands

@PeterN
Copy link
Member

PeterN commented Feb 1, 2019

Can we get a consensus from set authors as to whether this change is desirable across the board?

@LordAro LordAro added the needs triage This issue needs further investigation before it becomes actionable label Feb 1, 2019
@michicc
Copy link
Member

michicc commented Feb 20, 2019

Set authors apparently don't follow GitHub. I like it anyway.

@Eddi-z
Copy link
Contributor Author

Eddi-z commented Feb 20, 2019

that's why we should put stuff like this in the dev diary, it'll hopefully get more feedback that way

src/engine.cpp Outdated Show resolved Hide resolved
@michicc
Copy link
Member

michicc commented Feb 23, 2019

As a side effect you are also influencing the base reliability values by this, as they are inside the changed random seeds. It would be easy to switch the order around to restore the current behaviour.
Some sets like the DBSetXL use reliability to some extent to guide the choice on similar engines, but then I didn't check if any of these have in fact the same intro date. The comment about the reset/restart does apply though.

@Eddi-z
Copy link
Contributor Author

Eddi-z commented Feb 23, 2019

As a side effect you are also influencing the base reliability values by this, as they are inside the changed random seeds.

I'm not convinced this is a bad thing, though

@PeterN
Copy link
Member

PeterN commented Feb 23, 2019

@andythenorth mentioned exclusive engine previews. Do they need to be taken into account?

@Eddi-z
Copy link
Contributor Author

Eddi-z commented Feb 23, 2019

I haven't changed anything about previews, so what probably happens when several non-wagon vehicles get introduced is:

  • the preview period starts as the same time, so likely the same company is chosen to offer the preview (i'm not sure how the exact algorithm works, but i think it has to do with company rating?)
  • that company has a bunch of preview windows opening at the same time, and can separately choose to accept them
  • any non-accepted previews will be passed on to other eligible companies
  • for the preview to be successful, the comany must purchase each of the (possibly very similar) vehicles separately

changing this behavior is IMHO out of scope for this PR, and requires more sophisticated methods e.g. to combine several vehicles into one single purchase menu entry

@PeterN PeterN added this to the 1.9.0 milestone Mar 1, 2019
@andythenorth
Copy link
Contributor

@PeterN
Copy link
Member

PeterN commented Mar 2, 2019

Yes, reliability will still be random, at least per game seed. Could be put back by moving the RestoreRandomSeeds after the first Random(), I suppose.

@andythenorth
Copy link
Contributor

I have tested this with Iron Horse 2 Alpha 7 (from bananas). Works as expected.

Test examples:

  • synchronised introduction of Helm Wind cab car and middle car (action 0 intro date is 01-01-1987)
  • synchronised introduction of all generation 5 wagons (action 0 intro date is 01-01-1990)

@Eddi-z
Copy link
Contributor Author

Eddi-z commented Mar 2, 2019

I've now mangled the seed with the GRFID, so it should not synchronize between different GRFs anymore (maybe addon-GRFs need special treatment?)

@Eddi-z
Copy link
Contributor Author

Eddi-z commented Mar 2, 2019

The synchronization should also show with the default vehicles (trucks, mostly)

…s introduced simultaneously will stay at the same date
@Eddi-z
Copy link
Contributor Author

Eddi-z commented Mar 2, 2019

Also decoupled the synchronization between different vehicle types

@PeterN PeterN merged commit 8139b14 into OpenTTD:master Mar 2, 2019
nielsmh pushed a commit to nielsmh/OpenTTD that referenced this pull request Mar 11, 2019
douiwby pushed a commit to douiwby/OpenTTD that referenced this pull request Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue needs further investigation before it becomes actionable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants