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

Remember user inputs after failed registration #4982 #4983

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions cockatrice/src/abstractclient.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class Event_ConnectionClosed;
class Event_ServerShutdown;
class Event_ReplayAdded;
class FeatureSet;
class DlgRegister;

enum ClientStatus
{
Expand All @@ -45,6 +46,7 @@ enum ClientStatus

class AbstractClient : public QObject
{
friend class DlgRegister;
Q_OBJECT
signals:
void statusChanged(ClientStatus _status);
Expand Down
26 changes: 18 additions & 8 deletions cockatrice/src/dlg_register.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "dlg_register.h"

#include "abstractclient.h"
#include "pb/serverinfo_user.pb.h"
#include "settingscache.h"
#include "stringsizes.h"
Expand All @@ -12,7 +13,8 @@
#include <QLabel>
#include <QMessageBox>

DlgRegister::DlgRegister(QWidget *parent) : QDialog(parent)
DlgRegister::DlgRegister(QWidget *parent, AbstractClient *abstractClient) : QDialog(parent)

{
ServersSettings &servers = SettingsCache::instance().servers();
infoLabel = new QLabel(tr("Enter your information and the information of the server you'd like to register to.\n"
Expand All @@ -30,29 +32,30 @@ DlgRegister::DlgRegister(QWidget *parent) : QDialog(parent)
portLabel->setBuddy(portEdit);

playernameLabel = new QLabel(tr("Player &name:"));
playernameEdit = new QLineEdit(servers.getPlayerName());
playernameEdit =
new QLineEdit(servers.getPlayerName().isEmpty() ? abstractClient->getUserName() : servers.getPlayerName());
playernameEdit->setMaxLength(MAX_NAME_LENGTH);
playernameLabel->setBuddy(playernameEdit);

passwordLabel = new QLabel(tr("P&assword:"));
passwordEdit = new QLineEdit();
passwordEdit = new QLineEdit(abstractClient->password);
Copy link
Member

@ebbit1q ebbit1q Jan 16, 2024

Choose a reason for hiding this comment

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

why not instead of all this messing around with accessing the client's private fields just pass the needed information to the dialog's constructor?

Copy link
Author

Choose a reason for hiding this comment

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

why not instead of all this messing around with accessing the client's private fields just pass the needed information to the dialog's constructor?

sure, that would work. Pretty sure I wanted to do it that way as well, cannot remember exactly why I decided to stop and try a different way. I believe we would need to modify the remote client (or the abstract client it inherits from) so that the window_main file could access those values.

So I think in the end we would still need to modify some class to give permission to another class to access its data

A better solution still would be to keep the dialogue open and not destroy it until the user has exited out of the flow by either canceling or quitting the program or successfully registering. Somewhere, there are some signals that cause the dialogue register to close. There's gotta be a failure one and a success one and maybe one or two others for errors or who knows what. We would need to find all of those slots that are listening for those signals and modify them to not close the dialogue DlgRegister if the user encounters a registration error. I also think just hitting the submit button on the registration dialogue closes the registration dialogue, so that one would need to be modified as well. This would definitely be the best approach if I could pull it off though because then we wouldn't need to modify any of the accessibility of the class attributes.

Because by simply having the same dialogue stay open, it would remember what its values are, since it never would've closed in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

I've been meaning to do that yet still haven't found the time to sit down and get it all sorted, it needs a lot of signals and stuff moved #4586

my comment here is just regarding the arguments in the constructor of the dialog, no changes to the header of the client will be required, just pass the values when the dialog is instantiated.

Copy link
Author

Choose a reason for hiding this comment

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

Sounds like an easy change to make then @ebbit1q . RAII is good. I hope to get around to it not too long from now. Thanks for your advice

Copy link
Author

Choose a reason for hiding this comment

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

passwordEdit->setMaxLength(MAX_NAME_LENGTH);
passwordLabel->setBuddy(passwordEdit);
passwordEdit->setEchoMode(QLineEdit::Password);

passwordConfirmationLabel = new QLabel(tr("Password (again):"));
passwordConfirmationEdit = new QLineEdit();
passwordConfirmationEdit = new QLineEdit(abstractClient->password);
passwordConfirmationEdit->setMaxLength(MAX_NAME_LENGTH);
passwordConfirmationLabel->setBuddy(passwordConfirmationEdit);
passwordConfirmationEdit->setEchoMode(QLineEdit::Password);

emailLabel = new QLabel(tr("Email:"));
emailEdit = new QLineEdit();
emailEdit = new QLineEdit(abstractClient->email);
emailEdit->setMaxLength(MAX_NAME_LENGTH);
emailLabel->setBuddy(emailEdit);

emailConfirmationLabel = new QLabel(tr("Email (again):"));
emailConfirmationEdit = new QLineEdit();
emailConfirmationEdit = new QLineEdit(abstractClient->email);
emailConfirmationEdit->setMaxLength(MAX_NAME_LENGTH);
emailConfirmationLabel->setBuddy(emailConfirmationEdit);

Expand Down Expand Up @@ -311,13 +314,20 @@ DlgRegister::DlgRegister(QWidget *parent) : QDialog(parent)
countryEdit->addItem(QPixmap("theme:countries/za"), "za");
countryEdit->addItem(QPixmap("theme:countries/zm"), "zm");
countryEdit->addItem(QPixmap("theme:countries/zw"), "zw");
countryEdit->setCurrentIndex(0);

int initialCountryIndex = countryEdit->findText(abstractClient->country);
if (initialCountryIndex != -1) {
countryEdit->setCurrentIndex(initialCountryIndex);
} else {
countryEdit->setCurrentIndex(0);
}

QStringList countries = SettingsCache::instance().getCountries();
foreach (QString c, countries)
countryEdit->addItem(QPixmap("theme:countries/" + c.toLower()), c);

realnameLabel = new QLabel(tr("Real name:"));
realnameEdit = new QLineEdit();
realnameEdit = new QLineEdit(abstractClient->realName);
realnameEdit->setMaxLength(MAX_NAME_LENGTH);
realnameLabel->setBuddy(realnameEdit);

Expand Down
3 changes: 2 additions & 1 deletion cockatrice/src/dlg_register.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
class QLabel;
class QPushButton;
class QCheckBox;
class AbstractClient;

class DlgRegister : public QDialog
{
Q_OBJECT
public:
DlgRegister(QWidget *parent = nullptr);
DlgRegister(QWidget *parent = nullptr, AbstractClient *abstractClient = nullptr);
QString getHost() const
{
return hostEdit->text();
Expand Down
1 change: 1 addition & 0 deletions cockatrice/src/remoteclient.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <QWebSocket>

class QTimer;
class DlgRegister;

class RemoteClient : public AbstractClient
{
Expand Down
2 changes: 1 addition & 1 deletion cockatrice/src/window_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ void MainWindow::actConnect()

void MainWindow::actRegister()
{
DlgRegister dlg(this);
DlgRegister dlg(this, client);
if (dlg.exec()) {
client->registerToServer(dlg.getHost(), static_cast<unsigned int>(dlg.getPort()), dlg.getPlayerName(),
dlg.getPassword(), dlg.getEmail(), dlg.getCountry(), dlg.getRealName());
Expand Down