Skip to content

GDScript: Don't get invalid dictionary key during completion #108167

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vnen
Copy link
Member

@vnen vnen commented Jul 1, 2025

We try to get the value out of a dictionary in order to establish its type for completion purposes. However, if the dictionary or the key is not a constant, we cannot safely get the actual value, so we skip this and just try to infer from static typing.

Note

This also adds a change in core: it removes type validation from Dictionary.has() (and has_all()) because it simplifies checking if a given key is valid without having to check its type externally.

Fix #102926
Supersedes #104757

@vnen vnen added this to the 4.5 milestone Jul 1, 2025
@vnen vnen requested a review from a team as a code owner July 1, 2025 15:37
@vnen vnen marked this pull request as draft July 1, 2025 15:41
@vnen vnen force-pushed the gdscript-fix-getting-invalid-dict-key-completion branch from c1031c2 to a44885e Compare July 1, 2025 18:11
vnen added 2 commits July 1, 2025 15:12
This allows users to call this function to check if a key is set without
having to worry about checking types, which is quite cumbersome
currently.
We try to get the value out of a dictionary in order to establish its
type for completion purposes. However, if the dictionary or the key
is not a constant, we cannot safely get the actual value, so we skip
this and just try to infer from static typing.
@vnen vnen force-pushed the gdscript-fix-getting-invalid-dict-key-completion branch from a44885e to 7cf8e5d Compare July 1, 2025 18:12
@vnen vnen added the topic:core label Jul 1, 2025
@vnen vnen marked this pull request as ready for review July 1, 2025 18:14
@vnen vnen requested a review from a team as a code owner July 1, 2025 18:14
@@ -217,14 +217,12 @@ bool Dictionary::is_empty() const {

bool Dictionary::has(const Variant &p_key) const {
Variant key = p_key;
ERR_FAIL_COND_V(!_p->typed_key.validate(key, "use 'has'"), false);
Copy link
Member

Choose a reason for hiding this comment

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

This sort of makes sense to me, but it's kind of inconsistent with the rest of the API.
I'd rather add a function like const ContainerTypeValidate &get_key_validator(), and allow querying it if the given key is compatible. Something like that, anyway.

In any case, this change should be run by the core meeting, I think.

@Ivorforce
Copy link
Member

Ivorforce commented Jul 9, 2025

Following the core meeting, here's my suggestion:

  1. Expose validators through Dictionary API:
const ContainerTypeValidate &get_key_validator();
const ContainerTypeValidate &get_value_validator();
  1. Add a bool-based validation method to ContainerTypeValidate, mostly copying validate, ala
bool can_validate_variant(const Variant &p_variant) const
  1. Use the new functions from autocomplete to avoid error messages

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.

ERROR: Attempted to getptr a variable of type 'Nil' into a TypedDictionary.Key of type <T>.
2 participants