godot-escoria / escoria-issues

Central Escoria issue tracker
3 stars 0 forks source link

9 verbs doesn't clear the current command properly with an invalid verb #293

Closed balloonpopper closed 2 years ago

balloonpopper commented 2 years ago

Describe the bug Choosing a verb that doesn't have an associated script set up doesn't reset 9 verbs properly.

To Reproduce Open room 5 Pick up the pen "Use" the pen with the broken pipe

Expected behavior Invalid commands reset the tooltip.

Screenshots

Versions

Additional context Add any other context about the problem here.

ArturM commented 2 years ago

In esc_action_manager.gd when action is finished with ESCExecution.RC_OK we are calling escoria.action_manager.clear_current_action() and then emitting signal action_finished.

If invalid command has to reset tooltip we need to call clear_current_action every time signal action_finished is emmited. Even when this function returns ESCExecution.RC_ERROR.

In L307 we have:

if event_returned[0] == ESCExecution.RC_OK:
    escoria.action_manager.clear_current_action()

so not calling clear_current_action() on error was intentional, but don't see why.

clear_current_action():

# Clear the current action
func clear_current_action():
    set_current_action("")
    set_action_input_state(ACTION_INPUT_STATE.AWAITING_VERB_OR_ITEM)
    emit_signal("action_changed")

Adding calls to clear_current_action() after L327 and L342 fixes this error but maybe I don't see something.

BHSDuncan commented 2 years ago

@ArturM you're probably right, although I'm not exactly sure who did the original implementation for this. If I had to guess, it is the way it is in case there's a (different type of) failure and the user can try again; however, I also think the tooltip should be reset once an action/command is completed, whether it failed or not.

@StraToN thoughts?

StraToN commented 2 years ago

You're both right here: clear_current_action() should be called, and we should also call a tooltip clearing function in that case.