Skip to content

Conversation

@dpiers
Copy link

@dpiers dpiers commented Dec 1, 2025

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

Use the anim file set's model name instead of NPC_type for the modelOnly
filter check. NPC variants like "stormtrooper2" share the same model and
anim events as the base NPC, so we need to match against the model name.

Fixes: JACoders#1287
@dpiers dpiers requested a review from a team as a code owner December 1, 2025 06:29
@dpiers dpiers force-pushed the claude/review-jedi-academy-pr-01AkfJmyMvwdtCLPYtRSwKdy branch from 81718de to ea655b1 Compare December 1, 2025 22:38
@ensiform
Copy link
Member

ensiform commented Dec 2, 2025

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

@dpiers
Copy link
Author

dpiers commented Dec 2, 2025

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.

@ensiform
Copy link
Member

ensiform commented Dec 2, 2025

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.

@dpiers
Copy link
Author

dpiers commented Dec 2, 2025

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JA SP - Stormtrooper Death “Clank” Sounds Don’t Play for NPC Variants

3 participants