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 #6599: Can still click on buy button in vehicle selection window even if no vehicle is selected #7049

Closed
wants to merge 2 commits into from

Conversation

@Eddi-z
Copy link
Contributor

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

Patches imported as PR
not implemented or tested by me
minor conflict resolved
Fixes #6599

@nielsmh nielsmh changed the title 6599 Fix #6599: Can still click on buy button in vehicle selection window even if no vehicle is selected Jan 12, 2019
@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Jan 12, 2019

Tested, solves the issues described in #6599, needs a review.

Copy link
Member

@LordAro LordAro left a comment

Can't see anything wrong with the code
Can I be annoying and ask you to capitalise the commit messages properly? (and also fix typos) :>

src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
@Eddi-z Eddi-z force-pushed the Eddi-z:6599 branch from 09fd28c to 9fd7365 Jan 27, 2019
@Eddi-z Eddi-z force-pushed the Eddi-z:6599 branch 2 times, most recently from 9f521b6 to 4aaed36 Jan 27, 2019
@@ -93,7 +94,7 @@ static const CargoID CF_NONE = CT_INVALID; ///< Show only vehicles which do not

Listing _engine_sort_last_sorting[VEH_COMPANY_END] = { { 0, false }, { 0, false }, { 0, false }, { 0, false } };
bool _engine_sort_show_hidden_engines[VEH_COMPANY_END] = {false, false, false, false}; ///< Last set 'show hidden engines' setting for each vehicle type.
static CargoID _engine_sort_last_cargo_criteria[VEH_COMPANY_END] = {CF_ANY, CF_ANY, CF_ANY, CF_ANY}; ///< Last set filter criteria, for each vehicle type.
static byte _engine_sort_last_cargo_criteria[VEH_COMPANY_END] = {CF_ANY, CF_ANY, CF_ANY, CF_ANY}; ///< Last set filter criteria, for each vehicle type.

This comment has been minimized.

@Eddi-z

Eddi-z Jan 27, 2019
Author Contributor

I'm not entirely sure what this change is doing here

@Eddi-z Eddi-z force-pushed the Eddi-z:6599 branch from 4aaed36 to f0118ec Jan 27, 2019
Copy link
Contributor

@nielsmh nielsmh left a comment

The first two changes seem fine, but the remaining four codechanges are fishy and seem to make changes then undo/rework them right afterwards. Can we get just the functional/bugfixing patches in alone, then look at the code quality ones separately?

@PeterN
Copy link
Member

@PeterN PeterN commented Feb 3, 2019

In #6599, this is a problem:

I have something for this, and more.

The sortlist changes might be okay but they're nothing to do with disabling the Buy button.

@Eddi-z Eddi-z force-pushed the Eddi-z:6599 branch from f0118ec to f957413 Feb 10, 2019
@PeterN
Copy link
Member

@PeterN PeterN commented Feb 22, 2019

Already fixed by #7217

@PeterN PeterN closed this Feb 22, 2019
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.

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