gui, refactor: Register Qt meta types in application constructor #19104

1. 
   promag commented at 9:08 am on May 29, 2020: member

   Removes a warning when running QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt.
2. fanquake added the label GUI on May 29, 2020
3. 
   promag commented at 9:10 am on May 29, 2020: member

   Pushed in #17457, then #19033, and now on it’s own PR.
4. 
   jonasschnelli commented at 9:12 am on May 29, 2020: contributor

   We could share those registrations with the main GUI code as also commented here. Since its test only,… not really necessary but would prevent further testing issues.

   utACK 41c3af9fecd52d44fb205edec7c0f054036d41eb

5. 
   promag commented at 9:14 am on May 29, 2020: member

   @jonasschnelli yes, was already suggested by @ryanofsky somewhere and I’ve suggested adding GUIUtils::registerMetaTypes.

   Ah just saw you linked the suggestion comment.

6. 
   promag commented at 10:23 am on May 29, 2020: member

   Moving to GUIUtil::registerMetaTypes, and because of WalletModel* registration, I need to #include <qt/walletmodel.h> which causes

   0A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/guiutil" appears to have been introduced.
   1A new circular dependency in the form of "qt/clientmodel -> qt/guiutil -> qt/walletmodel -> qt/clientmodel" appears to have been introduced.
   2A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/optionsmodel -> qt/guiutil" appears to have been introduced.
   3A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/paymentserver -> qt/guiutil" appears to have been introduced.
   

7. 
   ryanofsky commented at 10:42 am on May 29, 2020: member

   re: #19104 (comment)

   Slight preference for just adding a new file like src/qt/types.h or src/qt/util_types.h to adding new circular dependencies here

8. 
   promag commented at 10:44 am on May 29, 2020: member

   How about BitcoinApplication::registerMetaTypes - maybe called from constructor?
9. 
   ryanofsky commented at 10:50 am on May 29, 2020: member

   How about BitcoinApplication::registerMetaTypes - maybe called from constructor?

   Seems good

10. promag force-pushed on May 29, 2020
11. promag renamed this:
    gui, test: Fix Cannot queue arguments of type size_t
    gui, refactor: Register Qt meta types in application constructor
    on May 29, 2020
12. 
    promag commented at 11:32 am on May 29, 2020: member

    Updated, almost move only.
13. 
    jonasschnelli commented at 11:39 am on May 29, 2020: contributor

    utACK 9df2528e57ff3c22f3d5777eb6a60a326c909995
14. 
    hebasto commented at 11:45 am on May 29, 2020: member

    Concept ACK.
15. in src/qt/bitcoin.cpp:257 in 9df2528e57 outdated

    253@@ -253,6 +254,23 @@ bool BitcoinApplication::baseInitialize()
    254     return m_node.baseInitialize();
    255 }
    256 
    257+void BitcoinApplication::registerMetaTypes()
    

    ---

    ---

    

    hebasto commented at 2:54 pm on May 29, 2020:

    Why a member function? Could it be a non-member function, either in unnamed namespace or static?

    ---

    

    promag commented at 3:06 pm on May 29, 2020:

    Good question 🤷 updated.
16. hebasto changes_requested
17. 
    hebasto commented at 2:54 pm on May 29, 2020: member

    Tested 9df2528e57ff3c22f3d5777eb6a60a326c909995 on macOS 10.15.5 – works as expected.
18. gui, refactor: Register Qt meta types in application constructor 4f49d5222e
19. promag force-pushed on May 29, 2020
20. hebasto approved
21. 
    hebasto commented at 3:16 pm on May 29, 2020: member

    ACK 4f49d5222eca11c149713ad34113d5a3d1c577b1, tested on macOS 10.15.5.
22. 
    DrahtBot commented at 4:35 pm on May 29, 2020: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #19011 (Reduce cs_main lock accumulation during GUI startup by jonasschnelli)
    • #18817 (doc: Document differences in bitcoind and bitcoin-qt locale handling by practicalswift)
    • #18604 (Display command line usage to console without requiring X Windows by rebroad)
    • #17877 (qt, refactor: Make enums in BitcoinUnits class scoped by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

23. 
    jonasschnelli commented at 6:47 am on June 2, 2020: contributor

    Re utACK 4f49d5222eca11c149713ad34113d5a3d1c577b1
24. jonasschnelli merged this on Jun 2, 2020
25. jonasschnelli closed this on Jun 2, 2020

    ---

26. sidhujag referenced this in commit 042590c325 on Jun 2, 2020
27. promag deleted the branch on Jun 7, 2020
28. ComputerCraftr referenced this in commit d30a975a2c on Jun 10, 2020
29. Fabcien referenced this in commit 8744502667 on Jul 6, 2021
30. DrahtBot locked this on Feb 15, 2022

Contributors
promag jonasschnelli ryanofsky hebasto DrahtBot

Labels
GUI