Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Pass interfaces::Node references to OptionsModel constructor #601

Merged
merged 1 commit into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ void BitcoinApplication::createPaymentServer()

void BitcoinApplication::createOptionsModel(bool resetSettings)
{
optionsModel = new OptionsModel(this, resetSettings);
optionsModel = new OptionsModel(node(), this, resetSettings);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that node() asserts for m_node.

}

void BitcoinApplication::createWindow(const NetworkStyle *networkStyle)
Expand Down Expand Up @@ -292,7 +292,6 @@ void BitcoinApplication::createNode(interfaces::Init& init)
{
assert(!m_node);
m_node = init.makeNode();
if (optionsModel) optionsModel->setNode(*m_node);
if (m_splash) m_splash->setNode(*m_node);
}

Expand Down Expand Up @@ -633,6 +632,12 @@ int GuiMain(int argc, char* argv[])
// Allow parameter interaction before we create the options model
app.parameterSetup();
GUIUtil::LogQtInfo();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hunk was moved because now createOptionsModel needs m_node instanced.

if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false))
app.createSplashScreen(networkStyle.data());

app.createNode(*init);

// Load GUI settings from QSettings
app.createOptionsModel(gArgs.GetBoolArg("-resetguisettings", false));

Expand All @@ -641,11 +646,6 @@ int GuiMain(int argc, char* argv[])
app.InitPruneSetting(prune_MiB);
}

if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false))
app.createSplashScreen(networkStyle.data());

app.createNode(*init);

int rv = EXIT_SUCCESS;
try
{
Expand Down
4 changes: 2 additions & 2 deletions src/qt/optionsmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const char *DEFAULT_GUI_PROXY_HOST = "127.0.0.1";

static const QString GetDefaultProxyAddress();

OptionsModel::OptionsModel(QObject *parent, bool resetSettings) :
QAbstractListModel(parent)
OptionsModel::OptionsModel(interfaces::Node& node, QObject *parent, bool resetSettings) :
QAbstractListModel(parent), m_node{node}
{
Init(resetSettings);
}
Expand Down
7 changes: 3 additions & 4 deletions src/qt/optionsmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class OptionsModel : public QAbstractListModel
Q_OBJECT

public:
explicit OptionsModel(QObject *parent = nullptr, bool resetSettings = false);
explicit OptionsModel(interfaces::Node& node, QObject *parent = nullptr, bool resetSettings = false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to not be strictly necessary to have to initialize OptionsModel with a reference to interfaces::Node and instead could just be set at a later point which is similar to how we do it now. Wondering what is the justification to add this to the constructor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, I see how this is needed in consideration of future changes :D


enum OptionID {
StartAtStartup, // bool
Expand Down Expand Up @@ -103,11 +103,10 @@ class OptionsModel : public QAbstractListModel
void setRestartRequired(bool fRequired);
bool isRestartRequired() const;

interfaces::Node& node() const { assert(m_node); return *m_node; }
void setNode(interfaces::Node& node) { assert(!m_node); m_node = &node; }
interfaces::Node& node() const { return m_node; }

private:
interfaces::Node* m_node = nullptr;
interfaces::Node& m_node;
/* Qt-only settings */
bool m_show_tray_icon;
bool fMinimizeToTray;
Expand Down
2 changes: 1 addition & 1 deletion src/qt/test/addressbooktests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)

// Initialize relevant QT models.
std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
OptionsModel optionsModel;
OptionsModel optionsModel(node);
ClientModel clientModel(node, &optionsModel);
WalletContext& context = *node.walletLoader().context();
AddWallet(context, wallet);
Expand Down
7 changes: 3 additions & 4 deletions src/qt/test/optiontests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@

#include <univalue.h>

//! Entry point for BitcoinApplication tests.
void OptionTests::optionTests()
void OptionTests::integerGetArgBug()
{
// Test regression https://github.com/bitcoin/bitcoin/issues/24457. Ensure
// that setting integer prune value doesn't cause an exception to be thrown
Expand All @@ -24,7 +23,7 @@ void OptionTests::optionTests()
settings.rw_settings["prune"] = 3814;
});
gArgs.WriteSettingsFile();
OptionsModel{};
OptionsModel{m_node};
gArgs.LockSettings([&](util::Settings& settings) {
settings.rw_settings.erase("prune");
});
Expand All @@ -49,7 +48,7 @@ void OptionTests::parametersInteraction()
QSettings settings;
settings.setValue("fListen", false);

OptionsModel{};
OptionsModel{m_node};

const bool expected{false};

Expand Down
2 changes: 1 addition & 1 deletion src/qt/test/optiontests.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class OptionTests : public QObject
explicit OptionTests(interfaces::Node& node) : m_node(node) {}

private Q_SLOTS:
void optionTests();
void integerGetArgBug();
void parametersInteraction();

private:
Expand Down
2 changes: 1 addition & 1 deletion src/qt/test/wallettests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ void TestGUI(interfaces::Node& node)
std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
SendCoinsDialog sendCoinsDialog(platformStyle.get());
TransactionView transactionView(platformStyle.get());
OptionsModel optionsModel;
OptionsModel optionsModel(node);
ClientModel clientModel(node, &optionsModel);
WalletContext& context = *node.walletLoader().context();
AddWallet(context, wallet);
Expand Down