Network: Batch individual particle packets

[appgurueu]

Alleviates / closes #16450.

Currently, each core.add_particle call sends a packet. This can be relatively wasteful and expensive.

This adds a new TOCLIENT_SPAWN_PARTICLE_BATCH command for a new protocol version >= 50. This command consists of zstd-compressed data comprising an array of particle parameters. core.add_particle calls are batched per server step and then sent to all clients; older clients are still sent one packet per particle. This should make core.add_particle much more usable than it is right now (assuming a recent client & server).

Particlespawners are an alternative solution, but likely to be unsuitable for specialized use cases as outlined in that issue. SSCSM will fix this in the longer term, but we can relatively easily optimize the cost of core.add_particle already.

To do

This PR is Ready for Review.

How to test

Spawn many particles in a single server step, e.g. by using the example code in the linked issue.

[SmallJoker]

Will test later with #16450 (comment).

[SmallJoker]

Do you need this copy? compressed_data_oss could be used directly to feed NetworkPacket with the necessary data.

[appgurueu]

What would you suggest? As far as I know there is no good C++ 17 way to get a view into the underlying buffer of a stringstream without copying. C++ 20 introduces the .view() method for that, but in C++ 17 we only have .str(). There is .rdbuf() but it is not obvious to me from the specification that this would be guaranteed to give us the entire buffer.

I also don't think this copy is very significant in the bigger picture.

[SmallJoker]

	std::ostringstream compressed_data;
	compressZstd(oss.str(), compressed_data);

	NetworkPacket pkt(TOCLIENT_SPAWN_PARTICLE_BATCH,
			4 + compressed_data.tellp(), player->getPeerId());
	pkt.putLongString(compressed_data.str());
	Send(&pkt);

EDIT: compressZstd(oss, compressed_data); could work too, if you're using an std::stringstream instead (separate in + out indexes).

[appgurueu]

compressZstd only takes strings at the moment, so I've left it at that. I think this is still doing same number of copies (first oss / particle_data is copied to get it out of the string buffer and into zstd compression, then the compressed data is copied afterwards; the assignment compressed_data = compressed_data_oss.str() should be a move assignment), but it is a bit nicer to read.

[sfan5]

compressZstd only takes strings at the moment

it takes a std::string_view.
unless you mean it doesn't take a stringstream, which is true but that would be weird.

[SmallJoker]

master: 100 packets are sent
PR: 1 packet is sent, and much less data
I would generally recommend to use particle spawners when there's plenty of particles to render, but such optimization is welcome too. Will approve after the comments are handled.

[lhofhansl]

Nit: Since you have to materialize the string below anyway, it might be a little bit more clear to materialize into a std::string, and use string::size() instead of tellp.

[appgurueu]

I had it like that before (albeit for the other branch): #16458 (comment) 🙃

[appgurueu]

I would generally recommend to use particle spawners when there's plenty of particles to render

If possible yes, but the point is flexibility. Say you want to spawn pink particles in a growing heart shape for a firework. Things like that is what the linked issue is about. Some time ago I also wrote https://github.com/appgurueu/ghosts which also requires many individual add_particle calls.

---

The next step should be to also batch particle spawners, imo, but one thing at a time.

[sfan5]

this will not work if we ever want to add new stuff at the end of ParticleParameters, will it?

[appgurueu]

Yes, but I don't think that's a problem: Particle serialization & deserialization is simply parameterized in terms of the protocol version. If we add new fields, reading/writing them will simply be behind protocol version checks.

So this should be extensible just fine. Or maybe I'm misunderstanding what you're saying?

[sfan5]

If we add new fields, reading/writing them will simply be behind protocol version checks.

I didn't check writing, but this isn't true even for reading today:

luanti/src/particles.cpp

Lines 213 to 216 in f390137

	// new texture properties were missing in ParticleParameters::serialize
	// before Minetest 5.9.0
	if (is.eof())
	return;

[appgurueu]

Okay, that is a relict of an older version, but should be irrelevant for us: When we send the batched packet, we know that the server is more recent so this doesn't happen. Likewise a recent client receiving the batched packet can't have this happen either.

Hence everything works as expected. No?

And when we add new things in the future, we just need to make sure not to use the "add things at the end" hack but rather actually checking protocol versions.

[sfan5]

I don't think appending at the end is a hack. Rather it's a simple way to do compatibility without needing an explicit version number for every revision.

Hence everything works as expected. No?

True. But are we going to add a comment like

// Note: the EOF check here is okay because it precedes the introduction of (...)
// DO NOT ever use such a check again when serializing here, or backwards compat will be broken.
// Instead the amount of data read must be 100% deterministic based on the protocol version.

IMO changing the principle in the middle of the method does not give an impression of clean code/protocol design.

---

To avoid all this I would serialize the particles individually in strings, use serializeString16Array and then compress that.

[Desour]

I'd like to note that having substructures serialized as string, with length, is nice if you want to write a tool that does deserialization but only needs part of the data.
As a negative example, if you want to parse the nodemeta of a mapblock, you have to write a parser for (the weird line based) inventories. If it had a length around it, one could just skip it.

---

Btw. shortly mentioned in last meeting: https://irc.luanti.org/luanti-dev/2025-09-07#i_6282029

[SmallJoker]

Note that the particle count does not solve this, as you still don't know where one ServerParticleTexture ends and the next begins.

