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

Gdpr compliance #3748

Merged
merged 4 commits into from
Feb 9, 2019
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
2 changes: 1 addition & 1 deletion src/config/user_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ namespace UserConfigParams
"A random number to avoid duplicated reports.") );

PARAM_PREFIX BoolUserConfigParam m_hw_report_enable
PARAM_DEFAULT( BoolUserConfigParam( true,
PARAM_DEFAULT( BoolUserConfigParam( false,
"hw-report-enabled",
&m_hw_report_group,
"If HW reports are enabled."));
Expand Down
8 changes: 6 additions & 2 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1807,17 +1807,21 @@ void askForInternetPermission()
} // onCancel
}; // ConfirmServer

GUIEngine::ModalDialog *dialog =
MessageDialog *dialog =
new MessageDialog(_("SuperTuxKart may connect to a server "
"to download add-ons and notify you of updates. We also collect "
"anonymous hardware statistics to help with the development of STK. "
"Please read our privacy policy at http://privacy.supertuxkart.net. "
"Would you like this feature to be enabled? (To change this setting "
"at a later time, go to options, select tab "
"'User Interface', and edit \"Connect to the "
"'General', and edit \"Connect to the "
"Internet\" and \"Send anonymous HW statistics\")."),
MessageDialog::MESSAGE_DIALOG_YESNO,
new ConfirmServer(), true, true, 0.7f, 0.7f);

// Changes the default focus to be 'cancel', which is not
Copy link
Collaborator

Choose a reason for hiding this comment

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

You meant that the current "ok" default focus is not compliant ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe so - gdpr requires a deliberate choice by the user to opt-in. With 'ok' being the default, and assuming that just pressing enter is not a deliberate choice (given that opt-out requires a click and pressing enter). IANAL, and if anyone can point out a counter example I am happy to leave the default, I don't like this.
We could add e.g. internet-permission as part of the installation process, but that's a lot of work and only covers a part of our user base :(
TBH, if we agree that we don't need the HW stats (has anyone looked at them in the last two years??), taking this out would be a really good choice imho.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leyyin had disabled stats viewing because it was OOMing the old VPS, but I think since then IIRC the VPS got more available RAM. @leyyin Can you re-enable stats viewing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hiker The comment is wrong, it says the new default is not compliant

Copy link
Member

@vampy vampy Feb 9, 2019

Choose a reason for hiding this comment

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

Unfortunately I can't because the generation script is badly optimized it needs more than 1 GB of ram.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leyyin I thought the VPS got 2GB RAM a little while back as the provider upgraded the plans? Also is the script in a github repo so someone can work on it? Stats are useless if we can't use them, this needs to be fixed.

Or maybe just buy a VPS with more RAM, AFAIK we're not short cash for servers.

Copy link
Member

@vampy vampy Feb 10, 2019

Choose a reason for hiding this comment

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

It got an upgrade to only 1 GB of ram.

As for the scripts, see:

Buying more servers won't be a solution, fixing the code is.
Fix software not buy more hardware ;)

// GDPR compliant, see #3378
dialog->setFocusCancel();
GUIEngine::DialogQueue::get()->pushDialog(dialog, false);
} // askForInternetPermission

Expand Down
14 changes: 10 additions & 4 deletions src/states_screens/dialogs/message_dialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ MessageDialog::MessageDialog(const irr::core::stringw &msg,
float width, float height)
: ModalDialog(width, height)
{
m_msg = msg;
m_type = type;
m_listener = listener;
m_own_listener = own_listener;
m_msg = msg;
m_type = type;
m_listener = listener;
m_own_listener = own_listener;
m_focus_on_cancel = false;
doInit(from_queue);
} // MessageDialog(stringw, type, listener, own_listener)

