Make undo system a little safer, and improve docs

src/dialogxml/field.cpp

@@ -696,22 +696,22 @@ std::string cTextField::parse(ticpp::Element& who, std::string fname) {
 
 aTextInsert::aTextInsert(cTextField& in, int at, std::string text) : cAction("insert text"), in(in), at(at), text(text) {}
 
-void aTextInsert::undo() {
+bool aTextInsert::undo_me() {
 	std::string contents = in.getText();
 	auto del_start = contents.begin() + at;
 	auto del_end = del_start + text.length();
 	auto result = contents.erase(del_start, del_end);
 	in.setText(contents);
 	in.selectionPoint = in.insertionPoint = result - contents.begin();
-	done = false;
+	return true;
 }
 
-void aTextInsert::redo() {
+bool aTextInsert::redo_me() {
 	std::string contents = in.getText();
 	contents.insert(at, text);
 	in.setText(contents);
 	in.selectionPoint = in.insertionPoint = at + text.length();
-	done = true;
+	return true;
 }
 
 void aTextInsert::append(char c) {
@@ -722,22 +722,22 @@ aTextDelete::aTextDelete(cTextField& in, int start, int end) : cAction("delete t
 
 aTextDelete::aTextDelete(cTextField& in, int start, std::string content, bool from_start) : cAction("delete text"), in(in), start(start), end(start + content.size()), text(content), ip(from_start ? 0 : content.size()) {}
 
-void aTextDelete::undo() {
+bool aTextDelete::undo_me() {
 	std::string contents = in.getText();
 	contents.insert(start, text);
 	in.setText(contents);
 	in.selectionPoint = in.insertionPoint = start + ip;
-	done = false;
+	return true;
 }
 
-void aTextDelete::redo() {
+bool aTextDelete::redo_me() {
 	std::string contents = in.getText();
 	auto del_start = contents.begin() + start;
 	auto del_end = contents.begin() + end;
 	auto result = contents.erase(del_start, del_end);
 	in.setText(contents);
 	in.selectionPoint = in.insertionPoint = result - contents.begin();
-	done = true;
+	return true;
 }
 
 void aTextDelete::append_front(char c) {

src/dialogxml/field.hpp

@@ -98,9 +98,9 @@ class aTextInsert : public cAction {
 	cTextField& in;
 	int at;
 	std::string text;
+	bool undo_me() override, redo_me() override;
 public:
 	aTextInsert(cTextField& in, int at, std::string text = "");
-	void undo(), redo();
 	void append(char c);
 	~aTextInsert() {}
 };
@@ -109,10 +109,10 @@ class aTextDelete : public cAction {
 	cTextField& in;
 	int start, end, ip;
 	std::string text;
+	bool undo_me() override, redo_me() override;
 public:
 	aTextDelete(cTextField& in, int start, int end);
 	aTextDelete(cTextField& in, int start, std::string content, bool from_start);
-	void undo(), redo();
 	void append_front(char c);
 	void append_back(char c);
 	~aTextDelete() {}

src/scenedit/scen.actions.hpp

@@ -49,9 +49,9 @@ bool save_check(std::string which_dlog);
 /// Represents the action of adding a new town to the end of the list
 class aNewTown : public cAction {
 	class cTown* theTown;
+	bool undo_me() override;
+	bool redo_me() override;
 public:
 	aNewTown(class cTown* t);
-	void undo() override;
-	void redo() override;
 	~aNewTown();
 };

src/scenedit/scen.townout.cpp

@@ -1297,20 +1297,20 @@ aNewTown::aNewTown(cTown* t)
 	, theTown(t)
 {}
 
-void aNewTown::undo() {
+bool aNewTown::undo_me() {
 	scenario.towns.resize(scenario.towns.size() - 1);
-	done = false;
 	set_current_town(scenario.towns.size() - 1);
+	return true;
 }
 
-void aNewTown::redo() {
+bool aNewTown::redo_me() {
 	scenario.towns.push_back(theTown);
-	done = true;
 	set_current_town(scenario.towns.size() - 1);
+	return true;
 }
 
 aNewTown::~aNewTown() {
-	if(!done) delete theTown;
+	if(!isDone()) delete theTown;
 }
 
 bool new_town() {

src/tools/undo.cpp

@@ -10,6 +10,16 @@
 
 cAction::~cAction() {}
 
+void cAction::undo() {
+	if(done && undo_me())
+		done = false;
+}
+
+void cAction::redo() {
+	if(!done && redo_me())
+		done = true;
+}
+
 cUndoList::cUndoList(){
 	lastSave = cur = theList.begin();
 }

src/tools/undo.hpp

@@ -18,15 +18,36 @@
 
 class cAction {
 	std::string actname;
-protected:
 	bool done = true;
-public:
+protected:
+	/// Construct a named action
+	/// @param name The name of the action to show in the Edit menu
 	cAction(std::string name) : actname(name) {}
-	virtual void undo() = 0; ///< Undoes this action if it has not already been undone
-	virtual void redo() = 0; ///< Redoes this action if it has been undone
-	bool isDone() {return done;}; ///< checks to see whether the action has been undone; returns false if it has
-	std::string getActionName() {return actname;} ///< returns the name of this action for display in the Edit menu
+public:
+	/// Undoes this action if it has not already been undone.
+	/// If it has already been undone, does nothing.
+	void undo();
+	/// Redoes this action if it has been undone.
+	/// If it has not been undone, does nothing.
+	void redo();
+	/// Cchecks to see whether the action has been undone.
+	/// @return false if it the action has been undone.
+	bool isDone() {return done;};
+	/// Get the name of this action for display in the Edit menu.
+	/// @return The action name
+	std::string getActionName() {return actname;}
 	virtual ~cAction();
+private:
+	/// Implementation of @ref undo().
+	/// Must be overridden by subclasses to undo the action.
+	/// It can assume that the action has not been undone yet.
+	/// @return whether the undo succeeded.
+	virtual bool undo_me() = 0;
+	/// Implementation of @ref redo().
+	/// Must be overridden by subclasses to redo the action.
+	/// It can assume that the action has been undone already.
+	/// @return whether the redo succeeded.
+	virtual bool redo_me() = 0;
 };
 
 using action_ptr = std::shared_ptr<cAction>;
@@ -36,17 +57,39 @@ class cUndoList {
 	std::list<action_ptr>::iterator cur, lastSave;
 	size_t num_actions = 0;
 public:
+	/// Construct a new undo list.
 	cUndoList();
-	void undo(); ///< Undoes the current action and decrements the cur pointer
-	void redo(); ///< Increments the cur pointer and redoes the current action
-	void save(); ///< Sets the last saved action to the current action
-	void revert(); ///< Undoes all actions back to (but excluding) the last saved action
-	void clear(); ///< Clears the list
-	bool noUndo() const; ///< Check whether there's an action to undo
-	bool noRedo() const; ///< Check whether there's an action to redo
-	std::string undoName() const; ///< Get the action name of the next action to undo
-	std::string redoName() const; ///< Get the action name of the next action to redo
+	/// Undoes the current action and decrements the cur pointer.
+	void undo();
+	/// Increments the cur pointer and redoes the current action.
+	void redo();
+	/// Sets the last saved action to the current action.
+	void save(); 
+	/// Undoes all actions back to (but excluding) the last saved action.
+	void revert();
+	/// Clears the undo list.
+	void clear();
+	/// Check whether there's an action to undo.
+	/// @return true if there are no actions to undo.
+	bool noUndo() const;
+	/// Check whether there's an action to redo.
+	/// @return true if there are no actions to redo.
+	bool noRedo() const;
+	/// Get the action name of the next action to undo.
+	/// @return The name of the action to be undone.
+	std::string undoName() const;
+	/// Get the action name of the next action to redo.
+	/// @return The name of the action to be redone.
+	std::string redoName() const;
+	/// Add a new action to the undo list.
+	/// Any actions at the end of the list that have been undone are dropped.
+	/// May also drop actions from the front of the list.
+	/// @param what The action to add.
 	void add(action_ptr what);
+	/// Controls the maximum size of the undo list.
+	/// If more actions are added after this point, actions will be
+	/// dropped from the front of the list.
+	/// Actions are only dropped when adding new actions.
 	static size_t maxUndoSize;
 };