Indeed. Handling compat with multiple clients can be annoying that way. We could re-use the practice of ActiveObjectCommand, such that more ClientEventType could be dispatched in the future.

I'd like to note that having substructures serialized as string, with length, is nice if you want to write a tool that does deserialization but only needs part of the data.

Extending NetworkPacket to allow nesting itself in another packet would be possible, although that would not belong into this PR.

[appgurueu]

I'd like to note that having substructures serialized as string, with length, is nice if you want to write a tool that does deserialization but only needs part of the data.

Sometimes yes, but not really applicable here - there is little reason why one would want to read only some particles.

As a negative example, if you want to parse the nodemeta of a mapblock, you have to write a parser for (the weird line based) inventories. If it had a length around it, one could just skip it.

Sure, but this example feels a lot like "I wish we had formed a cyst around a past bad serialization format decision" :)

---

I digress, but in general my preference for serialization formats is one of straightforward recursive serialization and parsing (CFGs). If there is a pressing need to be able to skip some parts, that can get some kind of wrapping. But generally I prefer to avoid that, as it adds unnecessary complexity (and is easy to mess up, e.g. by introducing enough copies that you get quadratic time complexity).

---

If there's more than 64k particles each step, then mods should consider using particlespawners instead.

I'd prefer to stick with higher limits. It costs virtually nothing, and if for whatever reasons this works for some people, why shouldn't we let them?

I'd like to avoid situations along the lines of actually it turns out someone wants to spawn 100k particles, and their hardware can handle that, and now we we need to bump and/or document the limits. (Negative example in a different context: #16426)

Sure, 65k+ long strings are probably degenerate cases for the time being. But I'd let game devs figure out themselves what a reasonable limit for contemporary hardware is.

---

Anyways, I hope 7ffd0f8 makes everyone happy. Personally I'm not a big fan, but not opposed strongly enough either.

[Desour]

Isn't the length the length of a single particle? I doubt a single serialized particle will ever be bigger than 64kiB.

Sometimes yes, but not really applicable here - there is little reason why one would want to read only some particles.

Same misconception here.
One might want to read only parts of the particle parameters.

But generally I prefer to avoid that, as it adds unnecessary complexity (and is easy to mess up, e.g. by introducing enough copies that you get quadratic time complexity).

(If your deserialization can't use string views, it's kinda hard to do any kind of hierarchical thing.)

[appgurueu]

I doubt a single serialized particle will ever be bigger than 64kiB.

Probably not, but I see no need to restrict. See #16458 (comment).

If your deserialization can't use string views, it's kinda hard to do any kind of hierarchical thing.

It actually is not. When you're parsing something context-free, a recursive descent parser that reads from a (peekable) stream is completely sufficient; no string views needed.

(The point about copies was also more about serialization, where the straightforward, naive approach is to just return a std::string or similar, and that's copied later on. Do that often enough recursively and things get slow. One proper approach is marking where in the stream you started writing and then backpatching a length, but that is also a bit nasty and means you need a fully seekable stream / string buffer for the entire thing.)

One might want to read only parts of the particle parameters.

Specifically the parts that are only at the beginning, or do we start wrapping every variable-length part of particle parameters in a string now too? Would seem like too narrow a use case to be worth supporting, especially for a network protocol.

[appgurueu]

Thanks for the review, addressed it except for two comments (see my replies above). Also tested with an older client to make sure the backward compat code is working again.

[Desour]

Idk if it's a big issue, but if you only send particles once per server-step, there can be a long delay between the spawn particle call and the user seeing the particle, if the server step is long (I have seen servers running at server steps of over 1 second). In practice this will probably mean that (1) it will more often look like there are waves of particles (if the spawn particle calls are spread over the server step for some reason) and (2) particles will be out of sync with other things that are sent immediately (e.g. play sound).

You could try to send the batch more often (e.g. after 1 ms), and it would probably result in similar gains (usually, if there are many particles, they were spawned in a somewhat tight loop). But idk if the implementation effort is worth it.

[appgurueu]

I considered that, but I think it's a non issue. We do not and should not guarantee that any API call "immediately" sends packets to clients. In other words, the only timing granularity we should guarantee is that of server steps, everything else is not viable.

particles will be out of sync with other things that are sent immediately (e.g. play sound)

This goes both ways: Now particles will be in sync with AO messages, which are already batched.
We should consider batching sounds too (though I think usually there won't be enough sounds for that to matter?).

I have seen servers running at server steps of over 1 second

At that point pretty much any chance at a smooth gameplay experience is already lost.

[sfan5]

consider serializeString32 for clarity

[appgurueu]

not a fan of the copy just to prepend 4 bytes. i think we should have a variant which just writes straight to the stream, analogeous to how deSerializeString32 reads straight from a stream. i also think that consistent with other stream I/O helpers, these new helpers which directly read from or write to streams should be called read-/writeString32. i'll refactor that later.

[sfan5]

Please just use serializeString32 in the meantime and then a cleanup PR can change it.
(also not really consistent to reject my suggestion to reuse the stringstream but then care about the string copy here)

[sfan5]

Please just use serializeString32 in the meantime and then a cleanup PR can change it.
(also not really consistent to reject my suggestion to reuse the stringstream but then care about the string copy here)

[sfan5]

tested, works

[SmallJoker]

Works. Backwards compat too.