Fix two latent Ground Zero bugs: random-respawn dmflag filters and tesla retargeting - #151
Merged
Merged
Conversation
FindSubstituteItem chooses a replacement item for randomrespawn. Its DF_NO_SPHERES, DF_NO_NUKES and DF_NO_MINES filters test ent->classname, the item being replaced, inside the loop that is choosing the item to replace it with. The condition does not depend on the loop, so the effect is the inverse of the comment above the sphere filter: a disabled sphere respawns as itself, while every other item can still be substituted into one. SpawnItem's own DF_NO_SPHERES guard then frees that substitute, and DoRandomRespawn returns the freed edict to DoRespawn, which relinks it as a solid trigger. The third sphere classname is misspelled "item_spehre_defender" besides, so of the three the Defender is the one DF_NO_SPHERES never recognised. Both come from the original Ground Zero source. The 2023 remaster tests the candidate item in these three filters and spells the Defender correctly, which is what this commit does as well. The sphere filter only existed in the counting pass. Once the filters depend on the candidate, the counting pass and the picking pass have to apply the same ones or they disagree on how many items are eligible and the pick walks off the count, so both now share a single predicate. It identifies spheres by the pickup handler, the same test SpawnItem already uses for DF_NO_SPHERES, rather than by three more classname literals.
SV_movestep retargets a blocked monster onto the tesla whose bad area blocks it. The original had four cases: no valid enemy, retarget; the enemy is already a tesla, do nothing; the enemy is a player, retarget only when it cannot be seen; any other enemy, retarget. The second was guarded by strcmp(ent->enemy->classname, "telsa"), a misspelling, so it never ran and its case fell through to the last one and retargeted after all. Collapsing the chain into a single condition kept the behaviour of the misspelling rather than of the case it was written for: a tesla is not a client, so !ent->enemy->client is true for a monster whose enemy is a tesla and it is retargeted on every blocked frame. TargetTesla skips the switch when the tesla is the one the monster is already angry at, but its AI_MEDIC bail runs before that test, so a medic repeatedly calls cleanupHealTarget on a tesla; and when the blocking area belongs to a different tesla the monster switches to that one and overwrites oldenemy with a tesla, losing the player it was chasing. The 2023 remaster keeps all four cases and spells the classname correctly, so restore the exclusion.
Member
|
Thank you :) |
0lvin
added a commit
to yquake2/yquake2remaster
that referenced
this pull request
Sep 6, 2026
Based on: * yquake2/rogue#151 Co-authored-by: Niehztog <niehztog@gmail.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Two latent bugs from the original Ground Zero source, both in code that
silently does the opposite of what it says. Neither shows up as a compiler
diagnostic, and the tree builds clean with and without the changes.
1.
FindSubstituteItemtests the wrong item (src/g_newdm.c)The
DF_NO_SPHERES,DF_NO_NUKESandDF_NO_MINESfilters testent->classname— the item being replaced — inside the loop that is choosingthe item to replace it with. The condition does not depend on the loop, so it
fires for every candidate or for none, and the effect is the inverse of the
comment above the sphere filter: a disabled sphere respawns as itself, while
every other item can still be substituted into one.
SpawnItem's ownDF_NO_SPHERESguard then frees that substitute, andDoRandomRespawnreturnsthe freed edict to
DoRespawn, which relinks it as a solid trigger.The third sphere classname is also misspelled
"item_spehre_defender", so ofthe three the Defender is the one
DF_NO_SPHERESnever recognised.The 2023 remaster tests the candidate item in all three filters and spells the
Defender correctly.
The sphere filter only existed in the counting pass. Once the filters depend on
the candidate, both passes have to apply the same ones or they disagree on how
many items are eligible and the pick walks off the count — so both now share one
predicate, which identifies spheres by
Pickup_Sphere, the same testSpawnItemalready uses for this dmflag.2. A monster already angry at a tesla is retargeted anyway (
src/monster/misc/move.c)SV_movesteporiginally had four cases: no valid enemy, retarget; the enemy isalready a tesla, do nothing; the enemy is a player, retarget only when it cannot
be seen; any other enemy, retarget. The second was guarded by
strcmp(ent->enemy->classname, "telsa")— a misspelling — so it never ran andfell through to the last case, retargeting after all.
Collapsing the chain into a single condition preserved the behaviour of the
misspelling rather than of the case it was written for: a tesla is not a client,
so
!ent->enemy->clientis true for a monster whose enemy is a tesla and it isretargeted on every blocked frame.
TargetTeslaskips the switch when it is the same tesla, but itsAI_MEDICbail runs before that test, so a medic repeatedly calls
cleanupHealTargetona tesla; and when the blocking area belongs to a different tesla the monster
switches to that one and overwrites
oldenemywith a tesla, losing the playerit was chasing.
The remaster keeps all four cases and spells the classname correctly, so this
restores the exclusion rather than reintroducing the typo.
Testing
makeis clean with no new warnings, before and after, on gcc 13 (Linuxaarch64). The two changes are independent and in separate commits if you would
rather take only one.