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

GDScript: Fix POT generator crash on assignee with index #81653

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev added bug topic:gdscript topic:editor crash cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Sep 14, 2023
@dalexeev dalexeev added this to the 4.2 milestone Sep 14, 2023
@dalexeev dalexeev requested a review from a team as a code owner September 14, 2023 14:42
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Not too familiar with this logic, but the changes seem innocuous / making things more reliable.

Copy link
Contributor

@anvilfolk anvilfolk left a comment

Choose a reason for hiding this comment

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

This is also how I was looking at the fix, and I can confirm from my own testing that the approach does work.

Like we discussed there are probably some further improvements possible with making the subscripts check recursive, in case we have something like base.index1.index2["index3"] or something like it... but this way it won't crash :)

@dalexeev
Copy link
Member Author

Note: This may be a non-trivial to cherry-pick for 4.1, since #80020 was merged into 4.2 (not into 4.1 as I thought).

@akien-mga akien-mga merged commit a79955c into godotengine:master Sep 14, 2023
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@dalexeev dalexeev deleted the gds-fix-pot-gen-crash-on-assignee-with-index branch September 15, 2023 03:50
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 20, 2023
@YuriSizov
Copy link
Contributor

Can't cherry-pick cleanly because it depends on #80020. I would appreciate a dedicated PR for 4.1.

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.

Crash on 'Generate POT' when Dictionary-modifying Method is Present
4 participants