From 93ad5ec32de124dfa0c054acfd1f2a378cb9ca75 Mon Sep 17 00:00:00 2001 From: Thorbjørn Lindeijer Date: Mon, 7 Oct 2024 12:40:35 +0200 Subject: Optimise PlayerRelationsManager::clear Previous implementation was O(n^2), doing lots of work (saving file and updating UI) for each removed player. --- src/configuration.h | 10 +++++----- src/gui/setup_players.cpp | 19 ++++++------------- src/gui/setup_players.h | 2 +- src/gui/widgets/tablemodel.h | 9 ++++----- src/playerrelations.cpp | 33 ++++++++++++--------------------- src/playerrelations.h | 12 ++++++------ 6 files changed, 34 insertions(+), 51 deletions(-) diff --git a/src/configuration.h b/src/configuration.h index a22eb258..a26c6e20 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -53,7 +53,7 @@ class ConfigurationListManager * \return obj, or otherwise NULL to indicate that this option should * be skipped */ - virtual ConfigurationObject *writeConfigItem(T value, + virtual ConfigurationObject *writeConfigItem(const T &value, ConfigurationObject *obj) = 0; /** @@ -124,7 +124,7 @@ class ConfigurationObject */ template void setList(const std::string &name, IT begin, IT end, - ConfigurationListManager *manager) + ConfigurationListManager &manager) { auto *nextobj = new ConfigurationObject; std::list &list = mContainerOptions[name]; @@ -132,7 +132,7 @@ class ConfigurationObject for (IT it = begin; it != end; it++) { - ConfigurationObject *wrobj = manager->writeConfigItem(*it, nextobj); + ConfigurationObject *wrobj = manager.writeConfigItem(*it, nextobj); if (wrobj) { // wrote something assert (wrobj == nextobj); @@ -160,12 +160,12 @@ class ConfigurationObject * \param manager An object capable of deserialising items into CONT */ template - CONT getList(const std::string &name, CONT empty, ConfigurationListManager *manager) + CONT getList(const std::string &name, CONT empty, ConfigurationListManager &manager) { CONT container = empty; for (auto obj : mContainerOptions[name]) - container = manager->readConfigItem(obj, container); + container = manager.readConfigItem(obj, container); return container; } diff --git a/src/gui/setup_players.cpp b/src/gui/setup_players.cpp index ebe570bb..89c588c0 100644 --- a/src/gui/setup_players.cpp +++ b/src/gui/setup_players.cpp @@ -120,7 +120,7 @@ public: return RELATION_CHOICE_COLUMN_WIDTH; } - virtual void playerRelationsUpdated() + void playerRelationsUpdated() { signalBeforeUpdate(); @@ -303,18 +303,11 @@ void Setup_Players::reset() // We now have to search through the list of ignore choices to find the // current selection. We could use an index into the table of config // options in player_relations instead of strategies to sidestep this. - int selection = 0; - for (unsigned int i = 0; - i < player_relations.getPlayerIgnoreStrategies().size(); - ++i) - if (player_relations.getPlayerIgnoreStrategies()[i] == - player_relations.getPlayerIgnoreStrategy()) - { - - selection = i; - break; - } + const auto &strategies = player_relations.getPlayerIgnoreStrategies(); + auto i = std::find(strategies.begin(), strategies.end(), + player_relations.getPlayerIgnoreStrategy()); + int selection = i == strategies.end() ? 0 : i - strategies.begin(); mIgnoreActionChoicesBox->setSelected(selection); } @@ -401,7 +394,7 @@ void Setup_Players::action(const gcn::ActionEvent &event) } } -void Setup_Players::updatedPlayer(const std::string &name) +void Setup_Players::playerRelationsUpdated() { mPlayerTableModel->playerRelationsUpdated(); mDefaultTrading->setSelected( diff --git a/src/gui/setup_players.h b/src/gui/setup_players.h index 90663029..758c646d 100644 --- a/src/gui/setup_players.h +++ b/src/gui/setup_players.h @@ -48,7 +48,7 @@ public: void action(const gcn::ActionEvent &event) override; - void updatedPlayer(const std::string &name) override; + void playerRelationsUpdated() override; private: StaticTableModel *mPlayerTableTitleModel; diff --git a/src/gui/widgets/tablemodel.h b/src/gui/widgets/tablemodel.h index 2e36992a..d4274e39 100644 --- a/src/gui/widgets/tablemodel.h +++ b/src/gui/widgets/tablemodel.h @@ -77,20 +77,19 @@ public: */ virtual gcn::Widget *getElementAt(int row, int column) const = 0; - virtual void installListener(TableModelListener *listener); - - virtual void removeListener(TableModelListener *listener); + void installListener(TableModelListener *listener); + void removeListener(TableModelListener *listener); protected: /** * Tells all listeners that the table is about to see an update */ - virtual void signalBeforeUpdate(); + void signalBeforeUpdate(); /** * Tells all listeners that the table has seen an update */ - virtual void signalAfterUpdate(); + void signalAfterUpdate(); private: std::set listeners; diff --git a/src/playerrelations.cpp b/src/playerrelations.cpp index 731092f3..fc9c991d 100644 --- a/src/playerrelations.cpp +++ b/src/playerrelations.cpp @@ -39,7 +39,7 @@ class PlayerConfSerialiser : public ConfigurationListManager, std::map *> { - ConfigurationObject *writeConfigItem(std::pair value, + ConfigurationObject *writeConfigItem(const std::pair &value, ConfigurationObject *cobj) override { cobj->setValue(NAME, value.first); @@ -68,8 +68,6 @@ class PlayerConfSerialiser : public ConfigurationListManager= 0) setPlayerIgnoreStrategy(getPlayerIgnoreStrategies()[ignore_strategy_index]); + PlayerConfSerialiser player_conf_serialiser; config.getList, std::map *> - ("player", &mRelations, &player_conf_serialiser); + ("player", &mRelations, player_conf_serialiser); } @@ -141,12 +133,13 @@ void PlayerRelationsManager::init() void PlayerRelationsManager::store() { + PlayerConfSerialiser player_conf_serialiser; config.setList::const_iterator, std::pair, std::map *> ("player", mRelations.begin(), mRelations.end(), - &player_conf_serialiser); + player_conf_serialiser); config.setValue(DEFAULT_PERMISSIONS, mDefaultPermissions); config.setValue(PERSIST_IGNORE_LIST, mPersistIgnores); @@ -156,12 +149,12 @@ void PlayerRelationsManager::store() config.write(); } -void PlayerRelationsManager::signalUpdate(const std::string &name) +void PlayerRelationsManager::signalUpdate() { store(); for (auto listener : mListeners) - listener->updatedPlayer(name); + listener->playerRelationsUpdated(); } unsigned int PlayerRelationsManager::checkPermissionSilently( @@ -226,7 +219,7 @@ void PlayerRelationsManager::setRelation(const std::string &playerName, PlayerRelation::Relation relation) { mRelations[playerName] = PlayerRelation(relation); - signalUpdate(playerName); + signalUpdate(); } std::vector PlayerRelationsManager::getPlayers() const @@ -247,7 +240,7 @@ void PlayerRelationsManager::removePlayer(const std::string &name) if (it != mRelations.end()) { mRelations.erase(it); - signalUpdate(name); + signalUpdate(); } } @@ -272,9 +265,7 @@ unsigned int PlayerRelationsManager::getDefault() const void PlayerRelationsManager::setDefault(unsigned int permissions) { mDefaultPermissions = permissions; - - store(); - signalUpdate(std::string()); + signalUpdate(); } diff --git a/src/playerrelations.h b/src/playerrelations.h index 2a0426a1..e3183da6 100644 --- a/src/playerrelations.h +++ b/src/playerrelations.h @@ -85,7 +85,7 @@ public: PlayerRelationsListener() = default; virtual ~PlayerRelationsListener() = default; - virtual void updatedPlayer(const std::string &name) = 0; + virtual void playerRelationsUpdated() = 0; }; /** @@ -96,7 +96,7 @@ public: class PlayerRelationsManager { public: - PlayerRelationsManager(); + PlayerRelationsManager() = default; ~PlayerRelationsManager(); /** @@ -222,12 +222,12 @@ public: } private: - void signalUpdate(const std::string &name); + void signalUpdate(); - bool mPersistIgnores; // If NOT set, we delete the ignored data upon reloading - unsigned int mDefaultPermissions; + bool mPersistIgnores = false; // If NOT set, we delete the ignored data upon reloading + unsigned int mDefaultPermissions = PlayerRelation::DEFAULT; - PlayerIgnoreStrategy *mIgnoreStrategy; + PlayerIgnoreStrategy *mIgnoreStrategy = nullptr; std::map mRelations; std::list mListeners; std::vector mIgnoreStrategies; -- cgit v1.2.3-70-g09d2