Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix random substitution of particles #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

illwieckz
Copy link

@illwieckz illwieckz commented Mar 14, 2023

Fix random substitution of particles.

I noticed there was no error message for MAX_PARTICLE_SYSTEMS being hit. I noticed there was an error message for MAX_TRAIL_SYSTEMS being hit.

So I wanted to add such error message to debug the random substitution of particles.

So I decided to copy-paste the MAX_TRAIL_SYSTEMS hit message and implement it for MAX_PARTICLE_SYSTEMS the cargo cult way.

So I discovered the particle code was not only not printing anything when MAX_PARTICLE_SYSTEMS was hit, but was also returning the particle system anyway, the previous one before hitting MAX_PARTICLE_SYSTEMS.

Then I discovered a similar unexpected return of reused pointer was also done when MAX_PARTICLES and MAX_PARTICLE_EJECTORS were hit.

  • Do not return previous particle if MAX_PARTICLES is hit
  • Do not return previous particle system if MAX_PARTICLE_SYSTEMS is hit
  • Do not return previous particle ejector if MAX_PARTICLE_EJECTORS is hit

This is a port of a patch from Unvanquished: https://unvanquished.net

Fix random substitution of particles.

I noticed there was no error message for MAX_PARTICLE_SYSTEMS being hit.
I noticed there was an error message for MAX_TRAIL_SYSTEMS being hit.

So I wanted to add such error message to debug the random substitution
of particles.

So I decided to copy-paste the MAX_TRAIL_SYSTEMS hit message and
implement it for MAX_PARTICLE_SYSTEMS the cargo cult way.

So I discovered the particle code was not only not printing anything
when MAX_PARTICLE_SYSTEMS was hit, but was also returning the particle
system anyway, the previous one before hitting MAX_PARTICLE_SYSTEMS.

Then I discovered a similar unexpected return of reused pointer
was also done when MAX_PARTICLES and MAX_PARTICLE_EJECTORS were hit.

- Do not return previous particle if MAX_PARTICLES is hit
- Do not return previous particle system if MAX_PARTICLE_SYSTEMS is hit
- Do not return previous particle ejector if MAX_PARTICLE_EJECTORS is hit

This is a port of a patch from Unvanquished: https://unvanquished.net
@illwieckz
Copy link
Author

This is a port of a patch from Unvanquished:

It fixes a long-standing bug: in servers with many players producing a lot of particles on screen, particles may start to be randomly substituted, like rifle projectiles being on fire, flamer throwing player disconnection bubbles, shotgun emitting jetpack smoke, bullet impact on rock displaying flames, and other unexpected combinations.

More detail on the issue is reported on Unvanquished issue tracker:

The bug is almost 20 years old, as it was introduced by @timangus in 2003 in commit 51f8195 .

commit 51f8195fe9846eaf3482da7ccce44064a7c2900a
Author: Tim Angus <***@***>
Date:   Sun Sep 21 18:23:47 2003 +0000

    * Fully generalised scriptable paricle system
    * Changes to Makefile and depend file for above
    * Tweaks to entities.def
    * Apparently a bunch of other stuff I've forgotten about

Once merged to master the fix would have to be merged to gpp as well.

@AsciiWolf
Copy link

Thanks for the fix! It could be a good idea to also submit the patch to the GrangerHub fork of Tremulous if it isn't already fixed there.

@illwieckz
Copy link
Author

Thanks for the fix! It could be a good idea to also submit the patch to the GrangerHub fork of Tremulous if it isn't already fixed there.

Good idea! Here it is:

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.

2 participants