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

Implement typed dictionaries #1162

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Jul 3, 2023

A godot-cpp counterpart to PR godotengine/godot#78656

Tests/builds will fail until the above is merged, as this PR is dependant on it.

@Repiteo Repiteo requested a review from a team as a code owner July 3, 2023 17:58
@Repiteo Repiteo marked this pull request as draft July 3, 2023 18:27
@Repiteo Repiteo force-pushed the typed-dictionary branch from 034b415 to eec44d2 Compare July 3, 2023 18:28
@Repiteo Repiteo force-pushed the typed-dictionary branch from eec44d2 to 191037d Compare July 18, 2023 14:56
@Repiteo Repiteo force-pushed the typed-dictionary branch 2 times, most recently from 65f0881 to f5b340a Compare October 2, 2023 16:57
@Repiteo Repiteo marked this pull request as ready for review September 6, 2024 21:29
@Repiteo Repiteo added this to the 4.4 milestone Sep 6, 2024
@Repiteo Repiteo force-pushed the typed-dictionary branch 3 times, most recently from 2775b10 to fd05eca Compare September 17, 2024 17:39
@dsnopek dsnopek added the enhancement This is an enhancement on the current functionality label Sep 18, 2024
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me :-)

@dsnopek dsnopek merged commit 57bd88a into godotengine:master Sep 19, 2024
12 checks passed
@Repiteo Repiteo deleted the typed-dictionary branch September 19, 2024 19:12
@@ -2765,6 +2827,12 @@ def correct_typed_array(type_name):
return type_name


def correct_typed_dictionary(type_name):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like it's unused?

fully_used_classes.add(dict_type_name)
else:
used_classes.add(dict_type_name)
dict_type_name = dict_type_names[2]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why index 2? That would be the third entry. I think dictionary only have 2 types?

fully_used_classes.add(dict_type_name)
else:
used_classes.add(dict_type_name)
dict_type_name = dict_type_names[2]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why index 2? That would be the third entry. I think dictionary only have 2 types?

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 29, 2024

@Repiteo Any thoughts on Zylann's comments above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants