Skip to content

Conversation

@TheYellowArchitect
Copy link
Contributor

@TheYellowArchitect TheYellowArchitect commented Dec 10, 2023

Solves a misunderstanding of iterating enums

@AThousandShips AThousandShips changed the title Added enum iteration example Add enum value iteration example Dec 10, 2023
@AThousandShips AThousandShips added enhancement topic:gdscript area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.0 cherrypick:4.1 cherrypick:4.2 labels Dec 10, 2023
@timothyqiu
Copy link
Member

I think you can put your example code snippet directly inside the codeblock above. It's showing how one can use named enum like a dictionary, iterating is one of the operations.

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Dec 10, 2023

I applied all suggested changes (github having built-in batch for suggestions is quite the feature addition)

If you want me to lessen it down (because right now there may be redundance), give the suggestion and I will apply it

@AThousandShips
Copy link
Member

Can apply it as a more global suggestion soon, but you should also squash your commits before this can be merged, see here

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Just need a squash and it looks good

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Dec 10, 2023

First time I squash succesfully. Also changed State to States
Do doublecheck to confirm I didn't do any mistake via squash

@AThousandShips
Copy link
Member

AThousandShips commented Dec 10, 2023

You need to rebase your branch, when squashing you accidentally rewound your branch because you hadn't updated your local fork, please do git rebase -i upstream/master

Also I'd say the naming doesn't matter, leave it as State to reduce the unnecessary changes

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Dec 10, 2023

I apologize for my newbieness. I understand that if someone else did this 2 minute pull request it would be done, so I apologize in advance. At least from this I hope I learn how to rebase for future commits.

=====

git rebase -i upstream/master
outputs
fatal: invalid upstream 'upstream/master' even after I did the below 2 git fetch commands

git fetch https://[githubPersonalTokenKey]@github.com/TheYellowArchitect/godot-docs.git master
outputs

From https://github.com/TheYellowArchitect/godot-docs
 * branch                       master          -> FETCH_HEAD

git fetch https://github.com/godotengine/godot-docs master
outputs

From https://github.com/godotengine/godot-docs
 * branch                       master          -> FETCH_HEAD

^With either of the above, if I git rebase -i [commitId] and select only this one commit
pick 27340cd89 Added enum iteration example
it outputs
Succesfully rebased and updated refs/heads/patch-11

And then I input
git push --force https://[githubkey]@github.com/TheYellowArchitect/godot-docs.git
it outputs
Everything up-to-date

@AThousandShips
Copy link
Member

AThousandShips commented Dec 10, 2023

And if you do git rebase -i master?

If you can't figure it out I can fix your branch for you

@TheYellowArchitect
Copy link
Contributor Author

And if you do git rebase -i master?

I think it worked out. Do confirm I didn't mess up the history 😅

@AThousandShips
Copy link
Member

Use git commit --amend when fixing, and then git push --force

# Use constant dictionary functions
# prints '["STATE_IDLE", "STATE_JUMP", "STATE_SHOOT"]'
print(State.keys())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I restore the whitespace here? If yes, should I place it between the below print statements so they aren't a united codeblock?

Copy link
Member

Choose a reason for hiding this comment

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

Put it after the last line of code, keep two lines before "Functions", easier to read

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Dec 10, 2023

Above commit should be good. That said, I apologize for taking so much of your precious time on such a simple/frequent task 😓

@AThousandShips
Copy link
Member

Thank you for bearing with the review 🙂 looks great!

@mhilbrunner
Copy link
Member

Indeed, thanks for being accomodating and doing all the work (and for taking your time to contribute in the first place!). And don't worry, we all have to learn some time, no one knows everything. Always feel welcome to ask for help in the comments or in the #documentation channel in contributors chat. Thank you!

And thanks for reviewing ATS :)

@mhilbrunner mhilbrunner merged commit 8209021 into godotengine:master Dec 11, 2023
Comment on lines +1242 to +1243
# prints '["STATE_IDLE", "STATE_JUMP", "STATE_SHOOT"]'
print(State)
Copy link
Member

@timothyqiu timothyqiu Dec 12, 2023

Choose a reason for hiding this comment

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

Wait...

This is not true. State is a Dictionary so it should print { "STATE_IDLE": 0, "STATE_JUMP": 5, "STATE_SHOOT": 6 }. Or it should be print(State.keys()) instead.

@mhilbrunner
Copy link
Member

Cherry-picked to 4.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.0 cherrypick:4.1 enhancement topic:gdscript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GDScript - Iterating enum integers casts them to string

4 participants