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

Patch to make filter_funcs members of BuildVehicleWindow #5695

Closed
DorpsGek opened this issue Aug 4, 2013 · 3 comments
Closed

Patch to make filter_funcs members of BuildVehicleWindow #5695

DorpsGek opened this issue Aug 4, 2013 · 3 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Aug 4, 2013

Vaulter opened the ticket and wrote:

Subj.
I see that this one filter func dont use anything window class specific, but... it is just

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/5695
@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 2, 2017

Alberth wrote:

Just moving a single static function into a class in the same file doesn't help much, given that static functions are common in OpenTTD (and perhaps even in C++, as not everything has to be a method or part of a class in that language).

The patch also breaks the general pattern of all the other filter functions, which makes it less easy to find and re-use if the need arises.

In other words, while the thought is alright, the scale is too small.

If you want to move these functions, it needs a new general pattern of handling these filter functions (or handling filtered lists, or something else, where the filter functions are apart of).
From such a new pattern you can then change all filter functions, and improve the general structure.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5695#comment14722

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 9, 2017

andythenorth wrote:

Close if no reply by end of 2017. Thanks.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5695#comment14761

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
@andythenorth
Copy link
Contributor

Ok it's 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

3 participants