-
Notifications
You must be signed in to change notification settings - Fork 14
feat(ui): bugix item compositor #31
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
Changes from all commits
1c9feaa
365f56d
88e1929
8d17fb5
21ee9f3
9b047a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,42 +25,113 @@ int ItemAnimation::getPhaseFromFrames(int frames, int64_t global_ms, | |||||||||||||
| return tick % frames; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| int ItemAnimation::getPingPongPhase(int raw_phase, int frames) { | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 Improved Animation Support The addition of ping-pong animation support and |
||||||||||||||
| if (frames <= 1) return 0; | ||||||||||||||
| int cycle = frames * 2 - 2; | ||||||||||||||
| int mod = raw_phase % cycle; | ||||||||||||||
| return mod >= frames ? cycle - mod : mod; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| int ItemAnimation::getPhase(const Domain::ItemType &item, int64_t global_ms, | ||||||||||||||
| int tile_x, int tile_y, int tile_z) { | ||||||||||||||
| int frames = std::max<int>(item.frames, 1); | ||||||||||||||
| if (frames <= 1) | ||||||||||||||
| return 0; | ||||||||||||||
|
|
||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 Restricted async animation check. bool is_async = !item.frame_durations.empty() && item.animation_mode == 0;This check requires duration data for an item to be considered asynchronous. However, some items without explicit duration data (defaulting to 500ms) might still be intended to run asynchronously if |
||||||||||||||
| bool per_instance = !item.frame_durations.empty() && item.animation_mode == 0; | ||||||||||||||
| int tile_offset = per_instance ? (tile_x * 17 + tile_y * 31 + tile_z * 7) : 0; | ||||||||||||||
| bool is_async = !item.frame_durations.empty() && item.animation_mode == 0; | ||||||||||||||
| int tile_offset = is_async ? (tile_x * 17 + tile_y * 31 + tile_z * 7) : 0; | ||||||||||||||
|
|
||||||||||||||
| int start_phase = 0; | ||||||||||||||
| if (item.start_frame == 255 && is_async) { | ||||||||||||||
| start_phase = (tile_x * 17 + tile_y * 31 + tile_z * 7) % frames; | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Animation Synchronization: The Consider allowing |
||||||||||||||
| } else if (item.start_frame > 0) { | ||||||||||||||
| start_phase = item.start_frame % frames; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if (item.loop_count == -1) { | ||||||||||||||
| // Ping-pong: apply start_phase to raw phase BEFORE reflection | ||||||||||||||
| if (item.frame_durations.empty()) { | ||||||||||||||
| int tick = static_cast<int>(global_ms / 500); | ||||||||||||||
| return getPingPongPhase(tick + tile_offset + start_phase, frames); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Build explicit ping-pong timeline from per-phase durations | ||||||||||||||
| const int num_phases = static_cast<int>(item.frame_durations.size()); | ||||||||||||||
| if (num_phases == 0) return start_phase; | ||||||||||||||
|
|
||||||||||||||
| // Per-phase durations (stack allocation — clamp to prevent overflow) | ||||||||||||||
| constexpr int MAX_PHASES = 32; | ||||||||||||||
| const int effective_phases = std::min<int>(num_phases, MAX_PHASES); | ||||||||||||||
| int phase_durs[MAX_PHASES]; | ||||||||||||||
| int cycle_length = 0; | ||||||||||||||
| for (int i = 0; i < effective_phases; ++i) { | ||||||||||||||
| phase_durs[i] = getPhaseDuration(item, i); | ||||||||||||||
| cycle_length += phase_durs[i]; | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Potential Buffer Overflow: If Additionally, the second loop does not check if
Suggested change
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Ping-pong timeline: [0, 1, ..., N-1, N-2, ..., 1] (omit duplicate endpoints) | ||||||||||||||
| constexpr int MAX_TIMELINE = MAX_PHASES * 2 - 2; | ||||||||||||||
| int timeline[MAX_TIMELINE]; | ||||||||||||||
| int timeline_len = 0; | ||||||||||||||
| for (int i = 0; i < effective_phases; ++i) | ||||||||||||||
| timeline[timeline_len++] = i; | ||||||||||||||
| for (int i = effective_phases - 2; i >= 1; --i) | ||||||||||||||
| timeline[timeline_len++] = i; | ||||||||||||||
|
|
||||||||||||||
| // Backward portion cycle length | ||||||||||||||
| int backward_length = 0; | ||||||||||||||
| for (int i = effective_phases - 2; i >= 1; --i) | ||||||||||||||
| backward_length += phase_durs[i]; | ||||||||||||||
| cycle_length += backward_length; | ||||||||||||||
|
|
||||||||||||||
| if (cycle_length <= 0) return start_phase; | ||||||||||||||
|
|
||||||||||||||
| // Start offset: sum durations of first start_phase phases | ||||||||||||||
| int start_offset = 0; | ||||||||||||||
| for (int i = 0; i < start_phase && i < effective_phases; ++i) | ||||||||||||||
| start_offset += phase_durs[i]; | ||||||||||||||
|
|
||||||||||||||
| int elapsed = static_cast<int>( | ||||||||||||||
| (global_ms + static_cast<int64_t>(tile_offset) * 500 + start_offset) % cycle_length); | ||||||||||||||
|
|
||||||||||||||
| // Walk the timeline to find the active phase | ||||||||||||||
| for (int i = 0; i < timeline_len; ++i) { | ||||||||||||||
| int dur = phase_durs[timeline[i]]; | ||||||||||||||
| if (elapsed < dur) | ||||||||||||||
| return timeline[i] % frames; | ||||||||||||||
| elapsed -= dur; | ||||||||||||||
| } | ||||||||||||||
| return 0; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Forward loop | ||||||||||||||
| if (item.frame_durations.empty()) { | ||||||||||||||
| int tick = static_cast<int>(global_ms / 500); | ||||||||||||||
| return (tick + tile_offset) % frames; | ||||||||||||||
| return (tick + tile_offset + start_phase) % frames; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| int total = static_cast<int>(item.total_duration); | ||||||||||||||
| if (total <= 0) | ||||||||||||||
| return 0; | ||||||||||||||
| return start_phase; | ||||||||||||||
|
|
||||||||||||||
| int64_t elapsed_ms = global_ms + static_cast<int64_t>(tile_offset) * 500; | ||||||||||||||
| int elapsed = static_cast<int>(elapsed_ms % total); | ||||||||||||||
|
|
||||||||||||||
| for (int phase = 0; phase < static_cast<int>(item.frame_durations.size()); ++phase) { | ||||||||||||||
| int dur = getPhaseDuration(item, phase); | ||||||||||||||
| if (elapsed < dur) | ||||||||||||||
| return phase % frames; | ||||||||||||||
| return (phase + start_phase) % frames; | ||||||||||||||
| elapsed -= dur; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return 0; | ||||||||||||||
| return start_phase; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| int ItemAnimation::getPhaseDuration(const Domain::ItemType &item, int phase) { | ||||||||||||||
| if (phase < 0 || phase >= static_cast<int>(item.frame_durations.size())) | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 Type Safety: The return value of |
||||||||||||||
| return 500; | ||||||||||||||
| const auto &d = item.frame_durations[phase]; | ||||||||||||||
| return (d.first + d.second) / 2; | ||||||||||||||
| return static_cast<int>((d.first + d.second) / 2); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| } // namespace Rendering | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Unused property: The
is_lying_objectproperty is merged intoItemTypefrom DAT, but it is not utilized in theItemRendereror any other part of the rendering pipeline.The PR summary mentions: "Multi-tile “lying” items now render beneath creatures for correct depth.", but without logic checking this flag in
ItemRenderer::queueAll(e.g., to move these items to Pass 1), they will continue to render in Pass 2 (Common items), which is already beneath creatures.If the intention was to specifically handle multi-tile corpses that might overlap creatures on adjacent tiles due to isometric rendering order, further logic in
ItemRendererwould likely be needed.