Skip to content

[TF2] Fix engineer building damage effects and placement previews not updating correctly #1280

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

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

Conversation

bakugo
Copy link

@bakugo bakugo commented May 14, 2025

This fixes two really old, annoying visual bugs with Engineer buildings:

  • Damage effects sometimes not being properly updated when a building is damaged/repaired, resulting in fully repaired buildings still emitting smoke/sparks/fire and vice-versa (ValveSoftware/Source-1-Games#4015)
  • Placement previews sometimes displaying as valid if the player begins placing a building while aiming at an invalid position (only properly updating to invalid if moved to a valid position and back to an invalid one)

And possibly more minor bugs caused by the same oversight that I'm not aware of.

The problem was an incorrect use of PreDataUpdate to keep track of old netprop values, that would then be compared to the current values in OnDataChanged to detect changes. PreDataUpdate is called once per packet, which means it can be called multiple times per render frame depending on network conditions (more likely with higher ping/jitter), while OnDataChanged is only called once per render frame. If PreDataUpdate was called twice or more before OnDataChanged, the old values would be updated to the new ones before being checked, and thus the changes would not be detected.

The solution is to simply use OnPreDataChanged instead of PreDataUpdate, which is the correct function to pair with OnDataChanged since it's also only called once per render frame.

`PreDataUpdate()` can be called multiple times per `OnDataChanged()` call, so use `OnPreDataChanged()` instead.
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.

1 participant