Skip to content

Fix dialog InvalidOperationException - #434

Merged
ikkentim merged 1 commit into
ikkentim:masterfrom
duydang2311:433-fix-dialog-task-exception
Oct 15, 2023
Merged

ikkentim merged 1 commit into
ikkentim:masterfrom
duydang2311:433-fix-dialog-task-exception

Conversation

@duydang2311

Copy link
Copy Markdown
Contributor

This PR should close #433. I have changed the response handler from SetResult to TrySetResult to make it safe from the task already completed exception.
However, a better fix might be to not let the response handler executes the second time by destroying the VisibleDialog component when the player responds or disconnects. There are two related callbacks: OnPlayerDisconnect and OnPlayerDialogResponse. But I'm not sure about the execution order of these two callbacks.

@ikkentim

Copy link
Copy Markdown
Owner

However, a better fix might be to not let the response handler executes the second time by destroying the VisibleDialog component when the player responds or disconnects. There are two related callbacks: OnPlayerDisconnect and OnPlayerDialogResponse. But I'm not sure about the execution order of these two callbacks.

It might be a good idea to try this out. To try that, edit the OnPlayerDisconnect event handler and destroy the component there, after it is being handled. player.Destroy();

https://github.com/ikkentim/SampSharp/blob/master/src/SampSharp.Entities/SAMP/Dialogs/DialogSystem.cs#L27

@duydang2311
duydang2311 force-pushed the 433-fix-dialog-task-exception branch from 3d93214 to cba02a9 Compare October 15, 2023 12:26
@duydang2311

Copy link
Copy Markdown
Contributor Author

I also think it's better to do that. OnPlayerDisconnect event handler now should destroy the VisibleDialog component too.

@ikkentim

Copy link
Copy Markdown
Owner

Thank you!

@ikkentim
ikkentim merged commit c09a54e into ikkentim:master Oct 15, 2023
LDami pushed a commit to LDami/SampSharp that referenced this pull request Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exception on player disconnect/crash when a Dialog window is active

2 participants