MumbleLink icon indicating copy to clipboard operation
MumbleLink copied to clipboard

Dimensional Height Offset

Open Robijnvogel opened this issue 4 years ago • 23 comments

Added distribution of different dimensions over positional audio height range.

  • This means that players that are in different dimensions should not hear each other if they are in a Vanilla dimension and otherwise are rather unlikely to hear each other.
  • DIM0 (Overworld) remains on offset 0 so positional audio for the Overworld does not change.
  • All players must use the same version of the mod for positional audio to work at all in non-Overworld dimensions.
  • Changed version number accordingly.

Robijnvogel avatar Feb 05 '21 20:02 Robijnvogel

We should aim for unison. This is how @magneticflux- did it on the fabric mod.

https://github.com/magneticflux-/fabric-mumblelink-mod/blob/b6ffa961f070ae17241ecb9f5d2a1a7fb91c9cdd/src/main/kotlin/com/skaggsm/mumblelinkmod/ClientMumbleLinkMod.kt#L74-L76

https://github.com/magneticflux-/fabric-mumblelink-mod/blob/b6ffa961f070ae17241ecb9f5d2a1a7fb91c9cdd/src/main/kotlin/com/skaggsm/mumblelinkmod/config/MumbleLinkConfig.kt#L17-L18

To me it looks like it stays at 0 or can be configured. But I don't know who/when the config would get changed.

zsawyer avatar Feb 26 '21 00:02 zsawyer

The way I did it is definitely super hacky, and could probably be done better. Some issues with it that I see currently:

  • It's purely client-side, which causes a lot of issues with configuration
  • Using hashCode could change on different versions since it uses the Object::hashCode implementation.
    • Also, the hashCode contract doesn't support this use case anyway: "This integer need not remain consistent from one execution of an application to another execution of the same application.

Some things to keep in mind with a new implementation:

  • It should be consistent over versions, mods, JVMs, anything that could differ between players on one server
  • It should support modded dimensions
  • It should be configured on the server-side

magneticflux- avatar Feb 26 '21 02:02 magneticflux-

It should be consistent over versions, mods, JVMs, anything that could differ between players on one server

We had a look at the 1.15 MC API and it looks like we should use constants for -1, 0, 1 (vanilla Minecraft dimensions). Other than that starting with 1.16.2 dimensions became a lot more complex:

I received some hints from the forge discord:

World#getDimensionKey Compare against the constants in World - OVERWORLD, NETHER and END

zsawyer avatar Feb 28 '21 01:02 zsawyer

It should support modded dimensions

@Robijnvogel figured that if we would get a number as an identifier we could use some custom hash function distribution for all other dimension and would distribute them across the height axis.

https://github.com/zsawyer/MumbleLink/pull/35/files#diff-f8536ccf08cf3ea815f4660b932f9ca9bb0bc5a2dc7037042d2b99329d38598aR261

zsawyer avatar Feb 28 '21 01:02 zsawyer

It should be configured on the server-side

I would love to avoid this dependency on this kind of integration. It is more robust and easier to maintain if it is only a client. Could you elaborate on the benefit?

zsawyer avatar Feb 28 '21 01:02 zsawyer

It should be consistent over versions, mods, JVMs, anything that could differ between players on one server

We had a look at the 1.15 MC API and it looks like we should use constants for -1, 0, 1 (vanilla Minecraft dimensions). Other than that starting with 1.16.2 dimensions became a lot more complex:

I received some hints from the forge discord: World#getDimensionKey Compare against the constants in World - OVERWORLD, NETHER and END

Yeah, those constants have been around for ages but they've been replaced with registry keys now. We can switch on those keys and return an integer like in the past, but that doesn't support modded dimensions as easily.

It should support modded dimensions

@Robijnvogel figured that if we would get a number as an identifier we could use some custom hash function distribution for all other dimension and would distribute them across the height axis.

https://github.com/zsawyer/MumbleLink/pull/35/files#diff-f8536ccf08cf3ea815f4660b932f9ca9bb0bc5a2dc7037042d2b99329d38598aR261

A custom hash function would be a possibility as long as it's consistent across different mod loading setups and JVMs and doesn't have collisions for common values.

Another possibility I was considering was creating an arbitrary (and unique) mapping from world registry keys to integers and syncing that to all clients. That would require server integration though.

