#14341 broke some mob death sounds

[grorp]

Minetest version

Minetest 5.9.0-dev-bb7f57b09 (Linux)
Using Irrlicht 1.9.0mt15
Using LuaJIT 2.1.1692716794
Built by Clang 17.0
Running on Linux/6.7.5-200.fc39.x86_64 x86_64
BUILD_TYPE=Release
RUN_IN_PLACE=1
USE_CURL=1
USE_GETTEXT=1
USE_SOUND=1
STATIC_SHAREDIR="."
STATIC_LOCALEDIR="locale"

Summary

#14341 broke some mob death sounds, for example in Repixture. Luckily, many many mob mods keep dead entities around for a while to play a death animation. These mods aren't affected.

While the new behavior arguably makes sense, breaking existing games/mods is not nice. I'm not sure what can/should be done about this, but I'd like it to be known at least.

Steps to reproduce

On 5.8.0:

1. Play Repixture
2. Kill a boar
3. Hear it scream

After #14341:

1. Play Repixture
2. Kill a boar
3. It doesn't scream anymore

--- OR ----

On 5.8.0:

1. Play "Mega Zombie Scream Adventure 3D"
2. Shoot those zombies
3. Hear them scream

After #14341:

1. Play "Mega Zombie Scream Adventure 3D"
2. Shoot those zombies
3. The screams are cut off

This is a rather silly example, but the difference is very noticeable.

[sfan5]

This looks like an unexpected mismatch between what we think should happen and what modders think should happen.
In #11521 the old behaviour was recognized as a bug.

Personally I think the new one is better. Mods who want sounds to vanish with the object don't need to do the tracking themselves. For death sounds like here the fix is to simply play it at the position rather than attached.

[appgurueu]

This looks like an unexpected mismatch between what we think should happen and what modders think should happen.

I agree.

Personally I think the new one is better.

I thought so too, but the new use cases made me reconsider. Death sounds are fine as you said, but I don't think we have a good solution for the "sound (scream for example) should continue playing after entity death" use case (and even for death sounds, there are some - arguably edge case - examples like entities attached to bones where playing it at the position is technically not exactly equivalent).