Expand Down Expand Up @@ -127,12 +128,17 @@ void MessageDialog::loadedFromFile()
{
IconButtonWidget* cancelbtn = getWidget<IconButtonWidget>("cancel");
cancelbtn->setText(_("No"));
if(m_focus_on_cancel)
cancelbtn->setFocusForPlayer(PLAYER_ID_GAME_MASTER);
}
else if (m_type == MessageDialog::MESSAGE_DIALOG_OK_CANCEL)
{
// In case of a OK_CANCEL dialog, change the text from 'Yes' to 'Ok'
IconButtonWidget* yesbtn = getWidget<IconButtonWidget>("confirm");
yesbtn->setText(_("OK"));
IconButtonWidget* cancelbtn = getWidget<IconButtonWidget>("cancel");
if (m_focus_on_cancel)
cancelbtn->setFocusForPlayer(PLAYER_ID_GAME_MASTER);
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/states_screens/dialogs/message_dialog.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ class MessageDialog : public GUIEngine::ModalDialog
irr::core::stringw m_msg;
void doInit(bool from_queue);

/** If set this will set the focus on 'cancel'/'no'
* instead of "yes"/"ok". */
bool m_focus_on_cancel;

public:

/**
Expand Down Expand Up @@ -101,6 +105,10 @@ class MessageDialog : public GUIEngine::ModalDialog
GUIEngine::EventPropagation processEvent(const std::string& eventSource) OVERRIDE;

virtual void loadedFromFile() OVERRIDE;

/** Calling this will make sure that the focus is set on the 'cancel' or
* 'no'. */
void setFocusCancel() {m_focus_on_cancel = true; }
};


Expand Down
25 changes: 17 additions & 8 deletions src/states_screens/options/options_screen_general.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,29 +81,35 @@ void OptionsScreenGeneral::init()
ribbon->setFocusForPlayer(PLAYER_ID_GAME_MASTER);
ribbon->select( "tab_general", PLAYER_ID_GAME_MASTER );

CheckBoxWidget* news = getWidget<CheckBoxWidget>("enable-internet");
assert( news != NULL );
news->setState( UserConfigParams::m_internet_status
CheckBoxWidget* internet_enabled = getWidget<CheckBoxWidget>("enable-internet");
assert( internet_enabled != NULL );
internet_enabled->setState( UserConfigParams::m_internet_status
==RequestManager::IPERM_ALLOWED );
CheckBoxWidget* stats = getWidget<CheckBoxWidget>("enable-hw-report");
assert( stats != NULL );
LabelWidget *stats_label = getWidget<LabelWidget>("label-hw-report");
assert( stats_label );
stats->setState(UserConfigParams::m_hw_report_enable);
stats->setState(UserConfigParams::m_hw_report_enable);

getWidget<CheckBoxWidget>("enable-lobby-chat")
->setState(UserConfigParams::m_lobby_chat);

CheckBoxWidget* chat = getWidget<CheckBoxWidget>("enable-lobby-chat");
LabelWidget* chat_label = getWidget<LabelWidget>("label-lobby-chat");

if(news->getState())
if(internet_enabled->getState())
{
stats_label->setVisible(true);
stats->setVisible(true);
stats->setState(UserConfigParams::m_hw_report_enable);
chat_label->setVisible(true);
chat->setVisible(true);
chat->setState(UserConfigParams::m_lobby_chat);
}
else
{
stats_label->setVisible(false);
stats->setVisible(false);
chat_label->setVisible(false);
chat->setVisible(false);
}
CheckBoxWidget* difficulty = getWidget<CheckBoxWidget>("perPlayerDifficulty");
assert( difficulty != NULL );
Expand Down Expand Up @@ -177,7 +183,7 @@ void OptionsScreenGeneral::eventCallback(Widget* widget, const std::string& name
stats_label->setVisible(true);
stats->setState(UserConfigParams::m_hw_report_enable);
chat->setVisible(true);
stats->setState(UserConfigParams::m_lobby_chat);
chat->setState(UserConfigParams::m_lobby_chat);
chat_label->setVisible(true);
}
else
Expand All @@ -186,6 +192,9 @@ void OptionsScreenGeneral::eventCallback(Widget* widget, const std::string& name
chat_label->setVisible(false);
stats->setVisible(false);
stats_label->setVisible(false);
// Disable this, so that the user has to re-check this if
// enabled later (for GDPR compliance).
UserConfigParams::m_hw_report_enable = false;
PlayerProfile* profile = PlayerManager::getCurrentPlayer();
if (profile != NULL && profile->isLoggedIn())
profile->requestSignOut();
Expand Down