Apply clang-tidy configuration to (some of) ui_item.h (#3719)

Source/DiabloUI/button.cpp

@@ -16,20 +16,15 @@ void LoadSmlButtonArt()
 
 void RenderButton(UiButton *button)
 {
-	int frame;
-	if (button->m_pressed) {
-		frame = UiButton::PRESSED;
-	} else {
-		frame = UiButton::DEFAULT;
-	}
-	DrawArt({ button->m_rect.x, button->m_rect.y }, button->m_art, frame, button->m_rect.w, button->m_rect.h);
+	DrawArt({ button->m_rect.x, button->m_rect.y }, button->GetArt(), button->GetFrame(), button->m_rect.w, button->m_rect.h);
 
 	Rectangle textRect { { button->m_rect.x, button->m_rect.y }, { button->m_rect.w, button->m_rect.h } };
-	if (!button->m_pressed)
+	if (!button->IsPressed()) {
 		--textRect.position.y;
+	}
 
 	const Surface &out = Surface(DiabloUiSurface());
-	DrawString(out, button->m_text, textRect, UiFlags::AlignCenter | UiFlags::FontSizeDialog | UiFlags::ColorDialogWhite);
+	DrawString(out, button->GetText(), textRect, UiFlags::AlignCenter | UiFlags::FontSizeDialog | UiFlags::ColorDialogWhite);
 }
 
 bool HandleMouseEventButton(const SDL_Event &event, UiButton *button)
@@ -38,10 +33,13 @@ bool HandleMouseEventButton(const SDL_Event &event, UiButton *button)
 		return false;
 	switch (event.type) {
 	case SDL_MOUSEBUTTONUP:
-		button->m_action();
-		return true;
+		if (button->IsPressed()) {
+			button->Activate();
+			return true;
+		}
+		return false;
 	case SDL_MOUSEBUTTONDOWN:
-		button->m_pressed = true;
+		button->Press();
 		return true;
 	default:
 		return false;
@@ -50,7 +48,7 @@ bool HandleMouseEventButton(const SDL_Event &event, UiButton *button)
 
 void HandleGlobalMouseUpButton(UiButton *button)
 {
-	button->m_pressed = false;
+	button->Release();
 }
 
 } // namespace devilution

Source/DiabloUI/diabloui.cpp

@@ -116,7 +116,7 @@ void UiInitList(void (*fnFocus)(int value), void (*fnSelect)(int value), void (*
 	textInputActive = false;
 	UiScrollbar *uiScrollbar = nullptr;
 	for (const auto &item : items) {
-		if (item->m_type == UiType::Edit) {
+		if (item->IsType(UiType::Edit)) {
 			auto *pItemUIEdit = static_cast<UiEdit *>(item.get());
 			SDL_SetTextInputRect(&item->m_rect);
 			textInputActive = true;
@@ -132,12 +132,12 @@ void UiInitList(void (*fnFocus)(int value), void (*fnSelect)(int value), void (*
 #endif
 			UiTextInput = pItemUIEdit->m_value;
 			UiTextInputLen = pItemUIEdit->m_max_length;
-		} else if (item->m_type == UiType::List) {
+		} else if (item->IsType(UiType::List)) {
 			auto *uiList = static_cast<UiList *>(item.get());
 			SelectedItemMax = std::max(uiList->m_vecItems.size() - 1, static_cast<size_t>(0));
 			ListViewportSize = uiList->viewportSize;
 			gUiList = uiList;
-		} else if (item->m_type == UiType::Scrollbar) {
+		} else if (item->IsType(UiType::Scrollbar)) {
 			uiScrollbar = static_cast<UiScrollbar *>(item.get());
 		}
 	}
@@ -146,9 +146,9 @@ void UiInitList(void (*fnFocus)(int value), void (*fnSelect)(int value), void (*
 
 	if (uiScrollbar != nullptr) {
 		if (ListViewportSize >= static_cast<std::size_t>(SelectedItemMax + 1)) {
-			uiScrollbar->add_flag(UiFlags::ElementHidden);
+			uiScrollbar->Hide();
 		} else {
-			uiScrollbar->remove_flag(UiFlags::ElementHidden);
+			uiScrollbar->Show();
 		}
 	}
 }
@@ -750,12 +750,12 @@ void UiPollAndRender()
 
 namespace {
 
-void Render(UiText *uiText)
+void Render(const UiText *uiText)
 {
 	Rectangle rect { { uiText->m_rect.x, uiText->m_rect.y }, { uiText->m_rect.w, uiText->m_rect.h } };
 
 	const Surface &out = Surface(DiabloUiSurface());
-	DrawString(out, uiText->m_text, rect, uiText->m_iFlags | UiFlags::FontSizeDialog);
+	DrawString(out, uiText->GetText(), rect, uiText->GetFlags() | UiFlags::FontSizeDialog);
 }
 
 void Render(const UiArtText *uiArtText)
@@ -763,20 +763,20 @@ void Render(const UiArtText *uiArtText)
 	Rectangle rect { { uiArtText->m_rect.x, uiArtText->m_rect.y }, { uiArtText->m_rect.w, uiArtText->m_rect.h } };
 
 	const Surface &out = Surface(DiabloUiSurface());
-	DrawString(out, uiArtText->text(), rect, uiArtText->m_iFlags, uiArtText->spacing(), uiArtText->lineHeight());
+	DrawString(out, uiArtText->GetText(), rect, uiArtText->GetFlags(), uiArtText->GetSpacing(), uiArtText->GetLineHeight());
 }
 
 void Render(const UiImage *uiImage)
 {
 	int x = uiImage->m_rect.x;
-	if (HasAnyOf(uiImage->m_iFlags, UiFlags::AlignCenter) && uiImage->m_art != nullptr) {
-		const int xOffset = GetCenterOffset(uiImage->m_art->w(), uiImage->m_rect.w);
+	if (uiImage->IsCentered() && uiImage->GetArt() != nullptr) {
+		const int xOffset = GetCenterOffset(uiImage->GetArt()->w(), uiImage->m_rect.w);
 		x += xOffset;
 	}
-	if (uiImage->m_animated) {
-		DrawAnimatedArt(uiImage->m_art, { x, uiImage->m_rect.y });
+	if (uiImage->IsAnimated()) {
+		DrawAnimatedArt(uiImage->GetArt(), { x, uiImage->m_rect.y });
 	} else {
-		DrawArt({ x, uiImage->m_rect.y }, uiImage->m_art, uiImage->m_frame, uiImage->m_rect.w);
+		DrawArt({ x, uiImage->m_rect.y }, uiImage->GetArt(), uiImage->GetFrame(), uiImage->m_rect.w);
 	}
 }
 
@@ -785,7 +785,7 @@ void Render(const UiArtTextButton *uiButton)
 	Rectangle rect { { uiButton->m_rect.x, uiButton->m_rect.y }, { uiButton->m_rect.w, uiButton->m_rect.h } };
 
 	const Surface &out = Surface(DiabloUiSurface());
-	DrawString(out, uiButton->m_text, rect, uiButton->m_iFlags);
+	DrawString(out, uiButton->GetText(), rect, uiButton->GetFlags());
 }
 
 void Render(const UiList *uiList)
@@ -799,10 +799,10 @@ void Render(const UiList *uiList)
 			DrawSelector(rect);
 
 		Rectangle rectangle { { rect.x, rect.y }, { rect.w, rect.h } };
-		if (item->args.size() == 0)
-			DrawString(out, item->m_text, rectangle, uiList->m_iFlags | item->uiFlags, uiList->spacing());
+		if (item->args.empty())
+			DrawString(out, item->m_text, rectangle, uiList->GetFlags() | item->uiFlags, uiList->GetSpacing());
 		else
-			DrawStringWithColors(out, item->m_text, item->args, rectangle, uiList->m_iFlags | item->uiFlags, uiList->spacing());
+			DrawStringWithColors(out, item->m_text, item->args, rectangle, uiList->GetFlags() | item->uiFlags, uiList->GetSpacing());
 	}
 }
 
@@ -845,14 +845,14 @@ void Render(const UiEdit *uiEdit)
 	Rectangle rect { { uiEdit->m_rect.x + 43, uiEdit->m_rect.y + 1 }, { uiEdit->m_rect.w - 86, uiEdit->m_rect.h } };
 
 	const Surface &out = Surface(DiabloUiSurface());
-	DrawString(out, uiEdit->m_value, rect, uiEdit->m_iFlags | UiFlags::TextCursor);
+	DrawString(out, uiEdit->m_value, rect, uiEdit->GetFlags() | UiFlags::TextCursor);
 }
 
 void RenderItem(UiItemBase *item)
 {
-	if (item->has_flag(UiFlags::ElementHidden))
+	if (item->IsHidden())
 		return;
-	switch (item->m_type) {
+	switch (item->GetType()) {
 	case UiType::Text:
 		Render(static_cast<UiText *>(item));
 		break;
@@ -882,9 +882,11 @@ void RenderItem(UiItemBase *item)
 
 bool HandleMouseEventArtTextButton(const SDL_Event &event, const UiArtTextButton *uiButton)
 {
-	if (event.type != SDL_MOUSEBUTTONDOWN || event.button.button != SDL_BUTTON_LEFT)
+	if (event.type != SDL_MOUSEBUTTONDOWN || event.button.button != SDL_BUTTON_LEFT) {
 		return false;
-	uiButton->m_action();
+	}
+
+	uiButton->Activate();
 	return true;
 }
 
@@ -959,9 +961,9 @@ bool HandleMouseEventScrollBar(const SDL_Event &event, const UiScrollbar *uiSb)
 
 bool HandleMouseEvent(const SDL_Event &event, UiItemBase *item)
 {
-	if (item->has_any_flag(UiFlags::ElementHidden | UiFlags::ElementDisabled) || !IsInsideRect(event, item->m_rect))
+	if (item->IsNotInteractive() || !IsInsideRect(event, item->m_rect))
 		return false;
-	switch (item->m_type) {
+	switch (item->GetType()) {
 	case UiType::ArtTextButton:
 		return HandleMouseEventArtTextButton(event, static_cast<UiArtTextButton *>(item));
 	case UiType::Button:
@@ -1018,8 +1020,9 @@ bool UiItemMouseEvents(SDL_Event *event, const std::vector<UiItemBase *> &items)
 	if (event->type == SDL_MOUSEBUTTONUP && event->button.button == SDL_BUTTON_LEFT) {
 		scrollBarState.downArrowPressed = scrollBarState.upArrowPressed = false;
 		for (const auto &item : items) {
-			if (item->m_type == UiType::Button)
+			if (item->IsType(UiType::Button)) {
 				HandleGlobalMouseUpButton(static_cast<UiButton *>(item));
+			}
 		}
 	}
 
@@ -1048,8 +1051,9 @@ bool UiItemMouseEvents(SDL_Event *event, const std::vector<std::unique_ptr<UiIte
 	if (event->type == SDL_MOUSEBUTTONUP && event->button.button == SDL_BUTTON_LEFT) {
 		scrollBarState.downArrowPressed = scrollBarState.upArrowPressed = false;
 		for (const auto &item : items) {
-			if (item->m_type == UiType::Button)
+			if (item->IsType(UiType::Button)) {
 				HandleGlobalMouseUpButton(static_cast<UiButton *>(item.get()));
+			}
 		}
 	}
 

Source/DiabloUI/selhero.cpp

@@ -75,7 +75,7 @@ void SelheroFree()
 
 void SelheroSetStats()
 {
-	SELHERO_DIALOG_HERO_IMG->m_frame = static_cast<int>(selhero_heroInfo.heroclass);
+	SELHERO_DIALOG_HERO_IMG->SetFrame(static_cast<int>(selhero_heroInfo.heroclass));
 	snprintf(textStats[0], sizeof(textStats[0]), "%i", selhero_heroInfo.level);
 	snprintf(textStats[1], sizeof(textStats[1]), "%i", selhero_heroInfo.strength);
 	snprintf(textStats[2], sizeof(textStats[2]), "%i", selhero_heroInfo.magic);
@@ -102,15 +102,15 @@ void SelheroListFocus(int value)
 	if (selhero_SaveCount != 0 && index < selhero_SaveCount) {
 		memcpy(&selhero_heroInfo, &selhero_heros[index], sizeof(selhero_heroInfo));
 		SelheroSetStats();
-		SELLIST_DIALOG_DELETE_BUTTON->m_iFlags = baseFlags | UiFlags::ColorUiGold;
+		SELLIST_DIALOG_DELETE_BUTTON->SetFlags(baseFlags | UiFlags::ColorUiGold);
 		selhero_deleteEnabled = true;
 		return;
 	}
 
-	SELHERO_DIALOG_HERO_IMG->m_frame = static_cast<int>(enum_size<HeroClass>::value);
+	SELHERO_DIALOG_HERO_IMG->SetFrame(static_cast<int>(enum_size<HeroClass>::value));
 	for (char *textStat : textStats)
 		strcpy(textStat, "--");
-	SELLIST_DIALOG_DELETE_BUTTON->m_iFlags = baseFlags | UiFlags::ColorUiSilver | UiFlags::ElementDisabled;
+	SELLIST_DIALOG_DELETE_BUTTON->SetFlags(baseFlags | UiFlags::ColorUiSilver | UiFlags::ElementDisabled);
 	selhero_deleteEnabled = false;
 }
 

Source/DiabloUI/ui_item.h

@@ -26,39 +26,64 @@ enum class UiType {
 
 class UiItemBase {
 public:
-	UiItemBase(UiType type, SDL_Rect rect, UiFlags flags)
-	    : m_type(type)
-	    , m_rect(rect)
-	    , m_iFlags(flags)
+	virtual ~UiItemBase() = default;
+
+	[[nodiscard]] UiType GetType() const
+	{
+		return type_;
+	}
+
+	[[nodiscard]] bool IsType(UiType testType) const
+	{
+		return type_ == testType;
+	}
+
+	[[nodiscard]] UiFlags GetFlags() const
+	{
+		return uiFlags_;
+	}
+
+	[[nodiscard]] bool IsHidden() const
 	{
+		return HasAnyOf(uiFlags_, UiFlags::ElementHidden);
 	}
 
-	virtual ~UiItemBase() {};
+	[[nodiscard]] bool IsNotInteractive() const
+	{
+		return HasAnyOf(uiFlags_, UiFlags::ElementHidden | UiFlags::ElementDisabled);
+	}
 
-	bool has_flag(UiFlags flag) const
+	void Hide()
 	{
-		return (m_iFlags & flag) == flag;
+		uiFlags_ |= UiFlags::ElementHidden;
 	}
 
-	bool has_any_flag(UiFlags flags) const
+	void Show()
 	{
-		return HasAnyOf(m_iFlags, flags);
+		uiFlags_ &= ~UiFlags::ElementHidden;
 	}
 
-	void add_flag(UiFlags flag)
+protected:
+	UiItemBase(UiType type, SDL_Rect rect, UiFlags flags)
+	    : type_(type)
+	    , m_rect(rect)
+	    , uiFlags_(flags)
 	{
-		m_iFlags |= flag;
 	}
 
-	void remove_flag(UiFlags flag)
+	void SetFlags(UiFlags flags)
 	{
-		m_iFlags &= ~flag;
+		uiFlags_ = flags;
 	}
 
-	// protected:
-	UiType m_type;
+private:
+	UiType type_;
+
+public:
 	SDL_Rect m_rect;
-	UiFlags m_iFlags;
+
+private:
+	UiFlags uiFlags_;
 };
 
 //=============================================================================
@@ -67,18 +92,41 @@ class UiImage : public UiItemBase {
 public:
 	UiImage(Art *art, SDL_Rect rect, UiFlags flags = UiFlags::None, bool animated = false, int frame = 0)
 	    : UiItemBase(UiType::Image, rect, flags)
-	    , m_art(art)
-	    , m_animated(animated)
-	    , m_frame(frame)
+	    , art_(art)
+	    , animated_(animated)
+	    , frame_(frame)
+	{
+	}
+
+	[[nodiscard]] bool IsCentered() const
 	{
+		return HasAnyOf(GetFlags(), UiFlags::AlignCenter);
 	}
 
-	~UiImage() {};
+	[[nodiscard]] Art *GetArt() const
+	{
+		return art_;
+	}
 
-	// private:
-	Art *m_art;
-	bool m_animated;
-	int m_frame;
+	[[nodiscard]] bool IsAnimated() const
+	{
+		return animated_;
+	}
+
+	[[nodiscard]] int GetFrame() const
+	{
+		return frame_;
+	}
+
+	void SetFrame(int frame)
+	{
+		frame_ = frame;
+	}
+
+private:
+	Art *art_;
+	bool animated_;
+	int frame_;
 };
 
 //=============================================================================
@@ -93,9 +141,9 @@ public:
 	 */
 	UiArtText(const char *text, SDL_Rect rect, UiFlags flags = UiFlags::None, int spacing = 1, int lineHeight = -1)
 	    : UiItemBase(UiType::ArtText, rect, flags)
-	    , m_text(text)
-	    , m_spacing(spacing)
-	    , m_lineHeight(lineHeight)
+	    , text_(text)
+	    , spacing_(spacing)
+	    , lineHeight_(lineHeight)
 	{
 	}
 
@@ -107,36 +155,34 @@ public:
 	 */
 	UiArtText(const char **ptext, SDL_Rect rect, UiFlags flags = UiFlags::None, int spacing = 1, int lineHeight = -1)
 	    : UiItemBase(UiType::ArtText, rect, flags)
-	    , m_ptext(ptext)
-	    , m_spacing(spacing)
-	    , m_lineHeight(lineHeight)
+	    , textPointer_(ptext)
+	    , spacing_(spacing)
+	    , lineHeight_(lineHeight)
 	{
 	}
 
-	const char *text() const
+	[[nodiscard]] string_view GetText() const
 	{
-		if (m_text != nullptr)
-			return m_text;
-		return *m_ptext;
+		if (text_ != nullptr)
+			return text_;
+		return *textPointer_;
 	}
 
-	int spacing() const
+	[[nodiscard]] int GetSpacing() const
 	{
-		return m_spacing;
+		return spacing_;
 	}
 
-	int lineHeight() const
+	[[nodiscard]] int GetLineHeight() const
 	{
-		return m_lineHeight;
+		return lineHeight_;
 	}
 
-	~UiArtText() {};
-
 private:
-	const char *m_text = nullptr;
-	const char **m_ptext = nullptr;
-	int m_spacing = 1;
-	int m_lineHeight = -1;
+	const char *text_ = nullptr;
+	const char **textPointer_ = nullptr;
+	int spacing_;
+	int lineHeight_;
 };
 
 //=============================================================================
@@ -161,27 +207,44 @@ public:
 
 class UiArtTextButton : public UiItemBase {
 public:
-	UiArtTextButton(const char *text, void (*action)(), SDL_Rect rect, UiFlags flags = UiFlags::None)
+	using Callback = void (*)();
+
+	UiArtTextButton(const char *text, Callback action, SDL_Rect rect, UiFlags flags = UiFlags::None)
 	    : UiItemBase(UiType::ArtTextButton, rect, flags)
-	    , m_text(text)
-	    , m_action(action)
+	    , text_(text)
+	    , action_(action)
 	{
 	}
 
-	// private:
-	const char *m_text;
-	void (*m_action)();
+	void SetFlags(UiFlags flags)
+	{
+		UiItemBase::SetFlags(flags);
+	}
+
+	[[nodiscard]] string_view GetText() const
+	{
+		return text_;
+	}
+
+	void Activate() const
+	{
+		action_();
+	}
+
+private:
+	const char *text_;
+	Callback action_;
 };
 
 //=============================================================================
 
 class UiEdit : public UiItemBase {
 public:
-	UiEdit(const char *hint, char *value, std::size_t max_length, bool allowEmpty, SDL_Rect rect, UiFlags flags = UiFlags::None)
+	UiEdit(const char *hint, char *value, std::size_t maxLength, bool allowEmpty, SDL_Rect rect, UiFlags flags = UiFlags::None)
 	    : UiItemBase(UiType::Edit, rect, flags)
 	    , m_hint(hint)
 	    , m_value(value)
-	    , m_max_length(max_length)
+	    , m_max_length(maxLength)
 	    , m_allowEmpty(allowEmpty)
 	{
 	}
@@ -201,12 +264,17 @@ class UiText : public UiItemBase {
 public:
 	UiText(const char *text, SDL_Rect rect, UiFlags flags = UiFlags::ColorDialogWhite)
 	    : UiItemBase(UiType::Text, rect, flags)
-	    , m_text(text)
+	    , text_(text)
 	{
 	}
 
-	// private:
-	const char *m_text;
+	[[nodiscard]] string_view GetText() const
+	{
+		return text_;
+	}
+
+private:
+	const char *text_;
 };
 
 //=============================================================================
@@ -215,28 +283,61 @@ public:
 
 class UiButton : public UiItemBase {
 public:
-	UiButton(Art *art, const char *text, void (*action)(), SDL_Rect rect, UiFlags flags = UiFlags::None)
+	using Callback = void (*)();
+
+	UiButton(Art *art, const char *text, Callback action, SDL_Rect rect, UiFlags flags = UiFlags::None)
 	    : UiItemBase(UiType::Button, rect, flags)
-	    , m_art(art)
-	    , m_text(text)
-	    , m_action(action)
-	    , m_pressed(false)
+	    , art_(art)
+	    , text_(text)
+	    , action_(action)
+	    , pressed_(false)
 	{
 	}
 
-	enum FrameKey : uint8_t {
-		DEFAULT,
-		PRESSED,
-	};
+	[[nodiscard]] int GetFrame() const
+	{
+		// Frame 1 is a held button sprite, frame 0 is the default
+		return IsPressed() ? 1 : 0;
+	}
 
-	// private:
-	Art *m_art;
+	[[nodiscard]] Art *GetArt() const
+	{
+		return art_;
+	}
 
-	const char *m_text;
-	void (*m_action)();
+	[[nodiscard]] string_view GetText() const
+	{
+		return text_;
+	}
+
+	void Activate() const
+	{
+		action_();
+	}
+
+	[[nodiscard]] bool IsPressed() const
+	{
+		return pressed_;
+	}
+
+	void Press()
+	{
+		pressed_ = true;
+	}
+
+	void Release()
+	{
+		pressed_ = false;
+	}
+
+private:
+	Art *art_;
+
+	const char *text_;
+	Callback action_;
 
 	// State
-	bool m_pressed;
+	bool pressed_;
 };
 
 //=============================================================================
@@ -258,8 +359,6 @@ public:
 	{
 	}
 
-	~UiListItem() {};
-
 	// private:
 	const char *m_text;
 	std::vector<DrawStringFormatArg> args;
@@ -267,10 +366,10 @@ public:
 	UiFlags uiFlags;
 };
 
-typedef std::vector<std::unique_ptr<UiListItem>> vUiListItem;
-
 class UiList : public UiItemBase {
 public:
+	using vUiListItem = std::vector<std::unique_ptr<UiListItem>>;
+
 	UiList(const vUiListItem &vItems, size_t viewportSize, Sint16 x, Sint16 y, Uint16 item_width, Uint16 item_height, UiFlags flags = UiFlags::None, int spacing = 1)
 	    : UiItemBase(UiType::List, { x, y, item_width, static_cast<Uint16>(item_height * viewportSize) }, flags)
 	    , viewportSize(viewportSize)
@@ -278,15 +377,13 @@ public:
 	    , m_y(y)
 	    , m_width(item_width)
 	    , m_height(item_height)
-	    , m_spacing(spacing)
+	    , spacing_(spacing)
 	{
-		for (auto &item : vItems)
+		for (const auto &item : vItems)
 			m_vecItems.push_back(item.get());
 	}
 
-	~UiList() {};
-
-	SDL_Rect itemRect(int i) const
+	[[nodiscard]] SDL_Rect itemRect(int i) const
 	{
 		SDL_Rect tmp;
 		tmp.x = m_x;
@@ -305,14 +402,14 @@ public:
 		return index;
 	}
 
-	UiListItem *GetItem(int i) const
+	[[nodiscard]] UiListItem *GetItem(std::size_t i) const
 	{
 		return m_vecItems[i];
 	}
 
-	int spacing() const
+	[[nodiscard]] int GetSpacing() const
 	{
-		return m_spacing;
+		return spacing_;
 	}
 
 	// private:
@@ -320,6 +417,8 @@ public:
 	Sint16 m_x, m_y;
 	Uint16 m_width, m_height;
 	std::vector<UiListItem *> m_vecItems;
-	int m_spacing;
+
+private:
+	int spacing_;
 };
 } // namespace devilution