Mumblelink fabric and mumblelink forge

[Arcadise]

Hello,
I'm playing with friends on a minecraft server with mumblelink (1.16.5).
But I have a problem. "Positional audio" only work between two versions of mumblelink fabric or two versions of mumblelink forge but not between fabric and forge.
I talked with fabric mumblelink's dev and it seems like some values are different between the two versions of the mod. That means you will have to work together with the fabric version's dev to achieve compatibility, do you agree with that ?
Repo : https://github.com/magneticflux-/fabric-mumblelink-mod

[magneticflux-]

It seems like a ContextManipulator can be used to change the context.

By default, the context is set to {"domain":"AllTalk"}, and Fabric MumbleLink Mod's context is set to Minecraft.

[Arcadise]

You can change it or we need to wait forge mumblelink's dev to make changes in both mods ?

[magneticflux-]

I don't currently see a way to use ContextManipulator without an addition or change to this Forge mod. If you're desperate, you could make the change here:

MumbleLink/mod/src/main/java/zsawyer/mods/mumblelink/mumble/UpdateData.java

Lines 198 to 209 in 1e590a6

	protected String generateContext(Minecraft game, int maxLength) {
	try {
	JSONObject newContext = new JSONObject();
	newContext.put(ContextManipulator.ContextKey.DOMAIN,
	MumbleLinkConstants.MUMBLE_CONTEXT_DOMAIN_ALL_TALK);
	return newContext.toString();
	} catch (JSONException e) {
	MumbleLinkImpl.LOG.fatal("could not generate context", e);
	}
	return MumbleLinkConstants.MUMBLE_CONTEXT_DOMAIN_ALL_TALK;
	}

to replace the JSON generation with a constant "Minecraft" and compile a custom version.

The best solution would be to establish a standard for a "Minecraft" context. I'm interested in @zsawyer's opinion on what that should be.

[Arcadise]

I'm just asking I have plenty of time

[zsawyer]

I agree. It should be as simple as you said.

I used this json format in anticipation that there might be more flexibility needed. Which wasn't really the case.

I am thinking that this could be achieved by a plugin mod (using the API of this Forge MumbleLink). However that is probably overkill and it would be best to agree on a base value for context.

[zsawyer]

I am thinking either go with “Minecraft“ or use json {"game":"Minecraft"}.

Mumble docs say it should even contain server details.

A management bot might want to use this information for distributing users into correct channels. But then this group would probably also need their own ContextManipulator plugin where they could code the data protocol then selves.
But if we guide them by using json here, they might adapt JSON as a convention.

Sadly this would break backwards compatibility with older versions of this forge mod. So I might end up implementing it only starting with a certain minecraft version.
And maybe I'd provide an optional compatibility mod that works with previous forge MumbleLink mod versions.

[magneticflux-]

I propose deciding on JSON fields to include and releasing new versions of the mods simultaneously. I don't think breaking backwards compatibility is a big problem because most people using mods like this have a shared modpack that they play with other people (citation needed). I also don't support versions older than the latest on each major Minecraft version, so users are encouraged update relatively quickly.

[zsawyer]

I don't think it is like that. Looking at the download numbers and questions over the past week I think a lot of people are actually downloading forge-MumbleLink on curseforge directly (almost 9,000 downloads in 2 weeks; https://www.curseforge.com/minecraft/mc-mods/mumblelink/files).

I usually try to support all Minecraft versions while using the latest stable Forge version if possible (simply because it is more likely to have less things for us to change in the mod - talking about mappings).

However I don't want a hypothetical MumbleLink-1.16.5-5.0.0 as it would break compatibility with all those that installed MumbleLink-1.16.5-4.6.3. The issue would be completely intransparent and cause unnecessary confusion and frustration for the users.
The scenario would be that we have group A using version 4.6.3 and group B using 5.0.0 - how would any normal user know that they are not using the "right" version when it seems to randomly work with some (A-A, B-B) but not the others (A-B, B-A). Also the impact is that group A hears group B non-positional full center (and vise versa) which is extremely disturbing (trust me we had those issues more than enough when playing Project Reality - and I did support at that time in the forums) - I sure don't want that nightmare again.

