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

stormcone commented Jul 23, 2019

…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

stormcone commented Jul 23, 2019

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 stormcone force-pushed the stormcone:fix-7667 branch from 767c057 to eac394a Jul 23, 2019
@stormcone
Copy link
Contributor Author

stormcone commented Jul 23, 2019

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

@michicc michicc merged commit 2e686ad into OpenTTD:master Jul 23, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190723.2 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
@stormcone stormcone deleted the stormcone:fix-7667 branch Jul 23, 2019
@andythenorth
Copy link
Contributor

andythenorth commented Jul 24, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.