Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #7188: check the validity of command callback for scripts #7701

Merged
merged 2 commits into from Sep 7, 2019

Conversation

glx22
Copy link
Contributor

@glx22 glx22 commented Aug 17, 2019

When a script is reloaded while waiting for the result of a command, this result will be sent to the new instance, and as the new instance is not expecting it, it can result in a crash.

@glx22 glx22 added the backport requested This PR should be backport to current release (RC / stable) label Aug 17, 2019
@nielsmh
Copy link
Contributor

nielsmh commented Sep 6, 2019

This looks very correct, but I can't think of a (good) way to set up a test of it.

The bug #7188 triggers if a script is reloaded after sending a command, before the command has been handled next tick, so some kind of single stepping mechanism?

@LordAro LordAro merged commit b3fd787 into OpenTTD:master Sep 7, 2019
@glx22 glx22 deleted the ccai_check branch September 7, 2019 16:38
@nielsmh
Copy link
Contributor

nielsmh commented Sep 7, 2019

Turns out I was too quick in approving this, we all forgot to test it in multiplayer. There, the command handling adds some flags to the cmd value which makes it no longer compare equal and kills off every AI on its first command. #7725 should fix that.

@LordAro LordAro added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants