Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve errors for add_to_group and remove_from_group, improve documentation #63986

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nathanfranke
Copy link
Contributor

Tested and seems to work as expected, including release builds

$ ./test.x86_64 
Godot Engine v4.0.alpha.custom_build.c80540f9a - https://godotengine.org
Vulkan API 1.2.0 - Using Vulkan Device #0: AMD - AMD Radeon RX 6800 XT
 
ERROR: Node is already in group 'a'.
   at: add_to_group (scene/main/node.cpp:1749)
ERROR: Node is not in group 'a'.
   at: remove_from_group (scene/main/node.cpp:1766)

At first I assumed the ERR_* macros were removed in release builds, but I guess they aren't.

@nathanfranke nathanfranke requested a review from a team as a code owner August 6, 2022 03:12
@nathanfranke nathanfranke requested a review from a team as a code owner August 6, 2022 03:17
@nathanfranke nathanfranke changed the title Improve errors for add_to_group and remove_from_group Improve errors for add_to_group and remove_from_group, improve documentation Aug 6, 2022
@Calinou
Copy link
Member

Calinou commented Aug 6, 2022

At first I assumed the ERR_* macros were removed in release builds, but I guess they aren't.

Since 3.4, error messages are included in release builds to make error reporting from users more relevant: #53405

@Calinou Calinou added this to the 4.0 milestone Aug 6, 2022
@nathanfranke
Copy link
Contributor Author

Since 3.4, error messages are included in release builds to make error reporting from users more relevant: #53405

Looks like that PR only effects error messages, I am more concerned with whether the function will return if COND is true.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 9, 2023
@MewPurPur
Copy link
Contributor

Is this still relevant?

@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@AThousandShips AThousandShips modified the milestones: 4.2, 4.3 Oct 26, 2023
@Mickeon
Copy link
Contributor

Mickeon commented Feb 25, 2024

I believe this PR is no longer relevant, since neither of these methods generate errors anymore since #68564 and their docs have been changed in #68560. Should this be closed?

@AThousandShips AThousandShips modified the milestones: 4.3, 4.4 Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants