From 334fb36dea931eb1a544c19957e661534cf77b0f Mon Sep 17 00:00:00 2001
From: Nathan Franke <me@nathan.sh>
Date: Fri, 5 Aug 2022 22:08:37 -0500
Subject: [PATCH] improve errors for node group methods, improve documentation

---
 doc/classes/Node.xml |  9 +++++++--
 scene/main/node.cpp  | 10 ++++------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml
index 8cc8498609ec..b45296f86d3b 100644
--- a/doc/classes/Node.xml
+++ b/doc/classes/Node.xml
@@ -164,7 +164,10 @@
 			<argument index="0" name="group" type="StringName" />
 			<argument index="1" name="persistent" type="bool" default="false" />
 			<description>
-				Adds the node to a group. Groups are helpers to name and organize a subset of nodes, for example "enemies" or "collectables". A node can be in any number of groups. Nodes can be assigned a group at any time, but will not be added until they are inside the scene tree (see [method is_inside_tree]). See notes in the description, and the group methods in [SceneTree].
+				Adds this node to the given [code]group[/code]. Groups are helpers to name and organize a subset of nodes, for example "enemies" or "collectables". A node can be in any number of groups.
+				Nodes can be assigned a group at any time, but will not be added until they are inside the scene tree (see [method is_inside_tree]).
+				Prints an error if this node is already in the group.
+				See notes in the description, and the group methods in [SceneTree].
 				The [code]persistent[/code] option is used when packing node to [PackedScene] and saving to file. Non-persistent groups aren't stored.
 				[b]Note:[/b] For performance reasons, the order of node groups is [i]not[/i] guaranteed. The order of node groups should not be relied upon as it can vary across project runs.
 			</description>
@@ -601,7 +604,9 @@
 			<return type="void" />
 			<argument index="0" name="group" type="StringName" />
 			<description>
-				Removes a node from a group. See notes in the description, and the group methods in [SceneTree].
+				Removes this node from the given [code]group[/code].
+				Prints an error if this node is not in the group.
+				See notes in the description, and the group methods in [SceneTree].
 			</description>
 		</method>
 		<method name="replace_by">
diff --git a/scene/main/node.cpp b/scene/main/node.cpp
index ea9788de276b..387963c730cd 100644
--- a/scene/main/node.cpp
+++ b/scene/main/node.cpp
@@ -1745,11 +1745,8 @@ bool Node::is_in_group(const StringName &p_identifier) const {
 }
 
 void Node::add_to_group(const StringName &p_identifier, bool p_persistent) {
-	ERR_FAIL_COND(!p_identifier.operator String().length());
-
-	if (data.grouped.has(p_identifier)) {
-		return;
-	}
+	ERR_FAIL_COND_MSG(p_identifier.operator String().is_empty(), "Group identifier is empty.");
+	ERR_FAIL_COND_MSG(data.grouped.has(p_identifier), "Node is already in group '" + p_identifier + "'.");
 
 	GroupData gd;
 
@@ -1765,7 +1762,8 @@ void Node::add_to_group(const StringName &p_identifier, bool p_persistent) {
 }
 
 void Node::remove_from_group(const StringName &p_identifier) {
-	ERR_FAIL_COND(!data.grouped.has(p_identifier));
+	ERR_FAIL_COND_MSG(p_identifier.operator String().is_empty(), "Group identifier is empty.");
+	ERR_FAIL_COND_MSG(!data.grouped.has(p_identifier), "Node is not in group '" + p_identifier + "'.");
 
 	HashMap<StringName, GroupData>::Iterator E = data.grouped.find(p_identifier);