Alberth wrote:

Hi,

We've had a little discussion about your patch. There are two things that need
improvement. Unfortunately, it means a complete rewrite of the patch.

- The logic of accepting and not-accepting a cargo type should be separated
from 'speed', a cargo type always has a speed that gets updated, no matter
whether you accept a cargo type or not (look for
"_settings_game.order.selectgoods" in station_cmd.cpp, it's around line 3511).
- The current CTRL+click is way too obscure for users. Even I did not find all
settings, even though I knew which ones existed.

The former is mostly splitting various uses of variables into separate
variables (eg from the special case "last_speed == 0" to a speed variable and
an acceptance variable. The latter can be solved by making a GUI for cargo
acceptance (probably in the station window). Each cargo would have a 'accept',
'accept when there is demand' and 'do not accept' setting which can be set with
yellow [<][>] buttons, like in the advanced settings window. Being able to set
a default seems useful too.

Note that the TODO page has been extended with more detailed information too.

Details about your current patch:
In general a very nice patch, the code seems a bit lengthy at some places (the
end of CmdChangeStationAcceptance can be written much more compact), but there
are no real serious problems with the code style. Below are some suggestions to
watch out for.

- Use an english dictionary to check your comments. Good editors allow
spell-checking of comments too.

- Doxygen headers start with "/**" and have no empty line between the comment
and the function (see station_cmd.cpp)

- Descriptions like "+ * @param waitind Click happen in WaitingCargo list or
not." are wrong. Texts should explain meaning of a value to the function (for
example "if true, the function fails"), not how you get its value ("you
pressed a button")

- "} else {" should be written at one line.

- Watch out for trailing white space (in particular at otherwise completely
empty lines). Have the editor display them clearly so you can actually see
where they are.

- C-style comment must be on a line all by itself (that is, not behind any code).

This comment was imported from FlySpray: https://bugs.openttd.org/task/5175#comment11230

dadymax wrote:

Thanks for general accept idea.
And thanks for very detailed review and things to improve. I have very short coding skills but I want to improve.

Unfortunately on my work I now have task that needs knowing of C# not C++. But I try to do my best for this great game and great open project.

P.S. And I use online translator very often (:

This comment was imported from FlySpray: https://bugs.openttd.org/task/5175#comment11233

andythenorth closed the ticket.

Reason for closing: Won't implement

Mass closure of patch tickets with no commentary for >5 years. Goal is to reduce patch queue as an experiment to see if it aids faster reviewing and rejection/acceptance (it may not). If this offends you and the patch is maintained and compiles with current trunk, discuss with andythenorth in irc. (andythenorth has no ability to review patches but can re-open tickets).

This comment was imported from FlySpray: https://bugs.openttd.org/task/5175