From 6f8e30c55df8dead8e53161cf295e31b96caa7f2 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Wed, 4 Dec 2024 12:18:34 +0000 Subject: [PATCH] Codechange: Use unique_ptr throughout instead of new raw pointer for company news data. (#13148) The pointer was already captured and converted to a unqiue_ptr, but hidden within the call stack. This now makes it clearer that the object passed to Add.*NewsItem will become owned by the news item. --- src/company_cmd.cpp | 8 ++++---- src/economy.cpp | 8 ++++---- src/news_func.h | 14 +++++++------- src/news_gui.cpp | 8 ++++---- src/news_type.h | 4 ++-- src/town_cmd.cpp | 4 ++-- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp index 83aa4d2dd8..25bdf33287 100644 --- a/src/company_cmd.cpp +++ b/src/company_cmd.cpp @@ -426,12 +426,12 @@ set_name:; MarkWholeScreenDirty(); if (c->is_ai) { - CompanyNewsInformation *cni = new CompanyNewsInformation(c); + auto cni = std::make_unique(c); SetDParam(0, STR_NEWS_COMPANY_LAUNCH_TITLE); SetDParam(1, STR_NEWS_COMPANY_LAUNCH_DESCRIPTION); SetDParamStr(2, cni->company_name); SetDParam(3, t->index); - AddNewsItem(STR_MESSAGE_NEWS_FORMAT, NT_COMPANY_INFO, NF_COMPANY, NR_TILE, c->last_build_coordinate.base(), NR_NONE, UINT32_MAX, cni); + AddNewsItem(STR_MESSAGE_NEWS_FORMAT, NT_COMPANY_INFO, NF_COMPANY, NR_TILE, c->last_build_coordinate.base(), NR_NONE, UINT32_MAX, std::move(cni)); } return; } @@ -946,13 +946,13 @@ CommandCost CmdCompanyCtrl(DoCommandFlag flags, CompanyCtrlAction cca, CompanyID if (!(flags & DC_EXEC)) return CommandCost(); - CompanyNewsInformation *cni = new CompanyNewsInformation(c); + auto cni = std::make_unique(c); /* Show the bankrupt news */ SetDParam(0, STR_NEWS_COMPANY_BANKRUPT_TITLE); SetDParam(1, STR_NEWS_COMPANY_BANKRUPT_DESCRIPTION); SetDParamStr(2, cni->company_name); - AddCompanyNewsItem(STR_MESSAGE_NEWS_FORMAT, cni); + AddCompanyNewsItem(STR_MESSAGE_NEWS_FORMAT, std::move(cni)); /* Remove the company */ ChangeOwnershipOfCompanyItems(c->index, INVALID_OWNER); diff --git a/src/economy.cpp b/src/economy.cpp index cc38b0468f..d72056fc56 100644 --- a/src/economy.cpp +++ b/src/economy.cpp @@ -597,11 +597,11 @@ static void CompanyCheckBankrupt(Company *c) /* Warn about bankruptcy after 3 months */ case 4: { - CompanyNewsInformation *cni = new CompanyNewsInformation(c); + auto cni = std::make_unique(c); SetDParam(0, STR_NEWS_COMPANY_IN_TROUBLE_TITLE); SetDParam(1, STR_NEWS_COMPANY_IN_TROUBLE_DESCRIPTION); SetDParamStr(2, cni->company_name); - AddCompanyNewsItem(STR_MESSAGE_NEWS_FORMAT, cni); + AddCompanyNewsItem(STR_MESSAGE_NEWS_FORMAT, std::move(cni)); AI::BroadcastNewEvent(new ScriptEventCompanyInTrouble(c->index)); Game::NewEvent(new ScriptEventCompanyInTrouble(c->index)); break; @@ -2011,14 +2011,14 @@ static void DoAcquireCompany(Company *c, bool hostile_takeover) { CompanyID ci = c->index; - CompanyNewsInformation *cni = new CompanyNewsInformation(c, Company::Get(_current_company)); + auto cni = std::make_unique(c, Company::Get(_current_company)); SetDParam(0, STR_NEWS_COMPANY_MERGER_TITLE); SetDParam(1, hostile_takeover ? STR_NEWS_MERGER_TAKEOVER_TITLE : STR_NEWS_COMPANY_MERGER_DESCRIPTION); SetDParamStr(2, cni->company_name); SetDParamStr(3, cni->other_company_name); SetDParam(4, c->bankrupt_value); - AddCompanyNewsItem(STR_MESSAGE_NEWS_FORMAT, cni); + AddCompanyNewsItem(STR_MESSAGE_NEWS_FORMAT, std::move(cni)); AI::BroadcastNewEvent(new ScriptEventCompanyMerger(ci, _current_company)); Game::NewEvent(new ScriptEventCompanyMerger(ci, _current_company)); diff --git a/src/news_func.h b/src/news_func.h index e24b115258..8e4ca6a591 100644 --- a/src/news_func.h +++ b/src/news_func.h @@ -15,11 +15,11 @@ #include "station_type.h" #include "industry_type.h" -void AddNewsItem(StringID string, NewsType type, NewsFlag flags, NewsReferenceType reftype1 = NR_NONE, uint32_t ref1 = UINT32_MAX, NewsReferenceType reftype2 = NR_NONE, uint32_t ref2 = UINT32_MAX, const NewsAllocatedData *data = nullptr); +void AddNewsItem(StringID string, NewsType type, NewsFlag flags, NewsReferenceType reftype1 = NR_NONE, uint32_t ref1 = UINT32_MAX, NewsReferenceType reftype2 = NR_NONE, uint32_t ref2 = UINT32_MAX, std::unique_ptr &&data = nullptr); -inline void AddCompanyNewsItem(StringID string, CompanyNewsInformation *cni) +inline void AddCompanyNewsItem(StringID string, std::unique_ptr cni) { - AddNewsItem(string, NT_COMPANY_INFO, NF_COMPANY, NR_NONE, UINT32_MAX, NR_NONE, UINT32_MAX, cni); + AddNewsItem(string, NT_COMPANY_INFO, NF_COMPANY, NR_NONE, UINT32_MAX, NR_NONE, UINT32_MAX, std::move(cni)); } /** @@ -42,14 +42,14 @@ inline void AddVehicleAdviceNewsItem(StringID string, VehicleID vehicle) AddNewsItem(string, NT_ADVICE, NF_INCOLOUR | NF_SMALL | NF_VEHICLE_PARAM0, NR_VEHICLE, vehicle); } -inline void AddTileNewsItem(StringID string, NewsType type, TileIndex tile, const NewsAllocatedData *data = nullptr, StationID station = INVALID_STATION) +inline void AddTileNewsItem(StringID string, NewsType type, TileIndex tile, std::unique_ptr &&data = nullptr, StationID station = INVALID_STATION) { - AddNewsItem(string, type, NF_NO_TRANSPARENT | NF_SHADE | NF_THIN, NR_TILE, tile.base(), station == INVALID_STATION ? NR_NONE : NR_STATION, station, data); + AddNewsItem(string, type, NF_NO_TRANSPARENT | NF_SHADE | NF_THIN, NR_TILE, tile.base(), station == INVALID_STATION ? NR_NONE : NR_STATION, station, std::move(data)); } -inline void AddIndustryNewsItem(StringID string, NewsType type, IndustryID industry, const NewsAllocatedData *data = nullptr) +inline void AddIndustryNewsItem(StringID string, NewsType type, IndustryID industry, std::unique_ptr &&data = nullptr) { - AddNewsItem(string, type, NF_NO_TRANSPARENT | NF_SHADE | NF_THIN, NR_INDUSTRY, industry, NR_NONE, UINT32_MAX, data); + AddNewsItem(string, type, NF_NO_TRANSPARENT | NF_SHADE | NF_THIN, NR_INDUSTRY, industry, NR_NONE, UINT32_MAX, std::move(data)); } void NewsLoop(); diff --git a/src/news_gui.cpp b/src/news_gui.cpp index 500d26ceaf..0e2453df2f 100644 --- a/src/news_gui.cpp +++ b/src/news_gui.cpp @@ -873,8 +873,8 @@ static std::list::iterator DeleteNewsItem(std::list::iterato * * @see NewsSubtype */ -NewsItem::NewsItem(StringID string_id, NewsType type, NewsFlag flags, NewsReferenceType reftype1, uint32_t ref1, NewsReferenceType reftype2, uint32_t ref2, const NewsAllocatedData *data) : - string_id(string_id), date(TimerGameCalendar::date), economy_date(TimerGameEconomy::date), type(type), flags(flags), reftype1(reftype1), reftype2(reftype2), ref1(ref1), ref2(ref2), data(data) +NewsItem::NewsItem(StringID string_id, NewsType type, NewsFlag flags, NewsReferenceType reftype1, uint32_t ref1, NewsReferenceType reftype2, uint32_t ref2, std::unique_ptr &&data) : + string_id(string_id), date(TimerGameCalendar::date), economy_date(TimerGameEconomy::date), type(type), flags(flags), reftype1(reftype1), reftype2(reftype2), ref1(ref1), ref2(ref2), data(std::move(data)) { /* show this news message in colour? */ if (TimerGameCalendar::year >= _settings_client.gui.coloured_news_year) this->flags |= NF_INCOLOUR; @@ -894,12 +894,12 @@ NewsItem::NewsItem(StringID string_id, NewsType type, NewsFlag flags, NewsRefere * * @see NewsSubtype */ -void AddNewsItem(StringID string, NewsType type, NewsFlag flags, NewsReferenceType reftype1, uint32_t ref1, NewsReferenceType reftype2, uint32_t ref2, const NewsAllocatedData *data) +void AddNewsItem(StringID string, NewsType type, NewsFlag flags, NewsReferenceType reftype1, uint32_t ref1, NewsReferenceType reftype2, uint32_t ref2, std::unique_ptr &&data) { if (_game_mode == GM_MENU) return; /* Create new news item node */ - _news.emplace_front(string, type, flags, reftype1, ref1, reftype2, ref2, data); + _news.emplace_front(string, type, flags, reftype1, ref1, reftype2, ref2, std::move(data)); /* Keep the number of stored news items to a managable number */ if (std::size(_news) > MAX_NEWS_AMOUNT) { diff --git a/src/news_type.h b/src/news_type.h index cd350faf2f..ee100f06d9 100644 --- a/src/news_type.h +++ b/src/news_type.h @@ -137,11 +137,11 @@ struct NewsItem { uint32_t ref1; ///< Reference 1 to some object: Used for a possible viewport, scrolling after clicking on the news, and for deleting the news when the object is deleted. uint32_t ref2; ///< Reference 2 to some object: Used for scrolling after clicking on the news, and for deleting the news when the object is deleted. - std::unique_ptr data; ///< Custom data for the news item that will be deallocated (deleted) when the news item has reached its end. + std::unique_ptr data; ///< Custom data for the news item that will be deallocated (deleted) when the news item has reached its end. std::vector params; ///< Parameters for string resolving. - NewsItem(StringID string_id, NewsType type, NewsFlag flags, NewsReferenceType reftype1, uint32_t ref1, NewsReferenceType reftype2, uint32_t ref2, const NewsAllocatedData *data); + NewsItem(StringID string_id, NewsType type, NewsFlag flags, NewsReferenceType reftype1, uint32_t ref1, NewsReferenceType reftype2, uint32_t ref2, std::unique_ptr &&data); }; /** diff --git a/src/town_cmd.cpp b/src/town_cmd.cpp index bae0eaed1a..ae02481c20 100644 --- a/src/town_cmd.cpp +++ b/src/town_cmd.cpp @@ -3530,12 +3530,12 @@ static CommandCost TownActionBuyRights(Town *t, DoCommandFlag flags) SetWindowClassesDirty(WC_STATION_VIEW); /* Spawn news message */ - CompanyNewsInformation *cni = new CompanyNewsInformation(Company::Get(_current_company)); + auto cni = std::make_unique(Company::Get(_current_company)); SetDParam(0, STR_NEWS_EXCLUSIVE_RIGHTS_TITLE); SetDParam(1, TimerGameEconomy::UsingWallclockUnits() ? STR_NEWS_EXCLUSIVE_RIGHTS_DESCRIPTION_MINUTES : STR_NEWS_EXCLUSIVE_RIGHTS_DESCRIPTION_MONTHS); SetDParam(2, t->index); SetDParamStr(3, cni->company_name); - AddNewsItem(STR_MESSAGE_NEWS_FORMAT, NT_GENERAL, NF_COMPANY, NR_TOWN, t->index, NR_NONE, UINT32_MAX, cni); + AddNewsItem(STR_MESSAGE_NEWS_FORMAT, NT_GENERAL, NF_COMPANY, NR_TOWN, t->index, NR_NONE, UINT32_MAX, std::move(cni)); AI::BroadcastNewEvent(new ScriptEventExclusiveTransportRights((ScriptCompany::CompanyID)(Owner)_current_company, t->index)); Game::NewEvent(new ScriptEventExclusiveTransportRights((ScriptCompany::CompanyID)(Owner)_current_company, t->index)); }