That being said - We can't always prevent breaking inter-version-compatibility.

So I made up my mind:

1. Starting with 5.0.0 (or 6.0.0 - we still have to decide if we want Dimensional Height Offset #35 in it's own version) I would go with what ever convention we come up with. But there won't be a MumbleLink-1.16.5-5.0.0 with the new context convention.
2. Instead there will be a separately available forge mod that one can install in addition to the already existing MumbleLink-Versions similar like ExtendedPASupport.
   a. This will achieve immediate compatibility with the new convention.
   b. Maybe I'll even do another one that provides compatibility with your current context "Minecraft" (probably depends on how fast you update your fabric mod)
3. I don't think it will be a major issue if you release with the new convention prior to me if what you said is true for the fabric mod (manly included in mod packs, encourage getting the latest version) - but I recommend you following this similar strategy. Especially if I'd provide a 2.b.

Edit: added 3.

[magneticflux-]

Okay, I'll let you handle your mod's versioning however you want.

I'm planning on updating to the standard we settle on ASAP and bumping the minor version number since it's still in the 0.y.z Semantic Versioning stage. I don't think you'll need to make a separate Forge mod for compatibility with older versions of my Fabric mod, but you can if you still want to support 1.14 and 1.15.

Context format

As far as the exact format to use, let's look at the recommendations from the Mumble wiki here:

What the Context does:

If context between two Mumble users does not match the positional audio the data is stripped server-side and voice will be received as non-positional.

The Context should be the same for players on the:

• same server
• same game
• same map
• same team (possibly)

The wiki also states:

When in doubt err on the side of including less.

(I'm not sure how true this is since the strings are just compared for equality and any change is equally different.)

Implementation ideas

Game, map, and team information is easily available to our mods. The game is constant, the map could be the namespaced ID of the current world, and the team could be the team string.

Things get a little tricky trying to figure out something to represent the server though... We can't use IP or URL since multiple IPs or URLs could correspond to a single host. We can't use the server name since that's client-side. We probably can't use the server MOTD because some servers modify that based on the server state or player connecting. The only option I see for the server would be to generate a token of some kind in-memory server-side and send it to each client.

With all that analysis out of the way, here's what I propose:

Option 1:

Minecraft or {"game":"Minecraft"}

This is the simplest option. It doesn't support adding fields for parsing like JSON, but then again, adding a JSON field is just as big a change as moving from this option to JSON in the first place.

Option 2:

Minecraft-$serverId or {"game":"Minecraft","serverId":"abc123"}

This includes an ID for the server. I don't really expect anyone to be in the same Mumble server but different Minecraft servers. Even if they were, still hearing the other people but without positional audio would probably be even more unexpected.
This method also requires additional client-server info, which, while not difficult, isn't as trivial as inserting something locally available.

Overall thoughts

I'm less in favor of using JSON because the field ordering (if there are multiple fields) and the whitespace used matters to Mumble (since the strings content is just directly compared). That notion of equality isn't intuitive for a JSON document, and could cause subtle breakages if different JSON serializers use different orderings/whitespace.

Including the map isn't possible if you want to also make #35 (and the similar feature that's already in my mod) work properly. If two people were on different maps, the position data would be stripped, preventing any offset from muting other people.

Including the team might be confusing for maps or servers that use the "team" of a player to control certain features not related to competition, or if the teams should be able to hear each other.

[zsawyer]

You have some very good points there.

on Context format:

When in doubt err on the side of including less.

I agree. My experience tells me to go with the lowest common denominator.

I did include all that information in the beginning - it was terrible. That's why we have that "AllTalk" (which is actually a somewhat known phrase among gamers using mumble with pa).

Here is a discussion I had with the mumble devs regarding that topic, way back then:
https://forums.mumble.info/topic/456-minecraft-positional-audio/

The main point there was this:

The basic rule however is: When in doubt put the information into identity or leave it out before messing up context.

And here the feature request to stop this dreaded unlinking:
https://sourceforge.net/p/mumble/feature-requests/890/

on Overall thoughts

Good points.

JSON is not the way to go for the context for the reasons you mentioned. We need a format that leaves no room for different flavors. (For identity I think JSON is perfectly fine.)

If we would need data like that for server side handling we should really put it all that into identity as suggested by ddot.

on Implementation ideas

Option 1 (accepted)

Minecraft is the way to go. Since I ruled out JSON thanks to your critique.

Option 2

I rule it out because of above reasons from the past.

The basic rule however is: When in doubt put the information into identity or leave it out before messing up context.

Conclusion on Fabric and Forge mod collaboration

@magneticflux- @Robijnvogel Do you agree?

Context Convention

Since fabric-mumblelink-mod already uses the "correct" context. Only this Forge Mod needs updating.

We agree to use Minecraft as the common context string across both mods.

Dimension Offset

I would appreciate your feedback in #35 and think we should agree on the same offset implementation. Else Forge and Fabric mods only are working together properly in the "overworld" dimension.

[zsawyer]

This renders #4 obsolete. There will by no dynamic in the context to ensure maximum compatibility with the fabric mod.

[zsawyer]

Regarding your thoughts on how to get the server details to match up. I simply ignored that and instead used this implementation to get that data into ìdentity`:

MumbleLink/mod/src/main/java/zsawyer/mods/mumblelink/addons/pa/es/ExtendedPASupport.java

Lines 166 to 181 in 7326a2d

	private void appendToIdentity(JSONObject identity, Minecraft game)
	throws JSONException {
	// build spawn location coordinates (sadly this is the only somewhat
	// identifiable information the client has about the world (and server)
	// it connects to.
	// TODO: test if we can use game.world.getSeed()
	JSONArray spawnCoordinates = new JSONArray();
	spawnCoordinates.put(game.world.getWorldInfo().getSpawnX());
	spawnCoordinates.put(game.world.getWorldInfo().getSpawnY());
	spawnCoordinates.put(game.world.getWorldInfo().getSpawnZ());
	// append coordinates
	identity.put(IdentityKey.WORLD_SPAWN, spawnCoordinates);
	// append the dimension
	identity.put(IdentityKey.DIMENSION, game.player.getEntityWorld().getDimensionKey());
	}

Basically I assume that if the worlds are exactly the same then they must be on the same server.

[magneticflux-]

#38 (comment) That sounds good to me! In terms of the identity field, I plan on adding more info to it using the JSON schema you've already established.

#38 (comment) As far as switching channel per dimension, my mod already responds to certain packets from the server and opens a mumble:// URI, which is used for joining a server, changing dimension, or changing team. In the future, we should look into making a standard packet that our respective server mods could send in order to inform connected clients about which server/channel to join.

#38 (comment) That's a really clever way to solve that issue! I hadn't thought of using the server spawn as a pseudo-unique identifier for a given server.

[zsawyer]

I'd still like to wait for @Robijnvogel 's feedback. Then we can proceed with the context change.

Regarding dimension switching: let's continue that in #35

[zsawyer]

We agree to use Minecraft as the common context string across both mods.

I started editing the code. Then I stumbled upon the reason I was using JSON... it was not because it should be interpreted by the server. Instead it was meant to provide inter-mod compatibility in case multiple addon-mod's wanted to append to the context using ContextManipulator.

I am pondering if I should get rid of the ContextManipulator interface in full. Does it even make sense for addon-mods to manipulate the context? But then again I am no oracle and won't even dare dream of what use cases future people might come up with.
But atm I just don't see a good way to properly orchestrate multiple addon mods - especially since currently this probably is a non-existent use-case.

Or I could leave this problem to said future generations to solve - it is FOSS after all.

[zsawyer]

We might have to unify the plugin name as well!

During testing I have just seen that mumblePAhelper prepends the name to the context resulting in a context being displayed as MinecraftMinecraft for this forge mod and probably Minecraft Mumble Link ModMinecraft for the fabric mod.

fabric mod

https://github.com/magneticflux-/fabric-mumblelink-mod/blob/b6ffa961f070ae17241ecb9f5d2a1a7fb91c9cdd/src/main/kotlin/com/skaggsm/mumblelinkmod/ClientMumbleLinkMod.kt#L85
mumble.name = "Minecraft Mumble Link Mod"

forge mod

MumbleLink/mod/src/main/java/zsawyer/mods/mumblelink/mumble/MumbleInitializer.java

Lines 43 to 45 in 7326a2d

	public static final String PLUGIN_NAME = "Minecraft";
	public static final String PLUGIN_DESCRIPTION = "Link plugin for Minecraft with ModLoader";
	public static final int PLUGIN_UI_VERSION = 2;

This also explains why I initially didn't bother putting the game name into the context.

Any idea if the plugin name is actually being checked / prepended to the context or if this is simply a visual "glitch" of the mumblePAhelper?

[magneticflux-]

It looks like the name + context in MumblePAHelper is right here: https://github.com/mumble-voip/mumble-pahelper/blame/2ed12916c5662ec04879960e91514f6d7afe9492/Plugins.cpp#L214 and was introduced with the first commit 11 years ago.

I'm checking the actual source of Mumble now...
It looks like Mumble does the same thing here: https://github.com/mumble-voip/mumble/blame/40b28b03c150b453e00c6bc4f8d6957caea59c51/src/mumble/Plugins.cpp#L451

It creates a MumbleProto::UserState protocol buffer (src/Mumble.proto) and then uses g.sh->sendMessage to send it to the server. g is a global macro. sh is a shared pointer to a ServerHandler.

The server receives it here https://github.com/mumble-voip/mumble/blob/c8e8a84f11359affd6ba34dc670527931b45c204/src/murmur/Messages.cpp#L842-L847 and sets the field of a ServerUser.

Finally, Murmur reads the field here https://github.com/mumble-voip/mumble/blob/0b352ebee5b3b2d703296aa8fb9a2effc39a913e/src/murmur/Server.cpp#L1079-L1085 and conditionally sends a message (a packet of audio) to users.

I think we have to make the names match.

[zsawyer]

Suggestions? I actually prefer only "Minecraft" as the name.

Also I am questioning if we would need a context at all then?

[TjeuKayim]

spawn location coordinates (sadly this is the only somewhat identifiable information the client has about the world (and server) it connects to.

I haven't tested it, but maybe the servers public key can be used from https://github.com/FabricMC/yarn/blob/69c470b145d3c0ef4c1d6a2b327760d5586d7fcf/mappings/net/minecraft/network/NetworkEncryptionUtils.mapping#L14
According to https://wiki.vg/Protocol_Encryption, “The server generates a 1024-bit RSA keypair on startup.” So, I assume this key is the same for all clients that connect to the same server. Could this be useful for the Mumble context or identity?
(I've virtually no experience with Minecraft modding, but I am a software engineer.)

[zsawyer]

Would there be situations where server do not use encryption? Like LAN or unofficial servers?

[zsawyer]

@magneticflux- can we agree that the plugin name should be Minecraft only and that the context should be empty?

Looking at the other plugins' names they are all just named like the game.
While the description is the game's name + the version in braces:

[TjeuKayim]

Would there be situations where server do not use encryption? Like LAN or unofficial servers?
@zsawyer

I did some testing with offline servers and open-to-LAN, and the server public key turns out to be used in both cases. Even the Minecraft instance that acts as server in open-to-LAN performs the encryption handshake when the first other player joins. Each time you start a server or open-to-LAN, a random public key is generated. See this screenshot of my simple mod that hashes the public key bytes and logs it to the console:

So, such public key hash could to be used as identifiable information for the Minecraft server in all vanilla cases of multi-player. Though I'm not sure about cracked servers.

[magneticflux-]

spawn location coordinates (sadly this is the only somewhat identifiable information the client has about the world (and server) it connects to.

I haven't tested it, but maybe the servers public key can be used from https://github.com/FabricMC/yarn/blob/69c470b145d3c0ef4c1d6a2b327760d5586d7fcf/mappings/net/minecraft/network/NetworkEncryptionUtils.mapping#L14
According to https://wiki.vg/Protocol_Encryption, “The server generates a 1024-bit RSA keypair on startup.” So, I assume this key is the same for all clients that connect to the same server. Could this be useful for the Mumble context or identity?
(I've virtually no experience with Minecraft modding, but I am a software engineer.)

Conceptually, that would work for traditional servers. Imagine, however, a proxied server using something like BungeeCord or Waterfall. Each proxy generates unique keys at startup: https://github.com/SpigotMC/BungeeCord/blob/a7c6edeb638f0a2ba4fb051f2b0be5a6e226c955/proxy/src/main/java/net/md_5/bungee/EncryptionUtil.java#L36-L47 Are those the keys that the client sees?

The shared secret from the server used here https://github.com/SpigotMC/BungeeCord/blob/a7c6edeb638f0a2ba4fb051f2b0be5a6e226c955/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java#L419-L423 to decode and encode packets on the proxy-to-server Netty channel, so it seems like the Minecraft server only "sees" the Bungeecord keys?

[TjeuKayim]

You are right magneticflux. If each BungeeCord proxy has its own key pair and several proxies point to the same server, the public key is no longer a good identifier for the server. I guess large servers do use such architecture.

[TjeuKayim]

Most players won't join two Minecraft servers at once at the same time. But it is common to switch between survival and creative worlds to replicate a design. If MumbleLink uses a context without an identifier for the server, the coordinates of different servers would be used for positional audio. That's confusing.
Alas, it seems like Minecraft servers don't expose a suitable identifier. Leaving the context empty would then be appropriate I guess.

[TjeuKayim]

I had another thought. Would it be a good idea to base the Mumble context on the dimension ID? That wouldn't identity the server, but at least make sure that overworld and nether coordinates are not mixed up.
And we could create an optional server-side mod for proxied or multi-world server networks, that uses a PluginMessage channel to expose a world identifier (like extending the existing Fabric server mod by magneticflux).

[magneticflux-]

@TjeuKayim I think including the dimension ID in the context was ruled out in the second to last paragraph of my analysis here: #38 (comment). The core issue is that mismatched contexts don't strip audio data, they just strip position data.

Does that make sense?

[TjeuKayim]

Ah, I see. Sorry for not reading that part of your analysis correctly. When contex data doesn't match, you hear the other person full volume. That would make sense if he/she is in another (unrelated) MC world, but not for another dimension of the same world.

[Kuratius]

https://www.planetminecraft.com/mod/164-better-pvp-v10/
This mod identifies server and dimension for the purpose of keeping track of waypoints.

Mini-map interface - works above and under ground. Has togglable chunk grid and slime chunks modes. Can display all types of entities in customizable colours. You can also set your own waypoints. Waypoints can be teleported to if a player has permission to do so. Each server, world and dimension will have separate sets of waypoints. Press "B" to create a waypoint. Press "U" to list all the waypoints.

It may be a good idea to ask, even though the dev hasn't open sourced their mod. That said, I think whatever method they use it not considered 100 % reliable, as it still provides the option manually override the automatic identification.

[Kuratius]

@thexaero can you comment on this ?

[TjeuKayim]

Maybe you already found this, but here is a screenshot from the Xaero's World Map GUI explaining the configuration options:

I'm curious what Xaero himself can give as advice, as he faced a similar problem when developing his mods.

[thexaero]

@TjeuKayim I only skimmed through the conversation (at 1 AM) and I haven't used Mumblelink, so I hope I understand the issue correctly. Is it that you want to get a server identifier on the client side without using dimension IDs or a server mod? The dimension IDs can't be used because you want players on different dimensions from the same world to have the same context and only different servers to have different context. Correct?
I've been dealing with the issue of identifying the server/world on the same address for a while, but the situation is slightly different here.
I'm almost certain it was not possible to differentiate between servers reliably pre 1.16 without a server mod. In 1.16, a lot has changed. Thanks to how dimensions IDs work now, servers often give sub-servers unique dimension ids, for example minecraft:overworld_creative, minecraft:the_nether_creative on one server and minecraft:overworld_survival, minecraft:the_nether_survival on another. You can't really extract a unique server identifier out of it though. It works in the case of my map mods because my mods don't need a server identifier when the dimension ID is already different. With the addition of custom compasses, even using the world spawn coordinates became more reliable, as modding the default compass became pretty pointless (but still happens). Although, from my experience, I do not recommend using world spawn coordinates as a server identifier. It is still unreliable and will likely cause problems that are worse than the one you are trying to solve.
Maybe there are other helpful recent changes that I'm not aware of but I doubt it. I would love to see this issue figured out though. Best of luck!

[Kuratius]

You probably want to tag @zsawyer and @magneticflux- since they're the maintainers of the respective projects.

I wonder if a combination of dimension_ID and the public key is unique enough for most situations?
Would automatically sorting people into the correct VC for respective dimensions use context or identity?

[zsawyer]

You probably want to tag @zsawyer and @magneticflux- since they're the maintainers of the respective projects.

I wonder if a combination of dimension_ID and the public key is unique enough for most situations?
Would automatically sorting people into the correct VC for respective dimensions use context or identity?

Identity seems to be best practice.

[zsawyer]

@thexaero thank you for your valuable input.

Is it that you want to get a server identifier on the client side without using dimension IDs or a server mod?

Dimension ID would be ok. But I had not considered it to be reliable or unique enough.

The dimension IDs can't be used because you want players on different dimensions from the same world to have the same context and only different servers to have different context. Correct?

We are not exactly sure what the best approach is regarding context. Shipping the dimension ID would only work if it is synced across all clients. But if it's a locally generated ID this is a deal breaker because clients would not be able to figure out that they are in the same dimension actually.

In 1.16, a lot has changed. Thanks to how dimensions IDs work now, servers often give sub-servers unique dimension ids, for example minecraft:overworld_creative, minecraft:the_nether_creative on one server and minecraft:overworld_survival, minecraft:the_nether_survival on another. You can't really extract a unique server identifier out of it though. It works in the case of my map mods because my mods don't need a server identifier when the dimension ID is already different. With the addition of custom compasses, even using the world spawn coordinates became more reliable, as modding the default compass became pretty pointless (but still happens). Although, from my experience, I do not recommend using world spawn coordinates as a server identifier. It is still unreliable and will likely cause problems that are worse than the one you are trying to solve.

Supplementing these unique dimension ids (since 1.16) to the world spawn sounds like the right thing to do.
What would be issues with using world spawn that you are taking about?

I would love to see this issue figured out though. Best of luck!

Same ;)

[thexaero]

@zsawyer No problem!

Dimension ID would be ok. But I had not considered it to be reliable or unique enough.
We are not exactly sure what the best approach is regarding context. Shipping the dimension ID would only work if it is synced across all clients. But if it's a locally generated ID this is a deal breaker because clients would not be able to figure out that they are in the same dimension actually.

I was talking about using the dimension ID for context not being useful. If I remember correctly, different context = full volume? Or can you control this behavior?
Otherwise, dimension ID should be pretty reliable on 1.16+, if you want to sort players into different channels. The dimension ID is usually sent by the server. I don't know if it's ever generated on the client side but that could be a thing. Although I think this would probably still result in matching IDs on different clients in most cases. But yeah, some servers don't give different world saves their own unique dimension IDs, which isn't fun. That shouldn't make the situation any worse than it currently is though.

Supplementing these unique dimension ids (since 1.16) to the world spawn sounds like the right thing to do.
What would be issues with using world spawn that you are taking about?

The issues come from the fact that the server can update the world spawn coordinates at any time. For example, a lot of servers use the compass to point players to their latest death location, other players or, most commonly, to their personal spawn (bed). They achieve it by sending fake world spawn coordinates to the client. Mojang finally gave us custom compasses, but some servers will still use the old method (e.g. multi-version servers).

[magneticflux-]

I'm preparing a release of my mod for 1.17 in the next few days, so I'd like to wrap this up so there aren't any version discrepancies in 1.17 and later (since some users may only install the first version for 1.17 and never update).

My current plan is to use Minecraft in the context field and to try to include more info in a JSON string in the identity field. I think that's the most compatible and extensible way forward. I think we can discuss adding more standard JSON info in the identity field if that's needed by plugins on an as-needed basis.