Catch async usage of playsound

[Owen1212055]

This causes the world random to scream due to multi-thread acces

[Machine-Maker]

Do we also need catchers for the adventure sound methods? Should be added to the adventure patch if so.

[Machine-Maker]

Looks good. This was expanded to catch any use of World#playSound off the main thread as none of the 3 methods are thread-safe. It does more specific checks for Server#playSound methods as there are some cases where the use is thread-safe.

[ghost]

In my opinion this is a bad idea, .playSound is just sending a packet, same as .spawnParticle people will use it async. If the world random is screaming use the multithreaded random, why does world random even matter when playing a sound

[ghost]

also wat

[Machine-Maker]

.playSound is just sending a packet

Very much not true on World and Server. On Player, playSound is as simple as sending a packet. But not on those other types. They either iterate over a thread-unsafe list of players in a world or use the vanilla packet broadcasting system based on location which contains thread-unsafe logic.

The "world random" aspect is just one part of this. If a sound isn't provided a seed, a seed must be generated from the world's random instance which is a thread-unsafe instance.

As to your second post, I'm not sure what you are "wat"ing? THat diff is just changing some hunk line numbers and index hashes.

EDIT: Feel free to ping me in the discord if you want to discuss this further (@machinemaker)

[ghost]

.playSound is just sending a packet

Very much not true on World and Server. On Player, playSound is as simple as sending a packet. But not on those other types. They either iterate over a thread-unsafe list of players in a world or use the vanilla packet broadcasting system based on location which contains thread-unsafe logic.

The "world random" aspect is just one part of this. If a sound isn't provided a seed, a seed must be generated from the world's random instance which is a thread-unsafe instance.

As to your second post, I'm not sure what you are "wat"ing? THat diff is just changing some hunk line numbers and index hashes.

EDIT: Feel free to ping me in the discord if you want to discuss this further (@machinemaker)

It appears the line

lighning.isSilent = isSilent

was removed, I could be mistaken though.

[Machine-Maker]

It appears the line
lighning.isSilent = isSilent
was removed, I could be mistaken though.

That's correct. That line was removed. But not as a part of this PR. That is just part of an existing patch that removes that line.

These 3 spots are the lines in that patch that changed as a result of this PR.