Use fixed point math for position, endurance, health - #1044
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
ac1def9 to
ae32b4a
Compare
|
|
This is the latest version available from their Mercurial Repo on Sourceforge: https://sourceforge.net/p/fixedptc/code/ci/57887bd8c046c0c0394c22adc806d67bd5a71eaa/tree/
24.8 format gives us 256 subpixels, which the original game seems to like.
8d15229 to
d8c7b6d
Compare
Left shifting a negative number is undefined behavior. https://learn.microsoft.com/en-us/cpp/code-quality/c26453
) * Try to improve the fixedpt_str function and add unit tests for it * Fix overflow in fixedpt_fracpart macro * Futher clarify the function * Clarify fixed point string printing test suite name * Clearer handling of negative numbers * Apply suggestions from code review --------- Co-authored-by: Magnus Larsen <golgothasTerror101@gmail.com>
| @@ -0,0 +1,25 @@ | |||
| This software contains fixedptc, whose license follows: | |||
There was a problem hiding this comment.
Perhaps this should be combined with the LICENSE file?
| #define ARENA_RIGHT_WALL 300 | ||
| #define ARENA_FLOOR 190 | ||
|
|
||
| // fixed point versions |
There was a problem hiding this comment.
Arguably we should remove the non fixed point versions?
There was a problem hiding this comment.
ARENA_FLOOR is important for object rendering, and we render in pixels-- not fixedpt fractions of pixels.
The arena walls can be fixedpt only, sure.
| } | ||
| fixedpt rand_fixedpt(fixedpt max) { | ||
| return random_fixedpt(&rand_state, max); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Need a newline at end of this file
| printf("%d\n", af->health); | ||
| break; | ||
| case 5: | ||
| printf("%f\n", af->forward_speed); |
There was a problem hiding this comment.
Arguably we could print these as fixedpt_strs
| if(info) { // Only Power Plant has the electric overlay effect | ||
| object *obj2 = omf_calloc(1, sizeof(object)); | ||
| object_create(obj2, scene->gs, vec2i_create(o_har->pos.x, o_har->pos.y), vec2f_create(0, 0)); | ||
| object_create(obj2, scene->gs, vec2f_to_i(o_har->pos), vec2f_createf(0, 0)); |
There was a problem hiding this comment.
Seems like we should create objects in the fixed point space?
|
|
||
| static vec2f center(component *c) { | ||
| return vec2f_create(c->x + c->w / 2, c->y + c->h / 2); | ||
| return vec2f_create(fixedpt_fromint(c->x + c->w) / 2, fixedpt_fromint(c->y + c->h) / 2); |
There was a problem hiding this comment.
Looks like the math is changing here due to order of operations
|
Would it make sense//Is it possible to split this up into multiple PRs? We can start with one PR for the fixed point library at least |
|
Fixed-point health, damage, endurance is a good candidate for being split off to a separate PR; these do not use the 24.8 fixed point format that the on-screen positions use (24.8 implemented here in fixedptc.h). When reviewing a PR like this, I suggest using the |
This PR hopes to improve netplay and rec compatibility & determinism across platforms.
Positions and velocities (vec2f) are now stored as 24.8 fixed point numbers, which means there are 24 bits for the whole part and 8 for the fractional part.
This PR has the annoying quirk of taking an existing name and appending 'f' to mark it as fixedpoint, some of these should be un-renamed before merging-- this was done to ensure non-updated code would compiler error while writing the PR.