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

Convert axisAlignedCollision tasts to Catch2 and improve docs #15427

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JosiahWI
Copy link
Contributor

@JosiahWI JosiahWI commented Nov 12, 2024

This is just cleanup. The content of the unit tests should not be changed, although one redundant test was removed. It looked like the author copy-pasted a test, intending to change the test parameters, but forgot to change them. Since I don't know what the author intended to change, I took it back out. I have not converted the collisionMoveSimple tests that were recently added, and I do not have time to in the near future.

I have also reworded the docstring of the function under test to clarify that dtime is an inout parameter. The original wording does not seem to mention its use as an input to the function. This information came from @appgurueu, and I observed it to be correct based on the unit test behavior; I have not read the code of the function under test.

To do

Ready for Review.

How to test

luanti --run-unittests --test-module "[collision]"

@Zughy Zughy added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Code quality labels Nov 13, 2024
@kno10
Copy link
Contributor

kno10 commented Nov 14, 2024

#15408 adds additional collision unit tests.

@JosiahWI
Copy link
Contributor Author

@kno10 That looks great, thank you! If that lands before this one, I can rebase this over some weekend thereafter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants