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.

Sign up for GitHub

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

Jump to bottom

#2275 Improve assertions for invalid transforms. #2276

Merged
merged 5 commits into from
Jun 1, 2024
Merged

#2275 Improve assertions for invalid transforms. #2276

merged 5 commits into from
Jun 1, 2024

Conversation

richardTingle
Copy link
Member

@richardTingle richardTingle commented Jun 1, 2024 •
edited
Loading

These are already caught by assertions in GLRenderer, but by that time it is unclear what call pushed in the bad values

closes #2275

@richardTingle
jMonkeyEngine#2275 Improve assertions for invalid transforms.
6864557 
These are already caught by assertions in GLRenderer, but by that time it is unclear what call pushed in the bad values
@stephengold
Copy link
Member

I'm generally supportive of these changes.

I recommend using the existing Vector3f.isValidVector() instead of defining a private method for that purpose.
Furthermore, I suggest moving isValidQuaternion() to com.jme3.math.Quaternion and making it a public method, to encourage re-use.
Finally, for isValidNumber() I think we should use Float.isFinite(), which was added in Java 1.8 .

@richardTingle
Copy link
Member Author

Sure @stephengold I've made those changes

@stephengold
Copy link
Member

I see you also added an automated test. That's great, but it needs javadoc and a copy of the JME license.

@stephengold
Copy link
Member

Thanks for your quick responses @richardTingle. Just a few more small things ...

@stephengold stephengold merged commit 2c877fe into jMonkeyEngine:master Jun 1, 2024
11 checks passed
scenemax3d pushed a commit that referenced this pull request Jun 2, 2024
@richardTingle @scenemax3d
#2275 Improve assertions for invalid transforms. (#2276)
d13df22 
* #2275 Improve assertions for invalid transforms.

These are already caught by assertions in GLRenderer, but by that time it is unclear what call pushed in the bad values

* #2275 Swap to use more generally available validation methods

* #2275 Add license and javadoc

* #2275 Correct copyright year

* #2275 It is a quaternion
@stephengold stephengold added this to the v3.7.0 milestone Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephengold stephengold stephengold approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
v3.7.0
Development

Successfully merging this pull request may close these issues.

Invalid-matrix4f-for-worldviewprojectionmatrix assertion is hard to diagose
2 participants
@richardTingle @stephengold