I think it would be safest if we revert the fix (especially as we also traded this demi-bug for another bug in #14341), document the current behavior, and tell modders to stop the sounds themselves on entity removal if they want this behavior. @SmallJoker @sfan5 @sfence thoughts?

[sfence]

It also looks like a bug in mods/games for me.
Death sound should be played as attached to the position, not to the entity that will be removed.

We can also disable the stop sound attached to the object for ephemeral pounds.

Repixture uses ephemeral sounds ( see https://codeberg.org/Wuzzy/Repixture/src/commit/b232b6dabcd40a872c8f9e218de457ff1b4e5d89/mods/mobs/api.lua#L59 ).

[grorp]

If

1. appgurueu is right that there are valid use cases for the old behavior which can't be replaced with positional sounds

2. the old behavior can't be emulated using the new behavior

3. the new behavior can easily be emulated using the old behavior

then we have a problem.

It also looks like a bug in mods/games for me.
Death sound should be played as attached to the position, not to the entity that will be removed.

With the old behavior, the simplest and the most obvious way to play a death sound was to play it attached to the entity. It's only a bug with the new behavior. Games/mods can't be blamed for this.

We can also disable the stop sound attached to the object for ephemeral pounds.

This sounds potentially confusing.

I don't really like the side I'm representing in this discussion...

[SmallJoker]

The current implementation makes more sense. The object should only emit sounds while it exists. Use positional sounds to play post-mortem effects. Needs documentation and perhaps occasional mod adjustments. I don't think reverting is a better solution.
PS: Semi-related thought: How about fading out sounds on client-side instead of instant cut-offs?

[sfence]

I think that removing ephemeral sounds is a bug.
Doc says that ephemeral should be used for short unstoppable sounds.

It can be fixed by changing one condition, see #14424.

[grorp]

PS: Semi-related thought: How about fading out sounds on client-side instead of instant cut-offs?

If I'm doing aerial combat with an aircraft mod and one of the aircraft explodes and is removed, I want the aircraft's engine sound to stop immediately. The sound shouldn't fade out, the aircraft has exploded after all.

If I have a jukebox playing music and it's removed, it would be nice if the music would fade out instead of just being cut off.

We should add a new "Sound parameter table" field allowing games/mods to control this behavior.

[sfan5]

Doc says that ephemeral should be used for short unstoppable sounds.

This is advice on when to use ephemeral sounds, not a promise that ephemeral sounds will never be stopped IMO.

If

1. appgurueu is right that there are valid use cases for the old behavior which can't be replaced with positional sounds

2. the old behavior can't be emulated using the new behavior

3. the new behavior can easily be emulated using the old behavior

then we have a problem.

I'm not sure about 1, but from a technical perspective 2 and 3 are true:

If you want the old behavior with the old code, nothing changes.
If you want the new behavior with the old code, you keep track of sounds and stop them manually.
If you want the old behavior with the new code, you have a problem because there is no (reasonable) way to move a sound and prevent it from being deleted.
If you want the new behavior with the new code, nothing changes.

Death sounds are an easy fix on the mod side, but I don't know if anyone has the usecase of sounds that persist beyond object lifetime because they're e.g. looped.

think it would be safest if we revert the fix (especially as we also traded this demi-bug for another bug in #14341), document the current behavior, and tell modders to stop the sounds themselves on entity removal if they want this behavior.

That indeed sounds like the safest way.
We can add it to the breakages list for the future.

PS: Semi-related thought: How about fading out sounds on client-side instead of instant cut-offs?

This would be another argument for putting the lifetime of attached sounds into the hands of modders (like it was before).

[sfence]

I see here a problem because there isn't and wasn't a way to stop the sound attached to an object that comes out from the client's active area.
#8094 occurs because of it and modders had no way to fight with it.

[appgurueu]

#8094 occurs because of it and modders had no way to fight with it.

Yes, that's also true. Let's try to keep this fixed so that everyone is happy.

If we let the server manage this, we can distinguish between just "unloading" an entity and "removing" an entity, whereas the client can not distinguish these.

The server should send a sound stop packet for the former case, but it may not send such a packet for the latter case, since we want to keep backwards compability for death sounds, and since modders can deal with removing sounds on death themselves.

Ephemeral sounds are an exception: Since the server has no handle for these, we need to make the client manage them. Ephemeral sounds should not be removed even if the entity they are attached to is removed. (If modders want to be able to remove sounds on object removal, they can use non-ephemeral sounds for that).

If I understand correctly, #14342 will also make the server resend sounds when entities come back into range on a client.

If we combine all of this, it should be possible to get a proper fix for sounds attached to objects.

[sfence]

Yes, #14342 resend sound to entities which comes back into range on client and client cams into max_hear_distance.

[sfan5]

@appgurueu's proposal sounds workable.

If I understand correctly, #14342 will also make the server resend sounds when entities come back into range on a client.

How does it deal with keeping sound progress when re-sending one?
I assume not at all, which is still better than the previous behaviour.

[appgurueu]

I have implemented my proposal.

How does it deal with keeping sound progress when re-sending one? I assume not at all, which is still better than the previous behaviour.

I don't think it does or can. This would require some kind of timekeeping. Related: #14431.

[sfence]

@appgurueu's proposal sounds workable.

If I understand correctly, #14342 will also make the server resend sounds when entities come back into range on a client.

How does it deal with keeping sound progress when re-sending one? I assume not at all, which is still better than the previous behaviour.

In #14342 is added field 'start_time' to struct ServerPlayingSound. This field is used to calculate correct sound start point in time of sound sending to client.

[sfan5]

Does that work for looped sounds? The server doesn't know the length.

[sfence]

Does that work for looped sounds? The server doesn't know the length.

Yes, it works also for looped sounds. I tested it.

See:

minetest/doc/lua_api.md

Lines 1091 to 1098 in e734b3f

	start_time = 0.0,
	-- Start with a time-offset into the sound.
	-- The behavior is as if the sound was already playing for this many seconds.
	-- Negative values are relative to the sound's length, so the sound reaches
	-- its end in `-start_time` seconds.
	-- It is unspecified what happens if `loop` is false and `start_time` is
	-- smaller than minus the sound's length.
	-- Available since feature `sound_params_start_time`.