HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...
Feature #903720
Qt: added trade information columns to goto dialog
0%
Description
Please review and advise.
Related issues
History
#1
Updated by Marko Lindqvist over 1 year ago
- Category set to gui-qt
CodingStyle:
"- When declaring a pointer, there should be a space before '*' and no space
after, except if it is a second '*'."
"- Braces shall always be used after conditionals, loops, etc.:"
while (foo) bar;
->
while (foo) {
bar;
}
In gotodlg.h includes are not in alphabetical order within the // common block
Within some 'case's indentation is off
#2
Updated by John Robertson over 1 year ago
- File .editorconfig .editorconfig added
If beneficial, here is a VisualStudio configuration file that matches the freeciv coding style guides. Place in the root source folder then open the folder in MSVC.
#3
Updated by John Robertson over 1 year ago
- File 0001-Qt-added-trade-related-columns-to-the-goto-dialog.patch 0001-Qt-added-trade-related-columns-to-the-goto-dialog.patch added
This one follows the coding standards and enhances the new Trade column. The patch is a little bigger because I needed a 'RDIR_NONE', or an empty or no direction for the trade enumerations.
(I'll start working on the S3_0 patch, ...)
#4
Updated by Marko Lindqvist over 1 year ago
John Robertson wrote:
I needed a 'RDIR_NONE'
That's a network protocol change (especially as the values of the existing enum entries change) ...
(I'll start working on the S3_0 patch, ...)
...so it might not be worth all the trouble of porting to S3_0 (where protocol has been frozen), with the need of network capabilities code and stuff.
#5
Updated by Marko Lindqvist over 1 year ago
- Related to Feature #905523: Document that enum route_direction is part of network protocol added
#6
Updated by John Robertson over 1 year ago
I have moved RDIR_NONE to the end with a value of 3 instead of at the beginning of the enumeration with a value of zero. This should preserve the network protocol.
But my testing is getting a SEGSEGV calling the c++ delete for the QPixmap of the sprite. The pointer is valid, the destructor completes and spills over into the library free() call correctly. I probably need to recompile Qt5 for Msys2, assuming there is a problem in the default Msys package.
#7
Updated by John Robertson over 1 year ago
@Marko, I have a small related change that updates the trade calculations as the selected unit changes. Do you want me to roll it into an inclusive patch for master and S3_0 or an independent patch?
#8
Updated by Marko Lindqvist over 1 year ago
John Robertson wrote:
@Marko, I have a small related change that updates the trade calculations as the selected unit changes. Do you want me to roll it into an inclusive patch for master and S3_0 or an independent patch?
Independent patch
#9
Updated by Marko Lindqvist over 1 year ago
- Sprint/Milestone set to 3.1.0
- There's still while() loops in one line instead of having proper code block
- Handling of that new RDIR_NONE in various switch:es need to be more than "break;" - e.g. sanity check module should fail if some traderoute actually uses it, many places should fc_assert() against it
#10
Updated by Marko Lindqvist 4 months ago
John Robertson wrote:
But my testing is getting a SEGSEGV calling the c++ delete for the QPixmap of the sprite. The pointer is valid, the destructor completes and spills over into the library free() call correctly. I probably need to recompile Qt5 for Msys2, assuming there is a problem in the default Msys package.
Ping. I'm going to test this myself anyway, but was this ever resolved?
#11
Updated by Marko Lindqvist 4 months ago
- File 0035-Qt-Add-trade-related-columns-to-the-goto-dialog.patch 0035-Qt-Add-trade-related-columns-to-the-goto-dialog.patch added
- Status changed from New to Resolved
- Assignee set to Marko Lindqvist
Made an updated patch myself.
Marko Lindqvist wrote:
- There's still while() loops in one line instead of having proper code block
These, and some other minor style issues I noticed, fixed
- Handling of that new RDIR_NONE in various switch:es need to be more than "break;" - e.g. sanity check module should fail if some traderoute actually uses it, many places should fc_assert() against it
Sanity check made to fail, fc_assert() added to one place.
I have not yet tested on msys2 to see if this has issue there.
#12
Updated by Marko Lindqvist 4 months ago
- Status changed from Resolved to Closed
.