Skip to content

Conversation

@wnbaum
Copy link
Contributor

@wnbaum wnbaum commented Jun 19, 2024

This is just a really small fix for an error that was happening in the pong game. For previous classes with custom blocks, like SimpleCharacter, you had to define a method that specified the base class so the picker would know which blocks to populate itself with.

However, there is a function script.get_instance_base_type() that gets this automatically from whichever class the script extends. Much nicer than having to define a function for this every time.

@wnbaum wnbaum requested review from dylanmccall, manuq and wjt June 19, 2024 22:01
@manuq
Copy link
Contributor

manuq commented Jun 19, 2024

Ah yes, I noticed this in the paddle blocks too. Paddles inherit from CharacterBody2D but they weren't getting Modulate from the parent CanvasItem class, position from the Node2D class, etc. Please add the explanation in the commit message, not in the PR message, then merge.

For previous classes with custom blocks, like SimpleCharacter, you had
to define a method that specified the base class so the picker would
know which blocks to populate itself with.

However, there is a function script.get_instance_base_type() that gets
this automatically from whichever class the script extends. Much nicer
than having to define a function for this every time.

This change also fixes an error when opening the pong game.
@wjt wjt force-pushed the fix-base-class-identification branch from 4e2df97 to 78e9390 Compare June 20, 2024 09:34
@wjt
Copy link
Member

wjt commented Jun 20, 2024

I have reworded the commit message with your words, @wnbaum.

If you open a pull request for a single commit, its commit message will be used as the PR's title & body. So there is no extra overhead to writing a longer commit message – and there is an upside that when someone is looking through the commit history later to understand a change, the full explanation is right there.

@wjt wjt merged commit 944a25c into main Jun 20, 2024
@wjt wjt deleted the fix-base-class-identification branch June 20, 2024 09:36
@wnbaum
Copy link
Contributor Author

wnbaum commented Jun 20, 2024

I have reworded the commit message with your words, @wnbaum.

If you open a pull request for a single commit, its commit message will be used as the PR's title & body. So there is no extra overhead to writing a longer commit message – and there is an upside that when someone is looking through the commit history later to understand a change, the full explanation is right there.

Great! Good to know, thank you!

@wnbaum wnbaum mentioned this pull request Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants