Skip to content

more cleanup in preparation for model translation#4443

Merged
wookieejedi merged 2 commits into
scp-fs2open:masterfrom
Goober5000:cleanup_and_stepped_rotation_polishing
Aug 11, 2022
Merged

more cleanup in preparation for model translation#4443
wookieejedi merged 2 commits into
scp-fs2open:masterfrom
Goober5000:cleanup_and_stepped_rotation_polishing

Conversation

@Goober5000

Copy link
Copy Markdown
Contributor
  1. Clean up stepped rotation, fixing some logic, adding safeguards for invalid values, and allowing rotations to go in the opposite direction
  2. Add more specific SEXP error checking for AWACS and rotating subsystems
  3. Assorted refactoring and polishing

@Goober5000 Goober5000 added cleanup A modification or rewrite of code to make it more understandable or easier to maintain. Waiting for Stable Marks a pull request that is to be merged after the next stable release, due to a release cycle labels Jun 26, 2022
@Goober5000 Goober5000 force-pushed the cleanup_and_stepped_rotation_polishing branch from ccf8652 to ee5929d Compare June 26, 2022 00:04

@BMagnu BMagnu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly good cleanup, but a few minor things

Comment thread code/model/modelread.cpp Outdated
Comment thread code/model/modelread.cpp Outdated
Comment thread code/parse/sexp.cpp
1) Clean up stepped rotation, fixing some logic, adding safeguards for invalid values, and allowing rotations to go in the opposite direction
2) Add more specific SEXP error checking for AWACS and rotating subsystems
3) Assorted refactoring and polishing
@Goober5000 Goober5000 force-pushed the cleanup_and_stepped_rotation_polishing branch from ee5929d to 6de16d2 Compare July 9, 2022 02:05
@Goober5000 Goober5000 requested a review from asarium as a code owner July 9, 2022 02:05
@Goober5000

Copy link
Copy Markdown
Contributor Author

Ok, changes addressed.

@Goober5000 Goober5000 requested a review from BMagnu July 9, 2022 02:05
Comment thread code/math/floating.h Outdated
@Goober5000 Goober5000 force-pushed the cleanup_and_stepped_rotation_polishing branch from 6de16d2 to baadf5d Compare July 9, 2022 17:13

@BMagnu BMagnu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now

@JohnAFernandez JohnAFernandez removed the Waiting for Stable Marks a pull request that is to be merged after the next stable release, due to a release cycle label Aug 9, 2022
@wookieejedi wookieejedi merged commit dfeb15f into scp-fs2open:master Aug 11, 2022
@Goober5000 Goober5000 deleted the cleanup_and_stepped_rotation_polishing branch August 11, 2022 20:44
Goober5000 added a commit to Goober5000/fs2open.github.com that referenced this pull request Aug 19, 2022
Floating-point comparisons are tricky beasts.  See this article for a good summary of the issues involved:
https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

This PR improves support somewhat, although there are obviously situations in the code (such as comparisons to 0) that should still be addressed.  This PR makes the following improvements:
1) Tightens the threshold on `IS_VEC_NULL` and `IS_VEC_NULL_SQ_SAFE` from 1e-16 and 1e-36 to FLT_EPSILON
2) Tightens the threshold for parsing `turn_time` in POFs
3) Restores the previous implementation of `fl_equal`
4) Adds a dedicated `IS_MOI_VEC_NULL` for moment of inertia values
5) Improves documentation

Follow-up to scp-fs2open#4535 and scp-fs2open#4443.
Goober5000 added a commit to Goober5000/fs2open.github.com that referenced this pull request Aug 20, 2022
As part of the cleanup in preparation for model translation, there was a bit of code review that was actually incorrect.  The calculation of the current step needs to be rounded down, because the step isn't actually complete until it fully reaches the next number.  Rounding the step causes the submodel to jump around in all sorts of weird ways.

Follow-up to scp-fs2open#4443.
@Goober5000 Goober5000 mentioned this pull request Aug 20, 2022
Goober5000 added a commit to Goober5000/fs2open.github.com that referenced this pull request Aug 20, 2022
As part of the cleanup in preparation for model translation, there was a bit of code review that was actually incorrect.  The calculation of the current step needs to be rounded down, because the step isn't actually complete until it fully reaches the next number.  Rounding the step causes the submodel to jump around in all sorts of weird ways.

Follow-up to scp-fs2open#4443.
Goober5000 added a commit to Goober5000/fs2open.github.com that referenced this pull request Aug 26, 2022
Floating-point comparisons are tricky beasts.  See this article for a good summary of the issues involved:
https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

This PR improves support somewhat, although there are obviously situations in the code (such as comparisons to 0) that should still be addressed.  This PR makes the following improvements:
1) Tightens the threshold on `IS_VEC_NULL` and `IS_VEC_NULL_SQ_SAFE` from 1e-16 and 1e-36 to FLT_EPSILON
2) Tightens the threshold for parsing `turn_time` in POFs
3) Restores the previous implementation of `fl_equal`
4) Adds a dedicated `IS_MOI_VEC_NULL` for moment of inertia values
5) Improves documentation

Follow-up to scp-fs2open#4535 and scp-fs2open#4443.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup A modification or rewrite of code to make it more understandable or easier to maintain.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants