darklegion / tremulous

Other
111 stars 41 forks source link

Fix random substitution of particles #24

Open illwieckz opened 1 year ago

illwieckz commented 1 year ago

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.

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

illwieckz commented 1 year ago

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 https://github.com/darklegion/tremulous/commit/51f8195fe9846eaf3482da7ccce44064a7c2900a .

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 commented 1 year ago

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 commented 1 year ago

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: