Executer, Variables in string are only replaced in SAY

[theludovyc]

When you put a variable in ask like "How old are you <gd.name> ?"

The variable is not replaced. It is only works for SAY.

[Jeremi360]

When you put a variable in ask like "How old are you <gd.name> ?"
The variable is not replaced. It is only works for SAY.

@theludovyc
You checked only ask ? We also need this to work in menu choices.

We must fix it, I think we could miss it because the Visual Novel Kit used a parser, which also had such functionality for variables and this meant that this error was not seen before.

[theludovyc]

You checked only ask ? We also need this to work in menu choices.

On all strings :)

[Jeremi360]

I'm working on it, @theludovyc , but I see that you make parsing strings for say in Executer.gd and not in Parser.gd can I know way ?

[theludovyc]

What do you want to mean by that ? 😊

[Jeremi360]

What do you want to mean by that ? 😊

@theludovyc I'm just curious way this is done in Executer.gd and not in Parser.gd.

But it dones't matter as I done this in #256 , I need just to test it - I will do it soon.
And maybe you can give better name for funcion I just than do_variables()

[theludovyc]

I not get it what you mean by parse string in executer and not in parser ?

You can name it: replace_variables()

[Jeremi360]

I not get it what you mean by parse string in executer and not in parser ?

@theludovyc never mind, I already understood why it must be done in Executer.gd and not in Parser.gd

You can name it: replace_variables()

Yes this is good idea.

[Jeremi360]

@theludovyc Unit Test I made for Variables in String gvies me pass for every one test 😁 :

But is this a good test ? I don't have much experience with those so I'm not sure:
https://github.com/Jeremi360/Rakugo/blob/do_variables/Test/TestExecuter/TestVarsInStrings/TestVarsInStrings.gd

[theludovyc]

Yes good, but at wrong places. You do not need to recreate one. Add them in TestSay, testMenu and testAsk. And you should verify the test end properly, with wait_execute_script_finished 😊

[Jeremi360]

You should verify the test end properly, with wait_execute_script_finished 😊

@theludovyc
I made one test do it simpler to test this one feature, and make me sure that I understat how to use GUT as it new for me.

I added assert_menu_return(0) before wait_execute_script_finished() and now it passes all checks 😎

[Jeremi360]

Yes good, but at wrong places. You do not need to recreate one. Add them in TestSay, testMenu and testAsk.

@theludovyc
No, no, no there is no sense in testing it using Paser.gd as replace_variables() is done by Executer.gd as it need to be done at run time so it reacts to changes in variables. If it was done by Paser.gd then value of variable in string would be the same:

a = 2
"a is <a>" # -> a is 2
a = 4
"a is <a>" # -> a is 2 # it wouldn't react to change in line above

Also there is already test TestSay.test_say_variable() made by you, but it doesn't make any sense as it dosen't define variable or checks it value in say string it self:

Rakugo-Dialogue-System/Test/TestParser/TestSay/TestSay.gd

Lines 54 to 73 in 9a7efde

	func test_say_variable():
	var rk_script = [
	"\"My name is <sy.name>\""
	]
	var parsed_script = parser.parse_script(rk_script)
	assert_false(parsed_script.is_empty())
	var parsed_array = parsed_script["parse_array"]
	assert_false(parsed_array.is_empty())
	assert_true(parsed_script.get("labels", [""]).is_empty())
	assert_true(parsed_array[0][0] == "SAY")
	var result = parsed_array[0][1]
	assert_eq(result["text"], "My name is <sy.name>")

[theludovyc]

When we split parser and executer we start to split tests too. But the job is not finished yet and you have test in Parser folder that parse and execute. But you have some tests in Execute folder. And if I am not wrong we have one for Say and Ask, maybe not Menu. You can update them to make them better. You do not need to create variable from an rkscript you can use Rakugo.set_variable if needed.

[Jeremi360]

@theludovyc never mind, Okay my bad, there are TestSay, testMenu and testAsk in TestExecuter.
Now I will move tests from TestVarsInStrings.gd to proper test in TestExecuter, sorry for the confusion.

[Jeremi360]

@theludovyc I see why I made this mistake and was so confused, by mistakie you connected TestParser scripts to TestExcute scenes:

Rakugo-Dialogue-System/Test/TestExecuter/TestAsk/TestAsk.tscn

Line 3 in 9a7efde

[ext_resource type="Script" path="res://Test/TestParser/TestAsk/TestAsk.gd" id="1"]

Rakugo-Dialogue-System/Test/TestExecuter/TestMenu/TestMenu.tscn

Line 3 in 9a7efde

[ext_resource type="Script" path="res://Test/TestParser/TestMenu/TestMenu.gd" id="1"]

Rakugo-Dialogue-System/Test/TestExecuter/TestSay/TestSay.tscn

Line 3 in 9a7efde

[ext_resource type="Script" path="res://Test/TestParser/TestSay/TestSay.gd" id="1"]

I will fix that!

[Jeremi360]

@theludovyc I fixed this and moved all test from TestVarsInStrings.gd to proper test in TestExecuter, but I need to fix one thing a = "a<b>" works in script, but not in Rakugo.set_variable() or Rakugo.set_character_variable() but it easy to fix.

[theludovyc]

Woups... Sorry about that, I never saw it because of gut. Maybe the scene is not needed ? Thank you to handle it 😊

[Jeremi360]

@theludovyc Now it works and pass all test and I added test for dynamic use <var> in side other variable string:

test_var = "a"
test_var_b = "<test_var>b"
test_var = "c"

and test it passes is 100%:

func  test_variables_in_strings():
	watch_rakugo_signals()

	await wait_parse_and_execute_script(file_path)

	assert_variable("test_var", TYPE_STRING, "c")

	assert_variable("test_var_b", TYPE_STRING, "cb")

	await wait_execute_script_finished(file_base_name)

[Jeremi360]

@theludovyc do you think I can merge #256 ?

[theludovyc]

No, I will make a review when I will be at home. It the meantime you can squash your commits and remove unrelated files 😊

[Jeremi360]

@theludovyc What "unrelated files" you mean ?
If your are talking about files in TestVarsInStrings dir, then it is alredy removed.

[theludovyc]

Review is ready 😊 (maybe next time we can talk all about the technical stuff in the pr and not in the issue 😅)