Skip to content

bug: a second hit on the same target in one tic overwrites the first's own mobj fields #504

Description

@MarcusKainth

Where

Decode or execute in SQL (sqlcpu)

Commit

65ddfca

OS, architecture and ClickHouse version

macOS aarch64, ClickHouse 26.8.2.7

What happened

Native mode (native/src/sql/sim/inter.rs's damage_fold/damaged), not
emulation mode's own sqlcpu, but the form has no separate native option.

damage_fold's own per-call answer for the mobj-generic fields a hit
moves (hurt::HEALTH, hurt::FLAGS, hurt::STATE, hurt::TICS,
hurt::MOMX/MOMY/MOMZ, hurt::HEIGHT, hurt::REACTIONTIME,
hurt::TARGET, hurt::THRESHOLD) reads the tic-start arrays
(Hurting's own m_health, m_flags and the rest) directly rather than
an accumulator. A second hit on a target this tic has already hit once -
whether inside one damage_fold call's own ask list or a later call
chained after it through start - overwrites the first hit's own answer
for these fields instead of building on it: the second call's own
dm_health still reads the mobj array's tic-start value, not what the
first call already left it at.

A recent PR (native/player-damage) added the same threading for the
player's own fields (health, armour, damagecount, attacker) precisely to
avoid this for the player, and its own two-hit test found the shared
m_health field for the same case still wrong even though the player's
own p_health correctly reflects both hits. DM_SAME_TARGET (a new
sim::unresolved bit, same PR) refuses a tic that reaches this rather
than committing the wrong shared field, so the gap is caught rather than
silent, but the fields themselves still do not thread.

The fix is the same shape as the player one: carry the mobj-generic
fields (or enough of them - HEALTH, FLAGS, STATE, TICS are the
ones another hit in the same tic can plausibly still be reading) through
the same start/chain mechanism damage_fold already carries the
player's own fields and the hit-targets set through, keyed by which
target a hit landed on, so a later hit on the same target reads what an
earlier one left rather than the tic's own starting row.

This also matters for mt_hurt_asks (the claw's own ask list,
mobj.rs), which today never carries more than one ask because
unresolved::AT_ATTACKERS already refuses more than one melee attacker
in a tic - a reshape of that path to carry several asks from several
attackers in one list (tracked separately) would hit the same gap for
two melee attackers landing on the same target.

What you expected instead

A second hit on a target already hit this tic builds on the first hit's
own answer for the mobj-generic fields, the way the player's own fields
already do, so the tic resolves instead of refusing with
DM_SAME_TARGET.

Reproduction

cargo test -p clickdoom-native --features clickhouse-tests \
  --test sim_player_damage_live two_fireballs_in_one_tic_thread_the_player_through_both \
  -- --test-threads 1

The test's own assertions confirm the gap rather than hide it: it checks
unresolved & sim::unresolved::DM_SAME_TARGET != 0 and that the shared
m_health field disagrees with the player's own p_health, which is
exactly what closing this issue should flip to agreeing (and the
DM_SAME_TARGET bit no longer firing for this arm).

Output

native/src/sql/sim/inter.rs, damaged():
    value(
        "dm_same_target",
        format!("toUInt8(has({player}.{}, dm_target))", hurt::HIT_TARGETS),
    );
    value(
        "dm_stuck",
        "toUInt8(dm_lands = 1 AND (dm_player_dies = 1 OR dm_sector11 = 1 \
         OR dm_same_target = 1 \
         OR (dm_routine != 0 AND dm_routine != a_pain AND dm_routine != a_scream)))"
            .to_owned(),
    );

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: nativeNative mode: the tic simulation and renderer as SQL, and the WAD loaderbugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions