[1.20.4/5] Improved ModMismatchDisconnectedScreen and clientside handling of channel mismatches in general

[RedstoneDubstep]

This PR aims to improve the user experience of getting disconnected from a NeoForge server due to channel mismatches, mainly by improving the usability of the ModMismatchDisconnectedScreen (the screen that shows instead of the default DisconnectedScreen if a NeoForge server disconnects the client due to channel mismatches) and also two other related changes. The ModMismatchDisconnectedScreen was previously adapted as part of the network refactor and its changes to handling channel mismatches, and this PR finalizes these adaptations to bring back most of the design elements and advantages that existed before the network refactor.

Changes to the ModMismatchDisconnectedScreen:

• Re-added "Connection Lost" message at the top of the screen, to clarify why the screen is shown
• Fixed the actual disconnection message possibly going outside the screen, now it will fit within the screen's dimensions
• Re-added alternate coloring for subsequent table entries, to be able to distinguish them better
• The channel mismatch reason is now automatically appended (either on the server or on the client) by the information of what mod likely owns the mismatched channel (based on the channel's namespace), so users have an easier time finding out which mod they might need to install or remove to fix their connection issues
• The channel list now merges channel mismatch entries with the same mismatch reason component into one singular entry (by default displaying one exemplary channel id and the amount of channels with an identical reason), to reduce list repetition and length; this also means that only entries of channels that are deemed to likely be owned by the same mod (based on the channel's namespaces) get merged
• Removed the assignment of the homepage URL of the mod of which the id matches a channel's namespace to that channel, mainly due to inconsistency (the URL could only be displayed when the mod was installed on the client)
• Added an "Simplified view" option button to the screen which controls if only one (default) or all channels that have the same reason component should be listed in the screen
• The channel list now displays a maximum 30 instead of 10 entries before showing the "[xyz additional, see latest.log for the full details]" message. This was done partly to bring back parity with the MMDS before the network refactor, in which the channel list had two parts (missing channels and mismatched channels) that showed 10 entries each, and partly because I felt that if we have a scroll list, we might as well include more stuff in it (since more entries don't take up more screen space).
• Fixed too early wrapping of the "Reason" component, and increased its size to half of the list (previously that table column stored a version string, which was a lot shorter than the reason components now are, which warrants the increased width of that column)
• Renamed some variables and updated some javadoc to reflect the recent changes made to this screen during the Network Refactor

Other changes:

• Configuration and play channels now store the id of the mod that registered them (the id is retrieved through the ModLoadingContext); this information is not sent over the network (to prevent this PR from potentially becoming a breaking change), and so only available on the side that the channel was registered on Edit: This was decided against by maintainers
• The client will now print a warning log message for all channels that failed to connect
• Fixed error message due to the packet handler trying to parse the ModdedNetworkSetupFailedPayload like a normal packet

Further points to discuss:

• Should we rename the current "Channel name" header to "Mod name" or something similar? This is because, in my opinion, users shouldn't concern themselves about what a channel is and thus the concept of channels should be mentioned as little as possible. This proposition is no longer relevant.
• Before the network refactor, the server sent all of its installed mods to the client so the client could use these mod names and particularly other metadata like version and URL for a more informative list of errors on the disconnected screen. Is that possible/feasible to add in the new protocol?
• The channel mismatch reasons sent from the server to the client are great for modders, but can be ambiguous to inexperienced mod users. Is it worth hardcode-checking the reason components (to know whether the channel was missing or mismatching) and adding a "Suggested actions: ..." text/button somewhere that shows players more user-friendly messages?

Visual comparison between the previous and updated ModMismatchDisconnectedScreen:

(Edit: the updated MMDS's design is outdated, for the current version see further below)


(Also of note in this example: the previous MMDS had 107 entries corresponding to the 107 mismatching channels, of which only 10 were actually displayed, while the updated MMDS has 5 entries corresponding to the 5 mismatching mods.)

[neoforged-pr-publishing]

• Publish PR to GitHub Packages

[RedstoneDubstep]

Alright so after these recent changes, channels are now properly grouped by their owning mod. This is achieved through the owning mod always being included (= mentioned) in the channel's mismatch reason, and because mismatched channel entries only get merged if their mismatch reasons are identical (note that in the new implementation, the channel namespace gets ignored in that merging logic, only the reason is checked), channels are strictly only merged if they were registered by the same mod.
Of note: There were some internal changes made to properly store and keep track of the mod id that owns the registered channel, however these changes have all been made in classes annotated as @ApiStatus.Internal, so none of the changes should be breaking ones.
Additionally, there is now a new button which controls if only one (default) or all channels that have the same reason component should be listed in the screen. Below is a visual comparison between the so-called "Simplified View" being active or not.
A re-review based on these new changes is greatly appreciated.

[RedstoneDubstep]

Hello, I know you are currently busy with the new snapshots and (if I understood that right) an internal network api refactor by Shadows, but a continuation of the conversation above would still be greatly appreciated. With my latest commit, I've now removed all usages of the static ModLoadingContext, as requested by Shadows, the (in my opinion even beneficial) side effect of this change is stated in the commit's description.

[Shadows-of-Fire]

To summarize the discussion surrounding this:

We have collectively decided the ownership-tracking approach (through the loading context) is not stable enough to warrant using, nor do we apply it for any of the other types of objects that are synchronized. We will be pursuing a new solution which better permits mods to declare their network compatibility and provide the user with actual information instead of relying solely on channel and registry compatibility.

That said, we are still interested in receiving the improvements to the Screen and the channel mismatch handling, so we can move forward here.

As far as the current change set goes, the modid params in ConfigurationRegistration, PlayRegistration, and NegotiableNetworkComponent should just be replaced with the actual registration ID (effectively making the modid in the IPayloadRegistrar defunct). If necessary, the ResourceLocation id can be added as a parameter to the *Registration classes, but NegotiableNetworkComponent should need no change.

[RedstoneDubstep]

Hello, thank you for the information! So to clarify, does that mean that no form of channel mismatch entry merging (even through tracking the owning mod of the channels) is going to get accepted (even though it has significant usability benefits compared to just listing all entries in my opinion)? If so, I can remove the relevant code from the PR to only make it consist of screen formatting fixes.

[Shadows-of-Fire]

We can still do the merging. Realistically the unmerged version is mostly useless to an end user due to the volume of information.

[RedstoneDubstep]

I agree, the unmerged information is just too vast and confusing for the general end user. However, I'm now confused about which criterion should be used to merge the channel mismatch entries, if both merging based on channel namespaces as well as merging based on owning mods (through mod ownership tracking) is off the table. Is there another option that allows for a usable and logical merging of channel mismatch entries that I have missed?

[Shadows-of-Fire]

Grouping by the registered namespace should be sufficient. This screen should basically be a last-effort fallback (same for the registry entry mismatch), as we will be expecting that mods providing networked components (registry entries, channels, data maps, etc) use the new system.

The new pipeline will be something akin to the following:

1. A declarative network version range, checked automatically as the first step of the connection.
2. Invocation of custom network compatibility checkers (for advanced handling beyond declarative version ranges).
3. The original checks, if no mods provide an explicit reason for the disconnect.

[RedstoneDubstep]

Alright, thank you for your explanation, I don't think I completely understand it (since I haven't fully kept track of maintainer discussions regarding these changes), but I will re-add grouping of channel mismatch entries based on channel id namespaces. Should I also re-add the logic that tries to find (and display the name of) the mod of which the mod id is the same as the merged channel namespace?

[Shadows-of-Fire]

Yeah we can retain the modid->name lookup feature.

[RedstoneDubstep]

Alright, the prior channel-namespace-merging logic has been restored. The visual appearance of the screen doesn't differ from the previous screenshot sent into here, so I'll not post another screenshot this time. I'll resolve the open review above, please tell me what else needs to be changed.

[RedstoneDubstep]

Hello, I've now resolved the merge conflicts with the 1.20.5 NeoForge changes and updated some code to make the MMDS work in 1.20.5 (the screen looks really cool with the main menu panorama background). Since Shadow's Network Refactor is now merged and there are (hopefully) no more major changes to the network architecture, I'd really like to know if/how to move forward with this PR. Since this PR does not introduce a breaking change, it doesn't need to be merged during the beta window, so if you have other priorities (which you probably do) those can always go first.

https://discord.com/channels/313125603924639766/852298000042164244/1244907334908641350