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

Fix instancing new Character Nodes on Simple Highlight Portrait #2486

Merged

Conversation

CakeVR
Copy link
Collaborator

@CakeVR CakeVR commented Nov 21, 2024

This portrait is simple and allows for only one image, there is no underlying node hierarchy that may need to change. Therefore, we need to ensure to not recreate a new character node.

We should look at handling this behaviour, to keep highlights working for other portrait scenes, that create a new character instance, too.

This portrait is simple and allows for only one image, there is no underlying node hierarchy that may need to change.
Therefore, we need to ensure to not recreate a new character node.
@CakeVR CakeVR added the Bug 🐞 Something isn't working label Nov 21, 2024
@Jowan-Spooner
Copy link
Collaborator

This seems correct. Looks like I changed the default for this method a while ago and didn't change this implementation. Thanks.

I think custom portraits that create new scenes every time should just check against the current speaker in _ready to generate the right highlight, but we could possibly add a helper method like is_current_speaker() either directly to the DialogicPortrait class or to the Portrait subsystem...

@CakeVR CakeVR merged commit 7e38e1d into main Nov 22, 2024
5 checks passed
@CakeVR CakeVR deleted the fix-instancing-character-nodes-on-simple-highlight-portrait branch November 22, 2024 20:50
@salianifo
Copy link
Contributor

This uses spaces for indentation instead of tabs causing Godot not to parse the file correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants