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

send ParticleSpawners to all players when time = 0 #13774

Merged
merged 1 commit into from Sep 2, 2023

Conversation

chmodsayshello
Copy link
Contributor

@chmodsayshello chmodsayshello commented Sep 2, 2023

Currently, ParticleSpawners are not sent to all players when they are only short lived, which is implemented checking if time is under a hardcoded value (1). However, when time is euqal to 0, that condition is also true, even though it means that the spawner exists forever (lua api).

This PR fixes that bug by making sure time is not 0.

How to test

  • Modify a client to log the addition of ParticleSpawners
  • Make sure they will recieve ParticleSpawners with time=0 even from far away

@grorp grorp added @ Network Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines Bugfix 🐛 PRs that fix a bug labels Sep 2, 2023
Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works. I have left a comment on the code.

Also, according to our Git Guidelines, commit messages must start with a capital letter:

Commit messages must start with a capital letter and must be in the present tense.

src/server.cpp Outdated Show resolved Hide resolved
@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Sep 2, 2023
@chmodsayshello
Copy link
Contributor Author

chmodsayshello commented Sep 2, 2023

should I perform a rebase to squash the two commits? I have to, in order to change the commit message

@grorp grorp removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Sep 2, 2023
@grorp grorp merged commit 294ad98 into minetest:master Sep 2, 2023
13 checks passed
@chmodsayshello chmodsayshello deleted the pspawner_bugfix branch September 2, 2023 20:59
kawogi pushed a commit to kawogi/minetest that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Network Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants