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 #7667: Buying an engine after buying wagons doesn't give a comple… #7668

Merged
merged 1 commit into from Jul 23, 2019

Conversation

stormcone
Copy link
Contributor

…te train.

This commit sets the 'data' argument to it's original value as it was before d54b6ac.

@michicc
Copy link
Member

michicc commented Jul 23, 2019

I really don't think this is right. Changing it to GB(p1, 16, 8) OTOH would make it match the comment for p1.

@stormcone
Copy link
Contributor Author

I'm not sure which comment you mean. In CmdBuildVehicle:

* @param p1 various bitstuffed data
* bits 0-15: vehicle type being built.
* bits 16-23: vehicle type specific bits passed on to the vehicle build functions.
* bits 24-31: refit cargo type.

it says bits 0..15.

In CmdBuild<VehicleType> functions 'data' is 'unused', except the case of trains, where it says:

* @param data bit 0 prevents any free cars from being added to the train.

So actually we only need CmdBuildVehicle's p1's bit 0.

OTOH, originally it was GB(p1, 16, 16), so that's why I chose this again.

@michicc
Copy link
Member

michicc commented Jul 23, 2019

I mean this one:

* bits 16-23: vehicle type specific bits passed on to the vehicle build functions.

In my understanding the 'vehicle type specific bits' are exactly what should be passed to CmdBuild<VehicleType>. And incidentally, the commit you've quoted changed that data from bits 16-31 to only bits 16-23. I.e. start bit 16, length 8 -> GB(p1, 16, 8).

@stormcone
Copy link
Contributor Author

Now I see, you are right. I updated the commit.

@michicc michicc merged commit 2e686ad into OpenTTD:master Jul 23, 2019
@stormcone stormcone deleted the fix-7667 branch July 23, 2019 21:21
@andythenorth
Copy link
Contributor

Thanks!

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

3 participants