bitcoin-core / gui-qml

Bitcoin GUI (experimental QML-based fork)
MIT License
106 stars 40 forks source link

Using a different configuration file name from QT #398

Closed pablomartin4btc closed 2 months ago

pablomartin4btc commented 3 months ago

At the moment both QT and QML gui apps are using the same naming convention for configuration files (Bitcoin*.conf), so separating guiconstants.h file from QT, copying it from src/qt to src/qml and updating QAPP_APP_NAME_* constants values will avoid them clashing with each other trying to persist/ read same or different settings (e.g. configuration file for QT on signet will be still named as Bitcoin-Qt-signet.conf, as of today, while QML will start using a separate file named Bitcoin-Qml-signet.conf - before this fix, currently sometimes a user can get a warning on reading incorrect settings or values: QVariant::load: unknown user type with name BitcoinUnits::Unit.).

This could be a temporary fix (? - gui constants file contents has been cleaned up as suggested) so instances from both QT and QML gui apps don't interfere between them during QML development. This change will be transparent for both QT gui app and users.

Sample of a separate QT config file on signet (Bitcoin-Qt-signet.conf). ``` [General] DisplayBitcoinUnit=@Variant(\0\0\0\x7f\0\0\0\x13\x42itcoinUnits::Unit\0\0) MainWindowGeometry=@ByteArray(\x1\xd9\xd0\xcb\0\x3\0\0\0\0\x2\xa3\0\0\x1v\0\0\x5\xb0\0\0\x3^\0\0\x2\xa3\0\0\x1v\0\0\x5\xb0\0\0\x3^\0\0\0\0\0\0\0\0\a\x80\0\0\x2\xa3\0\0\x1v\0\0\x5\xb0\0\0\x3^) PeersTabBanlistHeaderState=@ByteArray() PeersTabPeerHeaderState=@ByteArray() RPCConsoleWindowGeometry=@ByteArray(\x1\xd9\xd0\xcb\0\x3\0\0\0\0\x2q\0\0\x1\x8f\0\0\x5T\0\0\x3<\0\0\x2q\0\0\x1\x8f\0\0\x5T\0\0\x3<\0\0\0\0\0\0\0\0\a\x80\0\0\x2q\0\0\x1\x8f\0\0\x5T\0\0\x3<) RPCConsoleWindowPeersTabSplitterSizes=@ByteArray(\0\0\0\xff\0\0\0\x1\0\0\0\x2\xff\xff\xff\xff\xff\xff\xff\xff\0\xff\xff\xff\xff\x1\0\0\0\x1\0) SubFeeFromAmount=false enable_psbt_controls=false fCoinControlFeatures=false fHideTrayIcon=false fMinimizeOnClose=false fMinimizeToTray=false fRestartRequired=false nSettingsVersion=279900 strDataDir=/home/pablo/.bitcoin strThirdPartyTxUrls= ```
Sample of a separate QML config file on signet (Bitcoin-Qml-signet.conf). ``` [General] blockclocksize=0.4166666666666667 dark=true height=665 width=640 x=755 y=339 ```
pablomartin4btc commented 2 months ago

Concept ACK. Wonder if we need all of the other constants with the new application?

I haven't investigated it yet, the main purpose for this change was to get the config file separate from QT and I clarified at the top that we could do the cleanup later, in case there are features that we don't have/ use in QML at the moment but we'll need them later, but I'm open, we could do the cleanup now and bring the constants back as soon as we need them (if we ever need them).

johnny9 commented 2 months ago

I tried deleting everything else from that file and the only constant being used is DEFAULT_SPLASHSCREEN. I think we should only keep the constants we are using, I don't think its a good idea to add lines of code that aren't used.

so something like

// Copyright (c) 2011-2024 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_QML_GUICONSTANTS_H
#define BITCOIN_QML_GUICONSTANTS_H

static const bool DEFAULT_SPLASHSCREEN = true;

#define QAPP_ORG_NAME "Bitcoin"
#define QAPP_ORG_DOMAIN "bitcoin.org"
#define QAPP_APP_NAME_DEFAULT "Bitcoin-Qml"
#define QAPP_APP_NAME_TESTNET "Bitcoin-Qml-testnet"
#define QAPP_APP_NAME_SIGNET "Bitcoin-Qml-signet"
#define QAPP_APP_NAME_REGTEST "Bitcoin-Qml-regtest"

#endif // BITCOIN_QML_GUICONSTANTS_H
pablomartin4btc commented 2 months ago

DEFAULT_SPLASHSCREEN

I think not even that one, all command line args described in bitcoin.cpp::SetupUIArgs are useless (so far), but regarding the splash-screen (which I mentioned in the wallet intro PR review), @GBKS confirmed, on a designers call a couple of weeks ago, that we are not using that on this project, it's an old fashion approach nowadays.

I'll update the file soon. Thanks!

pablomartin4btc commented 2 months ago

Updates:

pablomartin4btc commented 2 months ago

Updates: