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

[#1933] Validate impulse_request values #1934

Merged
merged 5 commits into from
Apr 4, 2024

Conversation

oznogon
Copy link
Contributor

@oznogon oznogon commented Mar 4, 2023

#1933 shows that directly setting impulse_request on a SpaceShip can result in unusable or poorly defined values, such as infinity/-infinity from dividing by 0 impulse speed on Odins and Defense Platforms.

Add a setImpulseRequest() function that clamps the passed value and replace direct modifications of impulse_request with it.

Fixes #1933.

src/ai/ai.cpp Outdated Show resolved Hide resolved
@daid
Copy link
Owner

daid commented Apr 3, 2024

I see I totally forgot to reply to this.

While the idea is feels sound on the first glance. There is an issue with it. It's still dividing by zero, and assuming that it will produce "infinity". But as we are compiling with -funsafe-math-optimizations this does not have to be true, and division by zero is can be UB or an exception, so we should avoid it in the first place.

@oznogon
Copy link
Contributor Author

oznogon commented Apr 3, 2024

Added a check to confirm that impulse_max_speed is greater than 0 before attempting setImpulseRequest.

@daid daid merged commit e60b242 into daid:master Apr 4, 2024
7 checks passed
Tsht pushed a commit to Tsht/EmptyEpsilon that referenced this pull request Oct 12, 2024
* [SpaceShip] Add setImpulseRequest() function

* [AI] Use setImpulseRequest() to ensure valid request values

* [AI] Avoid division by 0 by validating impulse_max_speed

* [AI] Make it make sense

* [AI] No really
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.

[AI] CpuShips with 0 max_impulse_speed can set infinite impulse_request values
2 participants