AnidemDex / Godot-DialogPlugin

🗨️ A Dialog Node for Godot Engine
MIT License
213 stars 11 forks source link

Random missing options #54

Closed Hot-Cuddleccino closed 2 years ago

Hot-Cuddleccino commented 3 years ago

OS

Kde Neon 5.23. (Ubuntu)

Godot Version

3.4. stable

Plugin Version

1.0 (lastest)

Issue description

File: options_node.gd.

Signals were "sometimes" executed in bad order. (around 10% chance for the game run)

If one uses the on_option_selected signal to change dialog text and add new options:

  1. buttons with options are added prior to deletion.
  2. there are then deleted with the old ones.

It is weird that it either works this way or is ok depending on the game start (maybe when signals are connected, I have no inside knowledge on this).

Steps to reproduce

  1. Create an empty project.
  2. Add a new CanvasLayer node with this script
    
    extends CanvasLayer

var dialog = null

func _ready() -> void: dialog = DialogNode.instance() add_child(dialog) dialog.options_manager.connect("option_selected", self, "_on_option_selected") dialog.show() dialog.show_text("Hello world!") dialog.add_option("Hello program!")

func _on_option_selected(option: String) -> void: match option: "Hello program!": dialog.show_text("Hello again!") dialog.add_option("Hello!") "Hello!": dialog.show_text("Hi human") dialog.add_option("Hello program!")

3. Try running the game again and again (I am sorry there is no better way) (Also it seems to be important to shut the game down and run again)
4. Sometimes the second dialog options don't show up

### Workaround

When I tried to change this in the _options_node.gd_:
```js
func add_option(option:String) -> void:
    var option_button:Button = OptionButtonScene.instance() as Button
    option_button.connect("ready", self, "emit_signal", ["option_added", option_button])
    option_button.connect("pressed", self, "_on_OptionButton_pressed", [option])
    option_button.connect("pressed", self, "remove_options")
    option_button.text = option
    add_child(option_button)

func _on_OptionButton_pressed(option:String) -> void:
    emit_signal("option_selected", option)

into this:

func add_option(option:String) -> void:
    var option_button:Button = OptionButtonScene.instance() as Button
    option_button.connect("ready", self, "emit_signal", ["option_added", option_button])
    option_button.connect("pressed", self, "_on_OptionButton_pressed", [option])
    option_button.text = option
    add_child(option_button)

func _on_OptionButton_pressed(option:String) -> void:
    remove_options()
    emit_signal("option_selected", option)

The problem is no longer there (but I cannot promise it won't break anything else)

AnidemDex commented 3 years ago

You're right. I can replicate this and now I know why that happens.

The node is being added in the same frame that all options are being removed. Thank you for pointing this out.

Instead of avoiding removing options, we can deffer the signal call to make sure that every option node was removed before adding new ones.

So this

https://github.com/AnidemDex/Godot-DialogPlugin/blob/64bffb9e252fb09e1a5dec5e654aef2177c0e384/addons/textalog/nodes/dialogue_base_node/options_node/options_node.gd#L16

Will be this

option_button.connect("pressed", self, "emit_signal", ["option_selected", option], CONNECT_DEFERRED|CONNECT_ONESHOT)

test_fix

Hot-Cuddleccino commented 3 years ago

I am glad I could help!

Instead of avoiding removing options, we can deffer the signal call to make sure that every option node was removed before adding new ones.

I wasn't trying to avoid removing, I moved it in the _on_OptionButton_pressed function. I should have made that more clear. In my humble opinion, if you don't mind, I would try to not use one signal connected to two functions in the same node and also the name remove_options doesn't follow expected naming for signal-handling function.

Thank you for making this addon.