Skip to content

Add an optional default return value argument to Object.get()#80841

Open
nevarek wants to merge 1 commit intogodotengine:masterfrom
nevarek:object_get_default_param
Open

Add an optional default return value argument to Object.get()#80841
nevarek wants to merge 1 commit intogodotengine:masterfrom
nevarek:object_get_default_param

Conversation

@nevarek
Copy link
Copy Markdown
Contributor

@nevarek nevarek commented Aug 21, 2023 •
edited by YuriSizov
Loading

Implements feature request mentioned here godotengine/godot-proposals#3841

Production edit: Closes godotengine/godot-proposals#3841

@nevarek nevarek requested review from a team as code owners August 21, 2023 07:32
@nevarek nevarek requested a review from a team August 21, 2023 07:32
@nevarek nevarek requested review from a team as code owners August 21, 2023 07:32
@nevarek nevarek requested a review from a team August 21, 2023 07:32
@nevarek nevarek requested a review from a team as a code owner August 21, 2023 07:32
@nevarek nevarek requested review from a team August 21, 2023 07:32
@nevarek nevarek requested review from a team as code owners August 21, 2023 07:32
@nevarek nevarek force-pushed the object_get_default_param branch from 27003d8 to 0ae3a7b Compare August 21, 2023 07:34
@dalexeev dalexeev added this to the 4.x milestone Aug 21, 2023
@nevarek
Copy link
Copy Markdown
Contributor Author

nevarek commented Aug 21, 2023

Oh shoot I forgot about tests, I'll update those in the near future

@AThousandShips
Copy link
Copy Markdown
Member

AThousandShips commented Aug 21, 2023 •
edited
Loading

Needs compatibility methods and info added as this changes the API, there are also tests that need to be updated which use the original syntax

I'm unsure if changing the method in c++ is the right way to go, instead of adding a bind method to handle that, it affects the entire codebase in a significant way, not sure why the c++ side needs to change

AThousandShips
AThousandShips reviewed Aug 21, 2023
View reviewed changes
Comment thread core/object/object.cpp
Variant Object::get(const StringName &p_name, const Variant &p_default_value) const {
bool is_defined = false;
const Variant result = get_or_null(p_name, &is_defined);
if (result.is_null() and !is_defined) {
Copy link
Copy Markdown
Member

@AThousandShips AThousandShips Aug 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (result.is_null() and !is_defined) {
if (result.is_null() && !is_defined) {

@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Sep 5, 2023

Right, this seems to be making too many changes...
I think it's better to add a get_or_default() method instead, although it's inconsistent with other get-like methods, so not sure.

@YuriSizov YuriSizov removed request for a team September 5, 2023 15:43
@YuriSizov YuriSizov removed request for a team September 5, 2023 15:43
@Repiteo Repiteo requested review from a team as code owners February 17, 2026 20:11
@AdriaandeJongh AdriaandeJongh removed the request for review from a team February 18, 2026 10:54
@dmlary
Copy link
Copy Markdown
Contributor

dmlary commented Mar 2, 2026

@AThousandShips and @KoBeWi,

I came across this PR after hitting the inconsistency between Object.get() and Dictionary.get() supporting a default value argument. Reading through the comments, I see a possible path forward.

Would y'all accept updating only Object._get_bind() to take the second argument, with the default value of null? The change would only affect GDscript, and existing code would be unaffected by the change (still get null if not present). It would bring Object.get() in line with other get() functions in GDscript.

Also @KoBeWi , your comment seemed non-committal on whether any change to the get() behavior in GDscript would be accepted. Is that still the case?

@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Mar 2, 2026 •
edited
Loading

Changing the binding also affects C# and GDExtension. It might at least require a compatibility method.

your comment seemed non-committal on whether any change to the get() behavior in GDscript would be accepted. Is that still the case?

Yeah. If modifying get() can be done without lots of changes then it's fine.

@dmlary
Copy link
Copy Markdown
Contributor

dmlary commented Mar 2, 2026

It might at least require a compatibility method.

@KoBeWi Can you point me at an existing compatibility method? I don’t know the term.

@dmlary
Copy link
Copy Markdown
Contributor

dmlary commented Mar 10, 2026

Hey, following up on this. This looks like a simple change, but I still don't have any context for what a "compatibility method" is in the godot source. Could you link an existing one, or just give me the name of one, and I can sort it out from there.

@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Apr 6, 2026

https://docs.godotengine.org/en/stable/engine_details/development/handling_compatibility_breakages.html

@dmlary dmlary mentioned this pull request Apr 18, 2026
Open
dmlary added a commit to dmlary/godot that referenced this pull request Apr 18, 2026
@dmlary
Add default value for GDScript Object.get()
99a1050 
Dictionary.get() in GDScript supports a second argument that is the value to
return if the key does not exist in the Dictionary.  This commit adds that
behavior to Object.get() in GDScript.

Supercedes godotengine#80841
Closes godot-proposals#3841
dmlary added a commit to dmlary/godot that referenced this pull request Apr 18, 2026
@dmlary
Add default value for GDScript Object.get()
5b1c632 
Dictionary.get() in GDScript supports a second argument that is the value to
return if the key does not exist in the Dictionary.  This commit adds that
behavior to Object.get() in GDScript.

Supercedes godotengine#80841
Closes godot-proposals#3841
dmlary added a commit to dmlary/godot that referenced this pull request Apr 18, 2026
@dmlary
Add default value for GDScript Object.get()
47488c3 
Dictionary.get() in GDScript supports a second argument that is the value to
return if the key does not exist in the Dictionary.  This commit adds that
behavior to Object.get() in GDScript.

Supercedes godotengine#80841
Closes godot-proposals#3841
dmlary added a commit to dmlary/godot that referenced this pull request Apr 18, 2026
@dmlary
Add default value for GDScript Object.get()
728b98e 
Dictionary.get() in GDScript supports a second argument that is the value to
return if the key does not exist in the Dictionary.  This commit adds that
behavior to Object.get() in GDScript.

Supercedes godotengine#80841
Closes godot-proposals#3841
dmlary added a commit to dmlary/godot that referenced this pull request Apr 18, 2026
@dmlary
Add default value for GDScript Object.get()
5a5e677 
Dictionary.get() in GDScript supports a second argument that is the value to
return if the key does not exist in the Dictionary.  This commit adds that
behavior to Object.get() in GDScript.

Supercedes godotengine#80841
Closes godot-proposals#3841
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AThousandShips AThousandShips AThousandShips left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

enhancement topic:core

Projects

None yet

Milestone

4.x

Development

Successfully merging this pull request may close these issues.

Add an optional default return value argument to Object.get()

5 participants

@nevarek @AThousandShips @KoBeWi @dmlary @dalexeev