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

simple_nodes: Add SimpleEnding node #277

Merged
merged 1 commit into from
Oct 22, 2024
Merged

simple_nodes: Add SimpleEnding node #277

merged 1 commit into from
Oct 22, 2024

Conversation

dbnicholson
Copy link
Member

This provides a label that displays either a win or lose message along with blocks to set or clear the label during the game. Label is inherited directly so that all the label properties are available in the inspector.

https://phabricator.endlessm.com/T35661

@dbnicholson dbnicholson requested review from manuq and wjt October 18, 2024 17:27
Copy link
Contributor

@manuq manuq left a comment

Choose a reason for hiding this comment

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

Nice, I like how it inherits Label so things like font color can be tweaked (if you can find them).

Only a nit change requested.

Comment on lines +41 to +54
else:
text = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this makes it not work when starting, even adding the "wait for scene to be ready" in the middle:
Captura desde 2024-10-18 15-40-21

Suggested change
else:
text = ""
``

Copy link
Member Author

Choose a reason for hiding this comment

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

It is broken if you try to run it from _ready since simple_setup is run after the node is ready. The problem this is trying to address is that if you set the label text in the editor, it will be serialized to the scene and restored when loading. I tried to figure out how to clear the label text when saving so that it didn't get serialized, but I couldn't figure it out.

I think I'm just going to change this so that SimpleEnding descends from Control and the Label is created at runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated now so the Label isn't saved to the scene. Since the Label is created in simple_setup and that's not run until the scene becomes idle, I had to add a signal and await it to make the blocks work from the when starting block.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated now so the Label isn't saved to the scene. Since the Label is created in simple_setup and that's not run until the scene becomes idle, I had to add a signal and await it to make the blocks work from the when starting block.

Hmm I really liked it when it was inheriting Label, because it allowed the user to change the font, text color, etc. I was even going to propose this inheritance for SimpleScoring. I wasn't expecting a reimplementation like this for that little nit observation :) . Can you go back to it? Is actually unlikely that someone would like to set "you win" or "you lose" when starting.

Copy link
Member Author

Choose a reason for hiding this comment

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

A LabelSettings instance is still exposed in this case, but I can try to go back.

Possibly what I can do is toggle the node visibility from enter/exit_tree when not in editor mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it back to a Label but moved the hack to clear the text to _enter_tree when not in the editor. That seems to work well enough to use the game over blocks from "when starting".

Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent!

This provides a label that displays either a win or lose message along
with blocks to set or clear the label during the game. Label is
inherited directly so that all the label properties are available in the
inspector.

We want the label to be empty when the scene is run, but there may be
text persisted in the scene file. When not in the editor, the text is
cleared when entering the scene tree. Normally this would be done in
_ready, but it's likely that gets replaced by a block script.

https://phabricator.endlessm.com/T35661
@manuq manuq merged commit ebeae59 into main Oct 22, 2024
3 checks passed
@manuq manuq deleted the T35661-simple-ending branch October 22, 2024 10:12
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.

2 participants