Pin minqlx to fixed SHA, add damage-event patch

[dngrtech]

Summary

• Pin the minqlx build to a known upstream commit (fbdd915185337791d8e209dc4b686a1ee60d3721 on MinoMino/minqlx) instead of a floating master, in both setup_host.yml and rebuild_minqlx.yml.
• Add ql-assets/patches/minqlx-damage-event.patch, backporting the damage event/dispatcher (plus expanded DAMAGE_* C flag constants) from the mgaertne/minqlx fork's 11-commit diff — verified to be the only functional change in that diff.
• Document the pinned SHA and patch mechanism in docs/technical.md.

Compatibility

Checked qlsm_plugins/ (force_rate, highfps, improved_timer, nullmove, ql_netfix, spec_switch_guard) — none reference damage, is_chatting, or PlayerState, so this is a non-breaking, purely additive change for existing plugins.

Test plan

• Cloned MinoMino/minqlx at the pinned SHA, applied both ql-assets/patches/*.patch files in the same glob order the playbooks use, confirmed zero .rej files
• Ran make against the patched tree — build succeeds, bin/minqlx.x64.so produced
• Validated both edited playbooks parse as valid YAML

[github-actions]

PR Review: Pin minqlx build to fixed SHA + damage-event patch

Strengths

• Good reproducibility fix (rebuild_minqlx.yml:32, setup_host.yml:529): Pinning to a specific commit SHA eliminates non-deterministic builds caused by a floating master. The old TODO comment is cleanly removed.
• damage added to no_debug (_events.py:33): Damage events fire on every hit; excluding them from the debug log prevents log flooding, consistent with how frame and stats are handled.
• GIL handling is correct (python_dispatchers.c:296–314): PyGILState_Ensure/Release wraps the Python call properly; Py_XDECREF safely handles a NULL result from a failed PyObject_CallFunction.
• Documentation is thorough: docs/technical.md documents the patch mechanism and both patch filenames, and releases.md/version.json are updated consistently.

---

Issues

Critical (Must Fix)

ql-assets/patches/minqlx-damage-event.patch → hooks.c line ~258 (patch hunk +28)

attacker_id = attacker - g_entities;

attacker can legally be NULL in Quake 3 game code (world/environmental damage, trigger damage). Subtracting a null pointer from a non-null pointer is undefined behavior in C (C11 §6.5.6). On x86-64 the result is an enormous integer, and whether DamageDispatcher's attacker_id >= 0 guard catches it is implementation-defined. In the Python handler the attacker_id in range(0, 64) check would likely keep the raw garbage integer out of Player(), but the UB itself can cause miscompiles with aggressive optimizers.

Fix:

attacker_id = attacker ? (attacker - g_entities) : -1;

This mirrors how the existing KamikazeExplodeDispatcher pattern handles optional entities and makes the DamageDispatcher dispatch path deterministic.

---

Important (Should Fix)

SHA duplicated across two playbooks (rebuild_minqlx.yml:10, setup_host.yml:12)

minqlx_git_version is defined independently in both files. If someone bumps the SHA in one playbook and forgets the other, the two build paths silently diverge—setup_host (initial provision) and rebuild_minqlx (day-2 rebuild) will produce different binaries from the same repo. This is exactly the class of bug the SHA pin is meant to prevent.

Fix: Extract to a shared group_vars file (e.g., ansible/group_vars/all/minqlx.yml) or a dedicated vars_files include so there is one source of truth.

---

No Python-layer tests for handle_damage (python/minqlx/_handlers.py:435–447)

The new handle_damage function has two non-trivial behaviors: it guards target_id and attacker_id with range(0, 64) before constructing Player objects, and it swallows all exceptions. The existing test suite covers other handlers (e.g., handle_kamikaze_explode). A NULL attacker (raw integer, not in range) and a mid-range player id (within range) should both be exercised.

---

Minor (Nice to Have)

Trailing whitespace in patch (python_dispatchers.c hunk, line after Py_XDECREF(result);)

A trailing space is present after Py_XDECREF(result); in the dispatcher. Minor, but it will show up in future whitespace-sensitive diffs.

depth: 1 with a commit SHA (rebuild_minqlx.yml:33, setup_host.yml:530)

Ansible's git module resolves this by issuing git fetch --depth 1 origin <sha>, which requires the SHA to be advertised by the remote (i.e., reachable from a branch tip). This works on GitHub today, but is not guaranteed to work if the upstream repo is later force-pushed or the commit is no longer reachable. A tag on the pinned commit (or dropping depth: 1 when a SHA is specified) would be more robust.

---

Assessment

Ready to merge? No — with fixes.

Reasoning: The NULL-attacker pointer arithmetic in hooks.c is undefined behavior that could silently misbehave depending on compiler optimization level; it has a one-line fix. The SHA duplication is an operational risk that undermines the reproducibility goal of the PR. Both should be addressed before merge.