From c0479604aa82ade60b6331726a6c64972f3e53cb Mon Sep 17 00:00:00 2001
From: Gary Talent <gary@drinkingtea.net>
Date: Thu, 23 May 2024 21:50:27 -0500
Subject: [PATCH] [studio,nostalgia/studio] Make executing UndoCommands report
 errors

---
 .../paletteeditor/paletteeditor-imgui.cpp     | 16 +++++++-------
 .../tilesheeteditor/tilesheeteditormodel.cpp  |  2 +-
 src/olympic/studio/applib/src/studioapp.cpp   |  8 +++----
 .../studio/modlib/include/studio/editor.hpp   | 10 ++++-----
 .../modlib/include/studio/undostack.hpp       |  6 +++---
 src/olympic/studio/modlib/src/editor.cpp      |  4 ++--
 src/olympic/studio/modlib/src/undostack.cpp   | 21 ++++++++-----------
 7 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/src/nostalgia/modules/core/src/studio/paletteeditor/paletteeditor-imgui.cpp b/src/nostalgia/modules/core/src/studio/paletteeditor/paletteeditor-imgui.cpp
index 2abae5a0..c21a50d6 100644
--- a/src/nostalgia/modules/core/src/studio/paletteeditor/paletteeditor-imgui.cpp
+++ b/src/nostalgia/modules/core/src/studio/paletteeditor/paletteeditor-imgui.cpp
@@ -79,13 +79,13 @@ void PaletteEditorImGui::drawColorsEditor() noexcept {
 		if (ImGui::Button("Add", sz)) {
 			auto const colorSz = static_cast<int>(colors(m_pal, m_page));
 			constexpr Color16 c = 0;
-			undoStack()->push(ox::make_unique<AddColorCommand>(&m_pal, c, m_page, colorSz));
+			std::ignore = undoStack()->push(ox::make_unique<AddColorCommand>(&m_pal, c, m_page, colorSz));
 		}
 		ImGui::SameLine();
 		ImGui::BeginDisabled(m_selectedColorRow >= colors(m_pal, m_page));
 		{
 			if (ImGui::Button("Remove", sz)) {
-				undoStack()->push(
+				std::ignore = undoStack()->push(
 						ox::make_unique<RemoveColorCommand>(
 								&m_pal,
 								color(m_pal, m_page, static_cast<std::size_t>(m_selectedColorRow)),
@@ -97,7 +97,7 @@ void PaletteEditorImGui::drawColorsEditor() noexcept {
 			ImGui::BeginDisabled(m_selectedColorRow <= 0);
 			{
 				if (ImGui::Button("Move Up", sz)) {
-					undoStack()->push(ox::make_unique<MoveColorCommand>(&m_pal, m_page, m_selectedColorRow, -1));
+					std::ignore = undoStack()->push(ox::make_unique<MoveColorCommand>(&m_pal, m_page, m_selectedColorRow, -1));
 					--m_selectedColorRow;
 				}
 			}
@@ -106,7 +106,7 @@ void PaletteEditorImGui::drawColorsEditor() noexcept {
 			ImGui::BeginDisabled(m_selectedColorRow >= colors(m_pal, m_page) - 1);
 			{
 				if (ImGui::Button("Move Down", sz)) {
-					undoStack()->push(ox::make_unique<MoveColorCommand>(&m_pal, m_page, m_selectedColorRow, 1));
+					std::ignore = undoStack()->push(ox::make_unique<MoveColorCommand>(&m_pal, m_page, m_selectedColorRow, 1));
 					++m_selectedColorRow;
 				}
 			}
@@ -155,17 +155,17 @@ void PaletteEditorImGui::drawPagesEditor() noexcept {
 	constexpr auto toolbarHeight = 40;
 	auto const btnSz = ImVec2(paneSz.x / 3 - 5.5f, 24);
 	if (ImGui::Button("Add", btnSz)) {
-		undoStack()->push(ox::make_unique<AddPageCommand>(m_pal));
+		std::ignore = undoStack()->push(ox::make_unique<AddPageCommand>(m_pal));
 		m_page = m_pal.pages.size() - 1;
 	}
 	ImGui::SameLine();
 	if (ImGui::Button("Remove", btnSz)) {
-		undoStack()->push(ox::make_unique<RemovePageCommand>(m_pal, m_page));
+		std::ignore = undoStack()->push(ox::make_unique<RemovePageCommand>(m_pal, m_page));
 		m_page = std::min(m_page, m_pal.pages.size() - 1);
 	}
 	ImGui::SameLine();
 	if (ImGui::Button("Duplicate", btnSz)) {
-		undoStack()->push(ox::make_unique<DuplicatePageCommand>(m_pal, m_page, m_pal.pages.size()));
+		std::ignore = undoStack()->push(ox::make_unique<DuplicatePageCommand>(m_pal, m_page, m_pal.pages.size()));
 	}
 	ImGui::BeginTable("PageSelect", 2, tableFlags, ImVec2(paneSz.x, paneSz.y - (toolbarHeight + 5)));
 	{
@@ -198,7 +198,7 @@ void PaletteEditorImGui::drawColorEditor() noexcept {
 	ImGui::InputInt("Blue", &b, 1, 5);
 	auto const newColor = color16(r, g, b, a);
 	if (c != newColor) {
-		undoStack()->push(ox::make_unique<UpdateColorCommand>(
+		std::ignore = undoStack()->push(ox::make_unique<UpdateColorCommand>(
 				&m_pal, m_page, static_cast<int>(m_selectedColorRow), c, newColor));
 	}
 }
diff --git a/src/nostalgia/modules/core/src/studio/tilesheeteditor/tilesheeteditormodel.cpp b/src/nostalgia/modules/core/src/studio/tilesheeteditor/tilesheeteditormodel.cpp
index d5348252..01dde8c1 100644
--- a/src/nostalgia/modules/core/src/studio/tilesheeteditor/tilesheeteditormodel.cpp
+++ b/src/nostalgia/modules/core/src/studio/tilesheeteditor/tilesheeteditormodel.cpp
@@ -305,7 +305,7 @@ void TileSheetEditorModel::getFillPixels(bool *pixels, ox::Point const&pt, int o
 }
 
 void TileSheetEditorModel::pushCommand(studio::UndoCommand *cmd) noexcept {
-	m_undoStack.push(ox::UPtr<studio::UndoCommand>(cmd));
+	std::ignore = m_undoStack.push(ox::UPtr<studio::UndoCommand>(cmd));
 	m_ongoingDrawCommand = dynamic_cast<DrawCommand*>(cmd);
 	m_updated = true;
 }
diff --git a/src/olympic/studio/applib/src/studioapp.cpp b/src/olympic/studio/applib/src/studioapp.cpp
index 7d06e3bd..5231b679 100644
--- a/src/olympic/studio/applib/src/studioapp.cpp
+++ b/src/olympic/studio/applib/src/studioapp.cpp
@@ -197,10 +197,10 @@ void StudioUI::drawMenu() noexcept {
 		if (ImGui::BeginMenu("Edit")) {
 			auto undoStack = m_activeEditor ? m_activeEditor->undoStack() : nullptr;
 			if (ImGui::MenuItem("Undo", "Ctrl+Z", false, undoStack && undoStack->canUndo())) {
-				undoStack->undo();
+				oxLogError(undoStack->undo());
 			}
 			if (ImGui::MenuItem("Redo", "Ctrl+Y", false, undoStack && undoStack->canRedo())) {
-				undoStack->redo();
+				oxLogError(undoStack->redo());
 			}
 			ImGui::Separator();
 			if (ImGui::MenuItem("Copy", "Ctrl+C", false, m_activeEditor && m_activeEditor->copyEnabled())) {
@@ -315,14 +315,14 @@ void StudioUI::toggleProjectExplorer() noexcept {
 void StudioUI::redo() noexcept {
 	auto undoStack = m_activeEditor ? m_activeEditor->undoStack() : nullptr;
 	if (undoStack && undoStack->canRedo()) {
-		 m_activeEditor->undoStack()->redo();
+		 oxLogError(m_activeEditor->undoStack()->redo());
 	}
 }
 
 void StudioUI::undo() noexcept {
 	auto undoStack = m_activeEditor ? m_activeEditor->undoStack() : nullptr;
 	if (undoStack && undoStack->canUndo()) {
-		 m_activeEditor->undoStack()->undo();
+		 oxLogError(m_activeEditor->undoStack()->undo());
 	}
 }
 
diff --git a/src/olympic/studio/modlib/include/studio/editor.hpp b/src/olympic/studio/modlib/include/studio/editor.hpp
index 31421dcb..69cf082b 100644
--- a/src/olympic/studio/modlib/include/studio/editor.hpp
+++ b/src/olympic/studio/modlib/include/studio/editor.hpp
@@ -131,16 +131,14 @@ class Editor: public studio::BaseEditor {
 		[[nodiscard]]
 		UndoStack *undoStack() noexcept final;
 
-		void pushCommand(ox::UPtr<UndoCommand> &&cmd) noexcept;
+		ox::Error pushCommand(ox::UPtr<UndoCommand> &&cmd) noexcept;
 
 		template<typename UC, typename ...Args>
-		bool pushCommand(Args&&... args) noexcept {
+		ox::Error pushCommand(Args&&... args) noexcept {
 			try {
-				m_undoStack.push(ox::make_unique<UC>(ox::forward<Args>(args)...));
-				return true;
+				return m_undoStack.push(ox::make_unique<UC>(ox::forward<Args>(args)...));
 			} catch (ox::Exception const&ex) {
-				oxLogError(ex.toError());
-				return false;
+				return ex.toError();
 			}
 		}
 
diff --git a/src/olympic/studio/modlib/include/studio/undostack.hpp b/src/olympic/studio/modlib/include/studio/undostack.hpp
index 3ef654b4..b6a5d562 100644
--- a/src/olympic/studio/modlib/include/studio/undostack.hpp
+++ b/src/olympic/studio/modlib/include/studio/undostack.hpp
@@ -19,11 +19,11 @@ class UndoStack {
 		std::size_t m_stackIdx = 0;
 
 	public:
-		void push(ox::UPtr<UndoCommand> &&cmd) noexcept;
+		ox::Error push(ox::UPtr<UndoCommand> &&cmd) noexcept;
 
-		void redo() noexcept;
+		ox::Error redo() noexcept;
 
-		void undo() noexcept;
+		ox::Error undo() noexcept;
 
 		[[nodiscard]]
 		constexpr bool canRedo() const noexcept {
diff --git a/src/olympic/studio/modlib/src/editor.cpp b/src/olympic/studio/modlib/src/editor.cpp
index 1c78de60..9bd199b8 100644
--- a/src/olympic/studio/modlib/src/editor.cpp
+++ b/src/olympic/studio/modlib/src/editor.cpp
@@ -127,8 +127,8 @@ ox::CStringView Editor::itemDisplayName() const noexcept {
 	return m_itemName;
 }
 
-void Editor::pushCommand(ox::UPtr<UndoCommand> &&cmd) noexcept {
-	m_undoStack.push(std::move(cmd));
+ox::Error Editor::pushCommand(ox::UPtr<UndoCommand> &&cmd) noexcept {
+	return m_undoStack.push(std::move(cmd));
 }
 
 UndoStack *Editor::undoStack() noexcept {
diff --git a/src/olympic/studio/modlib/src/undostack.cpp b/src/olympic/studio/modlib/src/undostack.cpp
index 2f0ed079..77cb049b 100644
--- a/src/olympic/studio/modlib/src/undostack.cpp
+++ b/src/olympic/studio/modlib/src/undostack.cpp
@@ -6,42 +6,39 @@
 
 namespace studio {
 
-void UndoStack::push(ox::UPtr<UndoCommand> &&cmd) noexcept {
+ox::Error UndoStack::push(ox::UPtr<UndoCommand> &&cmd) noexcept {
 	for (auto const i = m_stackIdx; i < m_stack.size();) {
 		std::ignore = m_stack.erase(i);
 	}
-	if (cmd->redo()) {
-		return;
-	}
+	oxReturnError(cmd->redo());
 	redoTriggered.emit(cmd.get());
 	changeTriggered.emit(cmd.get());
 	if (m_stack.empty() || !(*m_stack.back().value)->mergeWith(*cmd)) {
 		m_stack.emplace_back(std::move(cmd));
 		++m_stackIdx;
 	}
+	return {};
 }
 
-void UndoStack::redo() noexcept {
+ox::Error UndoStack::redo() noexcept {
 	if (m_stackIdx < m_stack.size()) {
 		auto &c = m_stack[m_stackIdx];
-		if (c->redo()) {
-			return;
-		}
+		oxReturnError(c->redo());
 		++m_stackIdx;
 		redoTriggered.emit(c.get());
 		changeTriggered.emit(c.get());
 	}
+	return {};
 }
 
-void UndoStack::undo() noexcept {
+ox::Error UndoStack::undo() noexcept {
 	if (m_stackIdx) {
 		auto &c = m_stack[--m_stackIdx];
-		if (c->undo()) {
-			return;
-		}
+		oxReturnError(c->undo());
 		undoTriggered.emit(c.get());
 		changeTriggered.emit(c.get());
 	}
+	return {};
 }
 
 }