#45892: Ruledit: Counters tab
Open Date: 2022-10-17 20:20
Last Update: 2023-01-22 03:26
URL for this Ticket:
https://osdn.net//projects/freeciv/ticket/45892
RSS feed for this Ticket:
https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=45892
---------------------------------------------------------------------
Last Changes/Comment on this Ticket:
2023-01-22 03:26 Updated by: lachu
Comment:
Reply To cazfi
Reply To lachu
Reply To cazfi
- QRegExp is deprecated, and available to Qt6-client only via Qt5compat: https://doc.qt.io/qt-6/qregexp.html
1) There's no need to have Qt5 and Qt6 implementation to differ here by using QRegExp on Qt5, as the replacements exist in Qt-5.11 already.
2) This is used to validate that a string is actually a number. You should use widget that only accepts numbers in the first place. Likely QSpinBox
- Only "Add" and "Delete" would need to be limited to experimental builds only, I think. We can have safely have editing of the other values in normal build
- Put them to the last row, not in the middle.
- While adjusting that, use a 'row' counter to place widgets, instead of maintaining plain row numbers in the source code (forcing developer to update all of them when there is any change)
- You should iterate over all counter types instead of making the assumption that there's just two specific types (assumption that would break as soon as #45489 gets implemented)
Still not done
- Many unnecessary empty lines where they should not be
- Still there
- There's also empty lines missing where they should be (between variable declarations and the code)
- Do not use atoi(), but something more secure, e.g., str_to_int() or str_to_uint()
Improved a bit by using strtol(), but still not str_to_int()
- You should check is_counter_needed() before deleting it
This should use 'requirers_dlg' to give proper feedback to the user. See how the same thing is implemented in the other tabs
+ }
+ else {
- Should be on one line
- The toggle to make "name" same as "rule name" is not there
- tab_counter::counter_name() is not needed
- There's no effect editing button. Slot edit_effects() exist, but is never signaled
- I'd prefer that "Counters" tab would be before the "Nations" tab (that the "Nations" tab with not much functionality would be the last)
0001-OSDN-TICKET-45892-S-awomir-Lach-slawek-lach.art.pl.patch(17KB)
I missed int
Fast fix. I missed int in signal/slot reference, so it do not update value. I do not test this, because I stood my changes do not touch this functionality.
---------------------------------------------------------------------
Ticket Status:
Reporter: cazfi
Owner: (None)
Type: Patches
Status: Open
Priority: 5 - Medium
MileStone: 3.2.0
Component: Ruledit and ruleup
Severity: 5 - Medium
Resolution: None
---------------------------------------------------------------------
Ticket details:
Ruledit should have tab for counters editing. Initial version can have as little functionality as needed to make it to provide any value at all (likely it's easy to get functionality common to most ruleset objects by simply copying, e.g., extras tab and modify that)
Any counters removal functionality (and basically adding too, as that would be unreversable action without ability to remove the counter) depends on #45041
--
Ticket information of Freeciv project
Freeciv Project is hosted on OSDN
Project URL: https://osdn.net/projects/freeciv/
OSDN: https://osdn.net
URL for this Ticket:
https://osdn.net/projects/freeciv/ticket/45892
RSS feed for this Ticket:
https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=45892