var_val bugfix

[PipeYume]

Summary

Bugfixes "fix some issues of var_val"

Purpose of change

Just try these 2 eocs :

[
  {
    "type": "effect_on_condition",
    "id": "EOC_test_varval_1",
    "effect": [
      { "set_string_var": "npc_variable", "target_var": { "context_val": "link_val" } },
      { "set_string_var": "SOMETHING", "target_var": { "var_val": "link_val" } },
      { "u_message": "the u_val_variable: <u_val:variable>" },
      { "u_message": "the global_val_variable: <global_val:variable>" },
      { "u_message": "the context_val_variable: <context_val:variable>" }
    ]
  },
  {
    "type": "effect_on_condition",
    "id": "EOC_test_varval_2",
    "effect": [
      { "set_string_var": "v_link_val_2", "target_var": { "context_val": "link_val_1" } },
      { "set_string_var": "v_link_val_3", "target_var": { "context_val": "link_val_2" } },
      { "set_string_var": "v_link_val_1", "target_var": { "context_val": "link_val_3" } },
      { "set_string_var": "SOMETHING", "target_var": { "var_val": "link_val_1" } }
    ]
  }
]

The issue in "EOC_test_varval_1" is that the value of SOMETHING should be stored in npc_variable but is incorrectly stored in u_variable. And the expected behavior in this case is that it will show a debug message that there is no beta talker.

The issue in "EOC_test_varval_2" is that when running it, the game will crash without any debug messages.

Describe the solution

For issue1, I fixed it by setting the new talker after resolving the var_val.
For issue1, I fixed it by removing the unnecessary parameter talker *talk in function write_var_value, and when it processes var_type::u and var_type::npc, it will try to get the talker from the dialogue instead of using the talker *talk .

Additionally, parsing for values starting with "n_" has been added simultaneously to maintain consistency with math.

For issue2, track the call_depth when processing the var_val. If call_depth > 1000, a debugmsg "Possible infinite loop detected: var_val points to itself or forms a cycle. %s->%s" will be shown.

Describe alternatives you've considered

None

Testing

Additional context

[andrei8l]

For issue1, I fixed it by setting the new talker after resolving the var_val.

AFAICT from a quick look, the fix can go a little deeper and remove the talker argument from write_var_value() since you're also passing the dialogue instance.

You should add a test unit too, since the existing one failed to catch this issue.

Additionally, parsing for values starting with "n_" has been added simultaneously to maintain consistency with math.

I don't like this mixing. Either change it all to math syntax or not at all. IIRC we still can't change everything to math syntax because updating u_add_var would introduce a lot of churn.

For issue2, track the call_depth when processing the var_val. If call_depth > 1000, a debugmsg "Possible infinite loop detected: var_val points to itself or forms a cycle. %s->%s" will be shown.

Better just not resolve var_val recursively. I don't see any situation where that is useful or has a meaningful result. You'll note that recursive evaluation isn't mentioned in the documentation and isn't used anywhere.

[PipeYume]

I don't like this mixing. Either change it all to math syntax or not at all. IIRC we still can't change everything to math syntax because updating u_add_var would introduce a lot of churn.

Changing it to a math-like format might be better, as it currently parses in the form of u_name rather than {"u_val": "name"} or <u_val:name> format.

Better just not resolve var_val recursively. I don't see any situation where that is useful or has a meaningful result. You'll note that recursive evaluation isn't mentioned in the documentation and isn't used anywhere.

The previous code could recursively resolve var_val, and I'm not entirely sure what the intention was at the time. So, should I maintain the status quo, or should I simply remove resolving "v_" through var_val?

[github-actions]

Auto-requesting reviews from non-collaborators: @wapcaplet @andrei8l