Support function calls from text

[RakuJa]

Rationale

I've opened this PR because I love this project but comparing it with other libraries I'm seeing some built-in features that are missing (maybe I'm wrong!). The big one for me are dynamical texts, for example an actor changing the displayed name (without creating another actor) or being able to change a text based on the character name. Something like:

"Hello {{get_player_name()}}" => "Hello Alice"
* Conditional option in which the MC may ask the name of an NPC * -> "???: I'm Alice" -> "Alice: Now that you know my name [...]"

The first one there is no way to do it without modifying the default behavior, the second one is possible only with verbose graphs adding a completely duplicated path, if the mc asked then the actor name will be "Alice" Otherwise "???".
With this functionality we can keep the name as {{get_npc_name()}} and it can change dynamically

Code

The implementation is inspired by the idea of enabling the developer to enclose function calls inside double brackets and execute it. The code uses regex to search for the desired pattern (very broadly, does not check for syntax correctness) and then identifies the function that will be called and the arguments. => fun() OK, fun("HI") OK, fun("HI".asdasdas) OK
It will then parse using str_to_val to validate the function parameters

Issues

I see two big issues with the current state of the PR:

1. Arbitrary choice of delimeters, if a developer wants to display to the user the string "{{fun()}}" then this feature will try to execute the code and show an empty string instead
2. The library seems to be focused on the idea of graphically letting the developer build dialogues using the graph editor, with this we are adding behavior that is not as intuitive

[jonnydgreen]

Hi, thanks for submitting this PR - great idea, I really like it! I'm currently on holiday ATM but will have a look in detail as soon as I'm back :)

[RakuJa]

Enjoy the holidays! Once you'll be back, if you deem the idea worthy, I'll be available to work on it following your directions, this was just a raw draft to present the idea

[jonnydgreen]

Cheers, I've had a look this morning and firstly, I just wanna say again - great idea! I like the convention you've chosen: Hello {{expression}}, how're you?

I have a couple of ideas which would be good to get your thoughts on too!

I wonder if we should move this processing into: addons/parley/models/dialogue_sequence_ast.gd and resolve the defined function calls within the next() function so the returned nodes are always fully resolved. This is where the current processing happens so it would be good to keep all that together (this also has a load of tests that we can also extend for this use-case). Also, as this new feature is quite low level, users would then have the flexibility to use those APIs to process Dialogue Sequences with their own custom dialogue containers.

For something similar a while back, I was looking into Godot Expressions. I reckon this could provide us with some out of the box functionality so we don't have to write the function parse/call code ourselves. This would also provide some extra flexibility in that it's not restricted to only function calls but in theory, any valid expression. E.g. we can use provided arguments to the processing such as ctx (and others in the future).

Let me know what you think and happy to discuss!

[jonnydgreen]

On a side note, I'm currently implementing translation support which is likely to be touching similar parts of the codebase so we may need to coordinate a bit. Nothing to worry about ATM, just thought I'd mention it as a heads up :)

[RakuJa]

I agree with you that Godot expressions are a way better approach to this task, I've tested them and they work well for this use case.
I also think that moving the parsing logic where you've suggested is a good starting point but I'm also playing around with the idea of adding a new kind of node.

Let's call this new node "rich_dialogue", it would:

1. Avoid adding breaking changes for those users that have already used the special syntax;
2. Help us in the future, keeping dialogue_node as "barebone" and stable as possible while rich_dialogue can be expected to have more post-processing.

What do you think?

[jonnydgreen]

Awesome stuff! :)

I'm also playing around with the idea of adding a new kind of node.

That's a really good point about the potential breaking change!

That's a cool idea about the new rich node as well - I like the separation to increase stability/avoid breaking changes. I think my only concern about having an extra node type is that it could increase the complexity for both the user (and us) as they would have more to think about in terms of what nodes to add and a dialogue sequence would become cluttered with lots of different node types! For example, if we introduce rich_dialogues we'd probably want to introduce rich_dialogue_option nodes as well!

I wonder if it would be simpler and easier for the user for us to stick with one type of node but we extend the existing functionality of the dialogue and dialogue_option nodes in a defensive manner to avoid the breaking changes and stability concerns you raised?

For example, with this approach, we could make this post-processing configurable in parley settings or per-node options that are (in this version of Parley) both opt-in. This way, users can still use existing nodes but can optionally add in the extra processing if they'd like to. In subsequent major versions of Parley, we could then turn them on by default and have this functionality out of the box.

