This repository was archived by the owner on Sep 20, 2026. It is now read-only.
fix(combat-insights): attribute F Minor tempo - #221
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes missing Combat Impact attribution for F Minor tempo gains in the post-combat Recap by reconstructing tempo events when the native replay data folds the +Tempo into the triggering item’s same-frame tempo change (i.e., no standalone PlayerTempoApply for F Minor).
Changes:
- Extend
CombatImpactEntitysnapshots to include socket position and native hidden tags needed for deterministic attribution. - Add projector logic to reconstruct F Minor “TempoApplyAmount” events from authoritative
CardStats.UseCountfor Haste items occupying the F Note socket. - Add a regression test ensuring only qualifying items contribute and the projected events are marked as ReconstructedTransition.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/PostCombatImpact.Tests/CombatImpactProjectorTests.cs | Adds a regression test covering F Minor tempo reconstruction from Haste item uses on the F Note socket. |
| src/BazaarPlusPlus/Game/PostCombatImpact/Data/CombatImpactProjector.cs | Implements F Minor tempo reconstruction using template IDs, socket overlap, hidden tag checks, and UseCount. |
| src/BazaarPlusPlus/Game/PostCombatImpact/Data/CombatImpactModels.cs | Extends CombatImpactEntity with SocketId and HiddenTags to support attribution rules. |
| src/BazaarPlusPlus/Game/PostCombatImpact/Data/CombatImpactEntitySnapshotReader.cs | Populates SocketId/HiddenTags in entity snapshots from native card state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+214
to
+228
| foreach ( | ||
| var item in entities.Values.Where(entity => | ||
| entity.TypeLabel == "Item" | ||
| && entity.CombatantId == combatantId | ||
| && entity.SocketId.HasValue | ||
| && entity.HiddenTags?.Contains(EHiddenTag.Haste) == true | ||
| && OccupiesAnySocket(entity, noteSockets) | ||
| && useCounts.TryGetValue(entity.Id, out var uses) | ||
| && uses > 0 | ||
| ) | ||
| ) | ||
| { | ||
| var amount = skill.Attributes![ECardAttributeType.Custom_0]; | ||
| var uses = useCounts[item.Id]; | ||
| for (var index = 0; index < uses; index++) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
战斗结算 Recap 中，F Minor（F 小调）会显示“本场战斗未造成效果”，即使放在 F Note 上的 Haste 物品实际触发了它。
原始 replay 没有为 F Minor 单独发出
PlayerTempoApply：额外+2 Tempo被折叠进触发物品的同帧节奏变化。本次复现中 Turner 的基础节奏获取量为 4，但部分帧净增量为 6；F Minor 自身既没有 combat event，也没有CardStats，因此需要从稳定模板 ID、槽位、Haste 标签及物品UseCount确定性重建来源。根因与修正
第一次实现只读取运行时
Card.HiddenTags。合成投影测试直接注入了 Haste 标签，因此通过了，但旧 replay 重建的卡牌不会必然把模板隐藏标签复制到运行时集合，导致真实 Recap 在进入归因规则前就丢失 Haste 条件。本次修正：
UseCount；验证情况
./run.sh format-checkdotnet test tests/PostCombatImpact.Tests/PostCombatImpact.Tests.csproj ...：151/151 通过./run.sh test：全量测试通过；PTR compatibility 因本地无decompiled-vptr/按既有逻辑跳过dotnet build src/BazaarPlusPlus/BazaarPlusPlus.csproj -t:BuildAll ...：Debug + Release 成功，0 warning / 0 error4f189f64ed294b0188d0c0a8b5e1a663，进入真实 Recap 并悬停 F Minor，Combat Impact 显示Tempo Gain +14；日志记录该技能 tooltip 为shown，不再是shown_without_attributed_impact已知边界