It should be configured on the server-side

I would love to avoid this dependency on this kind of integration. It is more robust and easier to maintain if it is only a client. Could you elaborate on the benefit?

The benefit I see is it alleviates responsibility from myself and server admins to help users troubleshoot configuration mismatches. I've handled several issues, both on GitHub and over Discord, where clients had mismatched dimensional offset values.

The other configuration technique in my mod (sending a packet to the client with server info) is server-side, and I haven't had any issues with users of the client having issues connecting, just with server admins not understanding string templating.

magneticflux- avatar Feb 28 '21 01:02 magneticflux-

Maybe it would be easier if we would provide better server tools: Give them a mumo module similar to bf2 to use.

All that is need then is provide the data in identity in a uniform format.

zsawyer avatar Feb 28 '21 01:02 zsawyer

Maybe it would be easier if we would provide better server tools: Give them a mumo module similar to bf2 to use.

All that is need then is provide the data in identity in a uniform format.

The base version of the module would provide only basic dimension based moving (read dimension from identity -> move to channel).

However it would get a little tricky to support custom aspects like teams/guilds/factions/squads/parties or what ever I cannot think of right now:

  1. client needs to send appropriate information about new aspect for the Forge mod this could be done by creating addon-mods which implement https://github.com/zsawyer/MumbleLink/blob/7326a2ddfebf328a9a1d563128b9a11ef8a642e7/mod/src/main/java/zsawyer/mods/mumblelink/api/IdentityManipulator.java#L36
  2. mumo module needs to respond to unprecedented aspect

Edit: Maybe the source module gives a hint on how to solve it.

zsawyer avatar Feb 28 '21 01:02 zsawyer

I agree, Mumo is definitely the "correct" way to do this. Personally though I just don't want to manage plugins on my Mumble server 😄. I'm also not keen on maintaining a Murmur plugin alongside my mod, so if we can fit Minecraft data into an existing plugin's support, that would be great.

Supporting teams would be pretty difficult I think. I've never used teams, so I don't know what options they support.

We also have to keep in mind the 255-wchar limit on the identity field, which could be filled up by arbitrary dimension and team names. I assume just throwing a special error if that limit is reached would be enough, I doubt many people would reach it anyway.

magneticflux- avatar Feb 28 '21 02:02 magneticflux-

I somehow forgot about the 255 limit on identity.

The Forge mod code already does validate and throw an error before sending too long data.

zsawyer avatar Mar 01 '21 01:03 zsawyer

I just stumbled upon a request where someone wanted dead people to be muted: https://forums.mumble.info/topic/2375-paid-request-mumble-minecraft-plugin/?do=getNewComment

zsawyer avatar Mar 01 '21 01:03 zsawyer

@magneticflux- TeamSpeak. Not Teams. Teams is not really a consumer VOIP program and certainly not something that gamers would regularly use.

I see that you did a similar Y adjustment to what I did, but I am not quite sure in what range the values of the dimension hash code could be. If it's a very high value, the accuracy of the positional audio could decrease dramatically.

For as far as I see it, there are two options here and I do not see why we would not support both:

  1. Client sided configuration of a per-dimension identity/context/y-offset. This would only really be useful for modpacks where copies of the same configuration file are shared by all users. This would not really make sense in a "half of my users use Forge and the other half uses the Fabric version of the mod" situation. In MC 1.16 this should probably be a list of pairs of dimension registry names with an offset integer so something like:
{"minecraft:nether", -256},
{"minecraft:surface", 0},
{"minecraft:end", 256},
{"twilight_forest:twilight", 1},
etc.

If y-offset is going to be used, numbers -256 to 256 are allowed and the numbers should just be multiplied by 256 to get the y-offset. 512 dimensions is a lot for preregistered dimensions. I am ignoring dynamic dimension registration like RFTools and Mystcraft for now.

  1. An additional server-sided plugin/mod to synchronize on all clients which identity/context/y-offset should be used is very useful as long as both the Forge and Fabric mod support it. Configuration could/should be similar to a possible client-sided solution.

Should the client-sided mods include the server-sided solution to support the Open to LAN functionality? I've used this in combination with a Virtual LAN before, so I think it is a valid use-case.