wdyt?

[RakuJa]

I've come back to work on this issue with some news:

we could make this post-processing configurable in parley settings

I find this approach easy to code and I've already tested it, working without issues. I like the idea of adding per-node options, leading to a more granular control but I have no idea on how to implement this approach in the current code-base, is there some code that I can extend?

"Hello {{get_player_name()}}" => "Hello Alice"

I've implemented text parsing but

{{get_voice_name()}}: I'm Alice -> {{get_voice_name()}}: Now that you know my name [...]
"???: I'm Alice" -> "Alice: Now that you know my name [...]"

Character name parsing is proving difficult with the current approach since we move around an object with id and name, parsing it would mean saving the new parsed name in storage, rendering the functionality useless and broken.

Do you have any suggestions with character name parsing and per-node options?

[jonnydgreen]

Ah awesome stuff, sounds like great progress so far! 🚀

but I have no idea on how to implement this approach in the current code-base, is there some code that I can extend?

At the moment, your best starting point is to look at extending the following files (and maybe start with a Checkbox Button):

• addons/parley/models/dialogue_node_ast.gd - defines the shape of the data
• addons/parley/components/dialogue/dialogue_node_editor.gd - governs the node editor you see in the dock
• addons/parley/components/dialogue/dialogue_node_editor.tscn - governs the node editor you see in the dock

Once things are developed a bit further, we'll need to extend some other files but these are a good starting point :) I'll try and write up a proper contributing guide tomorrow evening - let me know if that helps or you have any further questions in the meantime!

Do you have any suggestions with character name parsing

Good question! I think I see what you mean yeah as I may have run into similar issues when working on adding translation support last week. Just so I fully understand your question, do you have some example code I can take a look at?

[RakuJa]

Sorry for the long wait, but I've had some personal stuff to work on.

I've pushed the cleaned up version of my work. It adds a global settings to add rich text rendering both on the dialogues and dialogue options. It also renders character names.

Example of node editor

Example of result

It's still missing the granular option to enable/disable rich text rendering for each node and I'm also open to change around code implementation.
P.S the character name is {{sqrt(4)}}, once the settings is disabled it returns as {{sqrt(4)}}

[RakuJa]

I'm now experimenting on the granular render (per node) and I've found some issues to discuss:

Having global settings + granular option on every node can be implemented but the logic to be applied needs to be discussed. I'm just writing the first two ideas that came to mind but there are more complex solutions that could be chosen.

Global setting overwrites granular access

• If the user sets the global setting to "ON" it will force each node to render
• If the user sets the global setting to "OFF" it will disable rendering for every node
• If the user sets the global setting to a value (ON/OFF) and later on modifies the node the nodes will keep the granular settings.

Pros

• Simplicity;
• The behavior is streamlined.

Cons

• It can overwrites lots of nodes granular data with a simple mistake

Granular setting decides

In case of conflict (Global ON, local OFF) the local settings is chosen, ignoring global settings
The global settings is used ONLY for initialization of node
e.g if the global settings is ON when node is created => node setting is ON
if the global settings is OFF when node is created => node setting is OFF

Pros

• No overwrites caused by global settings, no data lost;

Cons

• Very hard to backport to existing codebases;
• Not intuitive

[jonnydgreen]

Sorry for the long wait, but I've had some personal stuff to work on.

No problem at all and absolutely no rush! :)

Thanks for pushing up the code, it's looking great from an initial glance! I'm busy this evening but I'll take a closer look and answer your questions tomorrow/this weekend!

[jonnydgreen]

Good question about the granular settings. I think I'm leaning towards "Granular setting decides" but with the following difference:

• The global settings is used as a fallback (e.g. when the node-setting is not present on the node)
• Once the node setting is set, it then always overrides the global setting until it's unset

I think the flexibility we'd gain with this approach follows the idea that the lowest level settings/config have the most priority which imo outweighs the cons. And as you pointed out, it still ensures no data is lost.

This is just my initial thought - I need to think about it a bit more though as I may have missed something! :)

wdyt?

[jonnydgreen]

Looking great so far!! 🚀

[jonnydgreen]

Just FYI, we may need to coordinate with this PR here: #28

I was thinking about it earlier and reading around for best practices and I think for the most flexible and easy way of defining translations, the function calls should probably run after the translations are run. wdyt?

[jonnydgreen]

Nice! I was gonna ask about multiple expressions!