OpenJK icon indicating copy to clipboard operation
OpenJK copied to clipboard

[SP] Fix animation event sounds not playing for NPC variants

Open dpiers opened this issue 3 weeks ago • 4 comments

Summary

Animation event sounds (like stormtrooper death clanks) weren't playing for NPC variants such as stormtrooper2, stofficer, etc.

Cause

Model-specific animation events are loaded from models/players/<model>/animevents.cfg and tagged with modelOnly = hstring("<model>").handle() to restrict them to that model.

At runtime, CG_PlayerAnimEvents compared this against NPC_type, but NPC variants like "stormtrooper2" use the "stormtrooper" playermodel while having a different NPC_type. So the event's modelOnly (from "stormtrooper") didn't match myModel.handle() (from "stormtrooper2"), and the sound was skipped.

Fix

Use level.knownAnimFileSets[animFileIndex].filename - this is the model name that was used when loading the animation events, so it will always match.

Fixes #1287

dpiers avatar Dec 01 '25 06:12 dpiers

As of right now we have no intention of accepting any of these AI generated PRs.

ensiform avatar Dec 02 '25 00:12 ensiform

Why?

I can understand declining the pro-active memory safety fixes but this one and #1309 are clear and well documented bug fixes with open issues.

dpiers avatar Dec 02 '25 04:12 dpiers

1309 produces incompatible saves just by virtue of your change. And a majority of alleged memory safety ones are false positives/unnecessary/incorrect fixes even if they produce code that compiles and runs, such as the msg netcode change in MP using SP as an example which Claude apparently didn't fully understand why the codebases differ and what potential issues it leads to including your change which will truncate network messages on server or client side and this adds a network incompatibilities, the existing code was safe as it is.

Others may chime in as well but the jist is that we do not feel comfortable with the drive-by mass PRs generated by AI and are leaning towards blocking all AI generated commits anyway. In this instance it gives off vibes that you may attempt to sneak malicious code in see: xz takeover situation for example. We will still probably look further if they can be accepted but it may take time.

ensiform avatar Dec 02 '25 05:12 ensiform

Sorry - I didn't mean to PR spam. I am still learning to use/work with Claude Code and thought helping improve this codebase would be good practice, and it started automatically sending PRs before I realized it.

I actually started out having Claude look at my own Jedi Academy/Outcast repos, which were among the first on GitHub in April 2013 after the original source was made available on SourceForge, and then found this project.

I was a game engine programmer on Star Wars: Kinect and feel like helping fix some ~22 year old bugs here is the least I can do to repay my debt to the SW gaming community.

FWIW: The PRs that are still opened were actively investigated, reproduced and had the fixes tested.

dpiers avatar Dec 02 '25 06:12 dpiers