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 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

This comment has been minimized.

Copy link
Member

LordAro commented Jan 31, 2019

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

@andythenorth

This comment has been minimized.

Copy link
Contributor

andythenorth commented Jan 31, 2019

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

This comment has been minimized.

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

This comment has been minimized.

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 label Feb 1, 2019
@michicc

This comment has been minimized.

Copy link
Member

michicc commented Feb 20, 2019

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

@Eddi-z

This comment has been minimized.

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
@Eddi-z Eddi-z force-pushed the Eddi-z:intro branch from 6acd555 to a10d0c5 Feb 20, 2019
@michicc

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

PeterN commented Feb 23, 2019

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

@Eddi-z

This comment has been minimized.

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

This comment has been minimized.

@PeterN

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

andythenorth commented Mar 2, 2019

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 Eddi-z force-pushed the Eddi-z:intro branch from a10d0c5 to 75c77cc Mar 2, 2019
@Eddi-z

This comment has been minimized.

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

This comment has been minimized.

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 Eddi-z force-pushed the Eddi-z:intro branch from 75c77cc to a29b430 Mar 2, 2019
@Eddi-z

This comment has been minimized.

Copy link
Contributor Author

Eddi-z commented Mar 2, 2019

Also decoupled the synchronization between different vehicle types

@PeterN
PeterN approved these changes Mar 2, 2019
@PeterN PeterN merged commit 8139b14 into OpenTTD:master Mar 2, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190302.38 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.8) Linux linux-amd64-clang-3.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
nielsmh added a commit to nielsmh/OpenTTD that referenced this pull request Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.