Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upAdded support for sending additional data in the ParticleEffect packet #1951
Conversation
madmaxoft
reviewed
May 7, 2015
madmaxoft
reviewed
May 7, 2015
madmaxoft
reviewed
May 7, 2015
| void BroadcastParticleEffect(const AString & a_ParticleName, const Vector3f a_Src, const Vector3f a_Offset, float a_ParticleData, int a_ParticleAmount, cClientHandle * a_Exclude = nullptr); | ||
| void BroadcastParticleEffect(const AString & a_ParticleName, const Vector3f a_Src, const Vector3f a_Offset, float a_ParticleData, int a_ParticleAmount, std::array<int, 2> a_Data, cClientHandle * a_Exclude = nullptr); |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
worktycho
May 7, 2015
Member
Well, the previous api passed the coordinates as separate parameters which were just as large overall.
If you find any performance evidence though I'll change it.
worktycho
May 7, 2015
Member
Well, the previous api passed the coordinates as separate parameters which were just as large overall.
If you find any performance evidence though I'll change it.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
madmaxoft
May 7, 2015
Member
True, that. It's most likely faster to copy 3 floats than to access them later via an indirection.
madmaxoft
May 7, 2015
Member
True, that. It's most likely faster to copy 3 floats than to access them later via an indirection.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
I've fixed the style issues. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
madmaxoft
May 7, 2015
Member
Not yet, but close.
I like these changes, the cBroadcaster is a nice idea.
When you get this working, could you squash it all into a single commit?
|
Not yet, but close. I like these changes, the cBroadcaster is a nice idea. When you get this working, could you squash it all into a single commit? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Those should be fixed now. I'll squash if CI passes. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Squashed. |
added a commit
that referenced
this pull request
May 7, 2015
madmaxoft
merged commit 4888f67
into
master
May 7, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
madmaxoft
May 7, 2015
Member
You might want to link the "email address" you use for commits to your account on GitHub
|
You might want to link the "email address" you use for commits to your account on GitHub |
worktycho commentedMay 6, 2015
Also the start of my refactor to move Broadcasts out of cWorld.