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

2.0 Extra Data on Character Event #1138

Merged

Conversation

exelia-antonov
Copy link
Collaborator

Adds an extra_data shortcode parameter to Character events, to allow setting misc instructions on character Update events to give other directives to custom scene portraits, in a hopefully easier to use way than with a call_node to the custom portrait scene. Since this isn't going to be used by most people, this is not added to the Timeline Editor, so is only exposed by the Text Editor or editing the timelines outside of editor.

Updates the PlainPortrait node with the functions for this parameter, and some brief documentation for how to use it when modifying PlainPortrait into a custom scene.

@exelia-antonov exelia-antonov marked this pull request as ready for review August 23, 2022 13:39
@exelia-antonov exelia-antonov marked this pull request as draft August 23, 2022 13:42
@exelia-antonov
Copy link
Collaborator Author

exelia-antonov commented Aug 23, 2022

draft state while I figure out how to preserve it in Timeline Editor if it is set, without the parameter actually visible.

@exelia-antonov exelia-antonov marked this pull request as ready for review August 23, 2022 14:02
@exelia-antonov
Copy link
Collaborator Author

seems to be saving just fine by timeline editor, so marking ready

@Jowan-Spooner
Copy link
Collaborator

Why is the extra data only passed on the Update mode not on Join mode?

@Jowan-Spooner
Copy link
Collaborator

Also as an alternative to making methods mandatory I would always suggest to use has_method() instead.

So instead of

if sprite.does_portrait_accept_extra_data():
	sprite.set_portrait_extra_data(extra_data)

you could do

if sprite.has_method('set_portrait_extra_data'):
    sprite.set_portrait_extra_data(extra_data)

This means one method less, especially a mandatory one.

@exelia-antonov
Copy link
Collaborator Author

So on why only Update and not Join, I'd be open to both, but its the way the functions are constructed. The Event calls the functions from the Subsystem, and a Join first calls a character add and then a character update after that. Therere's already a bunch of overloads to the function for optional parameters already, and I didn't want to add yet another one to the add first to then just pass to the update afterward. Thinking about it today, this particular event it might be better to actually pass a reference to the entire Event Object to the portrait subsystem instead, so it can just pull the values it needs from that

And for the second question, I actually didnt know/remember about the has_method() function. You're right, that would actually be a simpler way to do it, both for this one and the mirroring property too

@exelia-antonov
Copy link
Collaborator Author

alright I changed it to use has_method, and updated a couple other checks for portraits to use them too. revised the documentation for it on DefaultPortrait as well, since that's the template for a custom scene portrait.

and I just ended up adding the extra method to add_portrait after all as well rather than passing the reference to the entire event, since it would take a lot more lines of rewriting to pull the data from the event instead, so its there on Join too now

@Jowan-Spooner
Copy link
Collaborator

Another reason not to do the "passing of an event" on the add_portrait method is, that at least in theory, the methods of the Subsystems are supposed to be usable from outside (so not from events).
That's the design idea at least, even though I'm not sure if it works everywhere.

@exelia-antonov
Copy link
Collaborator Author

ahh, yeah good point it would need to be usable in that way. I guess i just dont like how there's so many parameters with defaults on one function, but i did change it to add the other one onto add after all . I guess the other alternate way would be splitting it into a whole bucnch of individual smaller functions and calling them in sequence

@Jowan-Spooner Jowan-Spooner merged commit 278574b into dialogic-godot:version-2.0 Aug 25, 2022
Jowan-Spooner added a commit that referenced this pull request Aug 25, 2022
Allows to add a string of information on Join and Update:
`Join Emilio 3 [extra_data="Cool Sword"]`
If you use a custom scene, you can add a method called `func set_portrait_extra_data(data: String) -> void:`
If such a method exists, it will be called with the data as the argument.

This has no editor field for now. Showing it in an advanced mode was considered. It can be used in text-mode.

Authored-by: Danielle Cheney <thebardsrc@gmail.com>
@exelia-antonov exelia-antonov deleted the extra-character-option branch September 10, 2022 01:31
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.

None yet

2 participants