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

Rename button_pressed default signal binding to avoid shadowing #79064

Conversation

MewPurPur
Copy link
Contributor

Title says it all :3

@MewPurPur MewPurPur requested review from a team as code owners July 5, 2023 13:57
@MewPurPur MewPurPur force-pushed the rename-button-pressed-THIS-IS-NOT-A-COMPAT-BREAK branch from 4d76dc6 to 05bf32b Compare July 5, 2023 13:57
@YuriSizov YuriSizov added this to the 4.2 milestone Jul 5, 2023
@akien-mga
Copy link
Member

Title says it all :3

But commit seems confused :P

@AThousandShips
Copy link
Member

Documentation for signal hasn't been corrected

@MewPurPur
Copy link
Contributor Author

MewPurPur commented Jul 5, 2023

Oh, my terminal often glitches out on newlines and screws up text... Do I gotta amend it?

And I think I did fix up docs, is there something other than BaseButton.xml I have to fix?

@AThousandShips
Copy link
Member

AThousandShips commented Jul 5, 2023

You have to fix the documentation for the signal, the argument name, you've only updated the virtual function

@akien-mga
Copy link
Member

Oh, my terminal often glitches out on newlines and screws up text... Do I gotta amend it?

Yes, with git commit --amend.

@MewPurPur MewPurPur force-pushed the rename-button-pressed-THIS-IS-NOT-A-COMPAT-BREAK branch from 05bf32b to 3d128f6 Compare July 5, 2023 15:39
@MewPurPur
Copy link
Contributor Author

pokepoke?

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.

Looks good to me. Technically breaks compat for C# if users were using named arguments... but that's not the first nor the last.

@akien-mga akien-mga changed the title Rename button_pressed default signal binding to avoid shadowing Rename button_pressed default signal binding to avoid shadowing Aug 3, 2023
@akien-mga akien-mga merged commit ae8f101 into godotengine:master Aug 3, 2023
@akien-mga
Copy link
Member

Thanks!

@MewPurPur MewPurPur deleted the rename-button-pressed-THIS-IS-NOT-A-COMPAT-BREAK branch August 3, 2023 13:12
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.

4 participants