Robijnvogel avatar Mar 01 '21 15:03 Robijnvogel

I am inclined to post-pone this into a release after #41 - I'd first like to get the context unision in. Eventhough both Forge and Fabric mod might provide different height data.

@magneticflux- : is that dimension-based height-offset active? I cannot tell from your code (it looks like it gets set to 0 everytime :?).

zsawyer avatar Mar 20 '21 00:03 zsawyer

@magneticflux- : is that dimension-based height-offset active? I cannot tell from your code (it looks like it gets set to 0 everytime :?).

That's correct; the dimension-based height offset is disabled (set to 0) by default. It can however be enabled in the configuration. I typically recommend people set up channel switching rather than a dimension offset since it helps Mumble/Teamspeak reduce network traffic, so I don't think there are many people using the feature (I intended it as more of a backup solution for when you don't have access to the Minecraft server).

magneticflux- avatar Mar 20 '21 03:03 magneticflux-

Ok I have updated to 1.17. I think we got a little side tracked by identity. Let's first focus on trying to agree on an implementation of the coordinate-offset approach.

  1. Implement client-side only approach A. Which value can be used and is by default synchronized across clients? B. Which distribution algorithm to use?
  2. For now I think it should be on by default with an option to turn it off.

zsawyer avatar Aug 30 '21 20:08 zsawyer

@magneticflux- can we agree on an approach here? See my previous post.

zsawyer avatar Dec 12 '21 01:12 zsawyer

I think the idea of distributing the worlds like this is good, but I have two issues:

  1. We should avoid special cases as much as possible, i.e. apply the distribution to vanilla worlds as well. Having special cases will increase complexity over time.
  2. We should prevent partial overlaps by establishing 512-block ranges for each dimension (for compatibility with 1.18 and future height increases)

~Additionally, the dimension's raw ID is an incrementing number which means we can get away with using it as an "index" instead of hashing it. It would take 256 dimensions to be registered before they loop back every 256.~ Nevermind, I was looking at old code. This doesn't work anymore, see below:

magneticflux- avatar Dec 12 '21 03:12 magneticflux-

@Robijnvogel Here's what I found for older versions if you want to backport to 1.15 instead of rebasing to 1.18:

  • 1.15: Use world.getDimension().getType().getRawId(). On Fabric at least this picks up modded dimension IDs properly, I assume Forge does the same.
  • 1.16+: No great option since this was the first time the dimension was able to be loaded from a resourcepack and Mojang didn't bother making a Registry implementation to store them. Instead, the GameJoinS2CPacket just serializes the Identifier instances (they're only used for command autocomplete as well). I think our best-case is to use world.registryKey.hashcode() or, to be really safe, implement our own String/Identifier hashcode.

magneticflux- avatar Dec 12 '21 06:12 magneticflux-

@Robijnvogel I started a prototype of such a stable hash function that you can use here: https://github.com/magneticflux-/fabric-mumblelink-mod/commit/3866317c64f9b7f5b9b4f17e88cd51c0a717b993

I checked that the three vanilla worlds all result in different values mod 2048. I also chose 2048 so that there would be at least 4 bits of intra-block precision since 32-bit floats have 24 bits of precision. (2^20 / 512 == 2048)

magneticflux- avatar Dec 17 '21 08:12 magneticflux-

I'm releasing a 1.18 version of my mod soon, so my proposed world ID hash will become harder to change. I don't plan on backporting to any other versions unless requested, so you're safe to just update directly to 1.18.

magneticflux- avatar Dec 30 '21 10:12 magneticflux-

@magneticflux- can you please explain to me what an example value of world.registryKey.value would return (https://github.com/magneticflux-/fabric-mumblelink-mod/blob/12727324ae9ecfc9c6b0ab5b604e824d43cfffa1/src/main/kotlin/com/skaggsm/mumblelinkmod/client/ClientMumbleLinkMod.kt#L136)?

I cannot relate this to anything other than game.level.dimension() which returns this: "ResourceKey[minecraft:dimension / minecraft:overworld]"

Also note that for me there exists game.player.level.dimension() which seems more appropriate.

zsawyer avatar Jan 24 '23 20:01 zsawyer

replaced by #63

zsawyer avatar Jan 24 '23 22:01 zsawyer