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 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.

@nielsmh

This comment has been minimized.

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?

@nielsmh
nielsmh approved these changes Sep 7, 2019
@LordAro LordAro merged commit b3fd787 into OpenTTD:master Sep 7, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190817.8 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.8) Linux linux-amd64-clang-3.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@glx22 glx22 deleted the glx22:ccai_check branch Sep 7, 2019
@nielsmh

This comment has been minimized.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.