Add smooth-movement plugin - #5875
Conversation
|
A general comment as this is a pervasive issue and I do not wish to pepper the review with dozens if not hundreds of comments: there are a lot of operations in here that work on (x,y) coordinate pairs that involve effectively duplicated code. DFHack has a 2d coordinate type I've made some other specific comments but this should not be considered a full review. |
This will be a substantial refactor. I will look into it and the others comments as well. |
|
Camera feature was not working properly and introduced a mouse click bug so I opted to remove it. Edit: forgot to tag @ab9rf |
ab9rf
left a comment
ab9rf
left a comment
There was a problem hiding this comment.
note: this is not a complete review - i'll make another pass after these comments are addressed
|
|
||
| bool operator==(const visual_contextst &) const = default; | ||
| }; | ||
| visual_contextst previous_visual_context{}; |
| } | ||
| } | ||
|
|
||
| constexpr uint32_t fire_bits = 0x70000000U; |
There was a problem hiding this comment.
this is a magic number. please construct this out of published constants
| // On a pure pan the reset signature is unchanged, but last frame's blackout | ||
| // coverage is in the old viewport frame, so discard it (the engine repaints | ||
| // the whole scrolled viewport anyway). | ||
| const int32_t pan_x = window_x ? *window_x : 0; |
There was a problem hiding this comment.
shouldn't this be const coord2d pan with an appropriate initializer?
| previous_coverage.clear(); | ||
| previous_pan_x = pan_x; | ||
| previous_pan_y = pan_y; | ||
| previous_pan.x = pan_x; |
There was a problem hiding this comment.
previous_pan = pan with pan initialized as mentioned above
| float source_y; | ||
| int32_t target_x; | ||
| int32_t target_y; | ||
| point2dst<float> source; |
There was a problem hiding this comment.
toady likes the silly nonsense of putting st after structure names, but imo it detracts from readability here. consider renaming the point2d template class without toady's st suffix on types, which DFHack has no love for and is absolutely not required for DFHack-defined types
| // hypothesis-tested each frame to find where it lands. Detection stays | ||
| // suppressed until then: a shifted buffer makes every panned creature look | ||
| // like a real move. | ||
| if (state.has_pan && (state.pan.x != input.pan.x || state.pan.y != input.pan.y)) { |
| reset_facing(state); | ||
| } | ||
| state.pending.push_back({input.pan_x - state.pan_x, input.pan_y - state.pan_y}); | ||
| state.pending.emplace_back(input.pan.x - state.pan.x, input.pan.y - state.pan.y); |
There was a problem hiding this comment.
should probably be emplace_back(input.pan - state.pan) but might need to be emplace instead, in any case use the operator overloads, that's what they're for
| // changes, and while paused hardly at all. Re-reading a landed scroll steps | ||
| // every sprite by a tile. | ||
| const uint64_t signature = compute_buffer_signature(input); | ||
| const bool buffers_advanced = |
There was a problem hiding this comment.
this to me advertise that buffer_signature should be a std::optional
| // A prefix netting to zero is indistinguishable from "nothing landed yet". | ||
| // Accepting it would retire shifts the buffers have still to apply. | ||
| if (shift[0] == 0 && shift[1] == 0) | ||
| shift.x += state.pending[count - 1].x; |
There was a problem hiding this comment.
use coord2d operator overload here
| // A prefix netting to zero is indistinguishable from "nothing landed | ||
| // yet". Accepting it would retire shifts the buffers have still to | ||
| // apply. | ||
| if (shift.x == 0 && shift.y == 0) |
not necessary to tag me, i'm notified on all activity on dfhack repositories |
2 participants
Summary
Adds the
smooth-movementplugin to DFHack. It provides smooth visual interpolation for creatures, items, vehicles, and related overlays in the fortress viewport.Notes
I closed the previous PR in favor of this version, which preserves the original code structure that was already reviewed by humans in the standalone repository.
Standalone repository: https://github.com/notliad/df-smooth-movement
Testing
smooth-movementplugin through the DFHack build system.smooth-movement-test.