#410: add support for greying out menu items based on Statements

Some menu items (such as Brush/Prism...) are based on Statements (commands with arguments) rather than simple Commands. This change implements the canExecute() method on Statement objects, allowing menu items such as Prism to be disabled if the underlying command is not runnable (ignoring arguments). The only menu items this change currently affects are the first three items in the Brush menu, which are all variants of the QueryBrushPrefabSidesDialog command with different arguments.
codereader · Jun 28, 2022 · 1f8842d · 1f8842d
1 parent 5739fc5
commit 1f8842d
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 50 deletions.
diff --git a/include/icommandsystem.h b/include/icommandsystem.h
@@ -279,7 +279,8 @@ class ICommandSystem: public RegisterableModule
      * Function to check whether the command should be enabled based on current
      * application state.
      */
-    virtual void addWithCheck(const std::string& name, Function func, CheckFunction check) = 0;
+    virtual void addWithCheck(const std::string& name, Function func, CheckFunction check,
+                              const Signature& = {}) = 0;
 
     /// Returns true if the named command exists
     virtual bool commandExists(const std::string& name) = 0;

diff --git a/libs/selectionlib.h b/libs/selectionlib.h
@@ -79,7 +79,7 @@ class AmbiguousShaderException :
 	{}
 };
 
-/** 
+/**
  * greebo: Helper objects that compares each object passed
  * into it, throwing an AmbiguousShaderException as soon as
  * at least two different non-empty shader names are found.
@@ -125,7 +125,7 @@ class UniqueShaderFinder
 		{
 			// No shader encountered yet, take this one
 			_shader = foundShader;
-		} 
+		}
 		else if (_shader != foundShader)
 		{
 			throw AmbiguousShaderException(foundShader);
@@ -137,7 +137,7 @@ class UniqueShaderFinder
 
 }
 
-/** 
+/**
  * greebo: Retrieves the shader name from the current selection.
  * @returns: the name of the shader that is shared by every selected instance or
  * the empty string "" otherwise.
@@ -147,7 +147,7 @@ inline std::string getShaderFromSelection()
 	try
 	{
 		detail::UniqueShaderFinder finder;
-		
+
 		if (GlobalSelectionSystem().countSelectedComponents() > 0)
 		{
 			// Check each selected face
@@ -227,12 +227,12 @@ inline bool curSelectionIsSuitableForReparent()
 inline void assignNodeToSelectionGroups(const scene::INodePtr& node, const IGroupSelectable::GroupIds& groups)
 {
     auto groupSelectable = std::dynamic_pointer_cast<IGroupSelectable>(node);
-    
+
     if (!groupSelectable)
     {
         return;
     }
-    
+
     const auto& groupIds = groupSelectable->getGroupIds();
 
     auto previousGroups = groupSelectable->getGroupIds();
@@ -249,4 +249,14 @@ inline void assignNodeToSelectionGroups(const scene::INodePtr& node, const IGrou
     }
 }
 
+/// Selection predicates (e.g. for testing if a command should be runnable)
+namespace pred
+{
+    /// Return true if there is at least one selected brush
+    inline bool haveSelectedBrush()
+    {
+        return GlobalSelectionSystem().getSelectionInfo().brushCount > 0;
+    }
+}
+
 } // namespace selection
diff --git a/radiant/ui/UserInterfaceModule.cpp b/radiant/ui/UserInterfaceModule.cpp
@@ -433,9 +433,10 @@ void UserInterfaceModule::registerUICommands()
 	GlobalCommandSystem().addCommand("CreateSimplePatchDialog", PatchCreateDialog::Show);
 
 	GlobalCommandSystem().addCommand("ExportCollisionModelDialog", ExportCollisionModelDialog::Show);
-	GlobalCommandSystem().addCommand("QueryBrushPrefabSidesDialog", QuerySidesDialog::Show, { cmd::ARGTYPE_INT });
+    GlobalCommandSystem().addWithCheck("QueryBrushPrefabSidesDialog", QuerySidesDialog::Show,
+                                       selection::pred::haveSelectedBrush, {cmd::ARGTYPE_INT});
 
-	// Set up the CloneSelection command to react on key up events only
+    // Set up the CloneSelection command to react on key up events only
 	GlobalEventManager().addCommand("CloneSelection", "CloneSelection", true); // react on keyUp
 
 	GlobalEventManager().addRegistryToggle("ToggleRotationPivot", "user/ui/rotationPivotIsOrigin");

diff --git a/radiantcore/brush/csg/CSG.cpp b/radiantcore/brush/csg/CSG.cpp
@@ -11,6 +11,7 @@
 
 #include "scenelib.h"
 #include "shaderlib.h"
+#include "selectionlib.h"
 
 #include "registry/registry.h"
 #include "brush/Face.h"
@@ -89,11 +90,6 @@ void hollowBrush(const BrushNodePtr& sourceBrush, bool makeRoom)
 	scene::removeNodeFromParent(sourceBrush);
 }
 
-bool haveSelectedBrushes()
-{
-	return !selection::algorithm::getSelectedBrushes().empty();
-}
-
 void hollowSelectedBrushes(const cmd::ArgumentList& args) {
 	UndoableCommand undo("hollowSelectedBrushes");
 
@@ -498,11 +494,13 @@ void mergeSelectedBrushes(const cmd::ArgumentList& args)
 
 void registerCommands()
 {
+    using selection::pred::haveSelectedBrush;
+
     GlobalCommandSystem().addWithCheck("CSGSubtract", subtractBrushesFromUnselected,
-                                       haveSelectedBrushes);
-    GlobalCommandSystem().addWithCheck("CSGMerge", mergeSelectedBrushes, haveSelectedBrushes);
-    GlobalCommandSystem().addWithCheck("CSGHollow", hollowSelectedBrushes, haveSelectedBrushes);
-    GlobalCommandSystem().addWithCheck("CSGRoom", makeRoomForSelectedBrushes, haveSelectedBrushes);
+                                       haveSelectedBrush);
+    GlobalCommandSystem().addWithCheck("CSGMerge", mergeSelectedBrushes, haveSelectedBrush);
+    GlobalCommandSystem().addWithCheck("CSGHollow", hollowSelectedBrushes, haveSelectedBrush);
+    GlobalCommandSystem().addWithCheck("CSGRoom", makeRoomForSelectedBrushes, haveSelectedBrush);
 }
 
 } // namespace algorithm

diff --git a/radiantcore/commandsystem/CommandSystem.cpp b/radiantcore/commandsystem/CommandSystem.cpp
@@ -87,16 +87,16 @@ void CommandSystem::loadBinds() {
 		xml::Node& node = nodeList[i];
 
 		std::string name = node.getAttributeValue("name");
-		std::string statement = node.getAttributeValue("value");
+		std::string statementStr = node.getAttributeValue("value");
 
 		// remove all whitespace from the front and the tail
-		string::trim(statement);
+		string::trim(statementStr);
 
 		// Create a new statement
-		StatementPtr st(new Statement(
-			statement,
+		auto st = std::make_shared<Statement>(
+			statementStr,
 			(node.getAttributeValue("readonly") == "1")
-		));
+		);
 
 		std::pair<CommandMap::iterator, bool> result = _commands.insert(
 			CommandMap::value_type(name, st)
@@ -205,27 +205,17 @@ void CommandSystem::addCommand(const std::string& name, Function func,
 	addCommandObject(name, std::make_shared<Command>(func, signature));
 }
 
-void CommandSystem::addWithCheck(const std::string& name, Function func, CheckFunction check)
+void CommandSystem::addWithCheck(const std::string& name, Function func, CheckFunction check,
+                                 const Signature& sig)
 {
-	addCommandObject(name, std::make_shared<Command>(func, Signature(), check));
+	addCommandObject(name, std::make_shared<Command>(func, sig, check));
 }
 
 bool CommandSystem::commandExists(const std::string& name)
 {
 	return _commands.find(name) != _commands.end();
 }
 
-bool CommandSystem::canExecute(const std::string& name) const
-{
-	if (auto i = _commands.find(name); i != _commands.end()) {
-		return i->second->canExecute();
-	}
-
-	// We only return false if we know that a command cannot run; if the command
-	// was not found at all, return true by default.
-	return true;
-}
-
 void CommandSystem::removeCommand(const std::string& name)
 {
 	auto i = _commands.find(name);
@@ -291,16 +281,18 @@ namespace local
 	};
 }
 
-void CommandSystem::execute(const std::string& input)
+using StatementVec = std::vector<local::Statement>;
+
+StatementVec parseCommandString(const std::string& input)
 {
+	StatementVec statements;
+
 	// Instantiate a CommandTokeniser to analyse the given input string
 	CommandTokeniser tokeniser(input);
 
-	if (!tokeniser.hasMoreTokens()) return; // nothing to do!
+	if (!tokeniser.hasMoreTokens()) return statements;
 
-	std::vector<local::Statement> statements;
 	local::Statement curStatement;
-
 	while (tokeniser.hasMoreTokens())
 	{
 		// Inspect the next token
@@ -339,12 +331,39 @@ void CommandSystem::execute(const std::string& input)
 		statements.push_back(curStatement);
 	}
 
-	// Now execute the statements
-	for (const auto& statement : statements)
-	{
-		// Attempt ordinary command execution
-		executeCommand(statement.command, statement.args);
-	}
+    return statements;
+}
+
+bool CommandSystem::canExecute(const std::string& input) const
+{
+    // String could contain a command with arguments, so extract the main command
+    auto statements = parseCommandString(input);
+    if (!statements.empty()) {
+        // Check the first command, ignoring arguments or subsequent commands.
+        // For now we discount the possibility that the specific arguments could
+        // change whether a command can be run, although in theory this could be
+        // possible.
+        auto commandName = statements[0].command;
+        if (auto i = _commands.find(commandName); i != _commands.end()) {
+            return i->second->canExecute();
+        }
+    }
+
+    // We only return false if we know that a command cannot run; if the command
+    // was not found at all, return true by default.
+    return true;
+}
+
+void CommandSystem::execute(const std::string& input)
+{
+    // Parse the string into statement(s)
+    auto statements = parseCommandString(input);
+
+    // Now execute the statements
+    for (const auto& statement : statements) {
+        // Attempt ordinary command execution
+        executeCommand(statement.command, statement.args);
+    }
 }
 
 void CommandSystem::executeCommand(const std::string& name)

diff --git a/radiantcore/commandsystem/CommandSystem.h b/radiantcore/commandsystem/CommandSystem.h
@@ -22,7 +22,8 @@ class CommandSystem :
 
     void addCommand(const std::string& name, Function func,
                     const Signature& signature = Signature()) override;
-    void addWithCheck(const std::string& name, Function func, CheckFunction check) override;
+    void addWithCheck(const std::string& name, Function func, CheckFunction check,
+                      const Signature& signature) override;
     bool commandExists(const std::string& name) override;
     bool canExecute(const std::string& name) const override;
     void removeCommand(const std::string& name) override;

diff --git a/radiantcore/commandsystem/Executable.h b/radiantcore/commandsystem/Executable.h
@@ -17,7 +17,7 @@ class Executable
 	virtual void execute(const ArgumentList& args) = 0;
 
 	/// Test if this Executable can currently run
-	virtual bool canExecute() const { return true; }
+	virtual bool canExecute() const = 0;
 
 	/// Returns the function signature (argumen types) of this executable.
 	virtual Signature getSignature() = 0;

diff --git a/radiantcore/commandsystem/Statement.h b/radiantcore/commandsystem/Statement.h
@@ -21,12 +21,16 @@ class Statement :
 		_isReadOnly(isReadOnly)
 	{}
 
-	virtual void execute(const ArgumentList& args) {
+	void execute(const ArgumentList& args) override {
 		// Execution means parsing another string
 		GlobalCommandSystem().execute(_string);
 	}
 
-	Signature getSignature() {
+    bool canExecute() const override {
+        return GlobalCommandSystem().canExecute(_string);
+    }
+
+	Signature getSignature() override {
 		return Signature(); // signature is always empty
 	}