Fix multiplayer non host split screen#1031
Open
MrTheShy wants to merge 7 commits intosmartcmd:mainfrom
Open
Conversation
When a non-host client connected to a remote server through the in-game
UI (as opposed to the -ip/-port command line flags), the global variables
g_Win64MultiplayerIP and g_Win64MultiplayerPort were never updated from
their defaults ("127.0.0.1" and the default port). JoinSplitScreen()
relies on these globals to open a second TCP connection for the local
split-screen pad, so it would always attempt to connect to localhost,
failing immediately on any remote session.
Fix: update g_Win64MultiplayerIP and g_Win64MultiplayerPort inside
JoinGame() once the primary connection is established. This ensures
subsequent JoinSplitScreen() calls always reach the correct host
regardless of how the session was joined.
Additionally, guard PushFreeSmallId() against recycling smallIds in the
range [0, XUSER_MAX_COUNT), which are permanently reserved for the
host's local controller slots. Previously, if a host-side local pad
disconnected its smallId could re-enter the free pool and be handed
to an incoming remote client, causing that client's IQNetPlayer slot
to collide with a local pad slot on the non-host machine.
Replace the manual switch-case that computed viewport origin with the shared GetViewportRect/Fit16x9 helpers (from UISplitScreenHelpers.h). This ensures the tutorial popup is positioned and scaled consistently with the rest of the split-screen UI, fitting a 16:9 box inside each viewport and applying safezone offsets correctly. Also adds missing default:break to safezone switch statements to silence compiler warnings. Made-with: Cursor
Add g_KBMInput.IsWindowFocused() guard to the tryJoin condition so that gamepad input from background windows does not accidentally trigger a split-screen player join. This avoids phantom joins when the user is interacting with another application.
Pass eUIGroup_Fullscreen to NavigateToScene when opening the debug overlay, so it spans the entire window instead of being confined to a single split-screen viewport. This makes the debug info readable regardless of the current split-screen layout.
Contributor
Author
|
@codeHusky ready to be merged |
Previously, secondary (non-host) split-screen connections used isPrimaryConnection() to gate nearly all world update packets, meaning the second local player would never receive tile updates, entity movement, sounds, particles, explosions, etc. The fix introduces per-connection tracking of which entities and chunks each ClientConnection has loaded, and uses that information to decide whether a secondary connection needs to process a given packet or if the primary connection already handled it. New members in ClientConnection: - m_trackedEntityIds: set of entity IDs this connection has received AddEntity/AddMob/AddPlayer etc. for - m_visibleChunks: set of chunk coordinates (packed into int64) this connection has marked visible - Both sets are cleared on close(), respawn (dimension change), and destructor New helpers: - findPrimaryConnection(): walks the MultiPlayerLevel connection list to find the connection on the primary pad - shouldProcessForEntity(id): secondary connection skips the packet only if the primary is already tracking that entity - shouldProcessForPosition(x, z): secondary connection skips the packet only if the primary already has that chunk visible - anyOtherConnectionHasChunk(x, z): used when a chunk becomes invisible to avoid hiding it from the level if another connection still needs it - isTrackingEntity(id): public accessor used by shouldProcessForEntity on the primary connection Packet handler changes: - handleMoveEntity, handleMoveEntitySmall, handleSetEntityMotion, handleTakeItemEntity: replaced isPrimaryConnection() with shouldProcessForEntity() so secondary connections still process movement for entities they know about - handleExplosion, handleLevelEvent: replaced isPrimaryConnection() with shouldProcessForPosition() so block destruction and level events fire for the correct connection based on chunks - handleChunkTilesUpdate, handleBlockRegionUpdate, handleTileUpdate, handleSignUpdate, handleTileEntityData, handleTileEvent, handleTileDestruction, handleComplexItemData, handleSoundEvent, handleParticleEvent: removed the isPrimaryConnection() guard entirely -- these are world-state updates that all connections must process regardless of which pad is primary - handleChunkVisibilityArea / handleChunkVisibility: now populate m_visibleChunks; on visibility=false, setChunkVisible(false) is only called on the level if no other connection still has that chunk loaded - handleAddEntity, handleAddExperienceOrb, handleAddPainting, handleAddPlayer, handleAddMob: now insert into m_trackedEntityIds on arrival - handleRemoveEntity: now erases from m_trackedEntityIds on removal - handleLevelEvent: removed a duplicate levelEvent() call that was always firing regardless of the isPrimaryConnection() check above it (latent bug) MultiPlayerLevel: added friend class ClientConnection to allow access to the connections list without exposing it publicly.
Two issues in UIScene_FullscreenProgress::handleInput: 1. The touchpad/button press that triggers movie skip or input forwarding had no guard on m_threadCompleted, so pressing a button during the loading phase would fire the skip/send logic before the background thread finished. Added the m_threadCompleted check so that path is only reachable once the load is actually done. 2. The `handled = true` assignment was missing from that branch, so input events were not being consumed and could fall through to other handlers. Added it unconditionally at the end of the block.
|
Hey @MrTheShy That said, there are still a couple of quirks with the HUD in splitscreen (build based on commit
Vertical split, issues seen for player 2 (right):
In both variants there is also a minor glitch where the action bar moves when you switch focus to/from either the very first or the very last slot. I can provide screenshots if the descriptions are unclear. Sorry for posting this here in the PR, I can also create an issue, but since you're cleaning up the splitscreen experience I thought I'd just mention it here right away. Big thank you again! |
Refactor the condition for decrementing the player count in CPlatformNetworkManagerStub::DoWork. The previous check was replaced with a while loop to ensure that the player count is only decremented when there are more than one player and the last player's custom data value is zero. This change improves the handling of player connections in the network manager.
Contributor
Author
|
thanks! i'll try to replicate, can you please check if this still happens in the latest commit from this pr? write me up on discord @MrTheShy |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Several split-screen bugs affecting the non-host player on Windows64: joining a session started via the UI, phantom joins from unfocused windows, tutorial popup misplacement, and the debug overlay being clipped to one viewport. This PR addresses all four.
Changes
Previous Behavior
g_Win64MultiplayerIP/g_Win64MultiplayerPortglobals were never populated. A second local player attempting to join split-screen would therefore try to connect to an empty address, failing silently.switchover viewport types, manually offsetting byscreenWidth/2orscreenHeight/2. This did not account for the actual 16:9 fit box used by every other scene, so the popup appeared at the wrong coordinates — especially visible in quadrant and non-square split layouts.Root Cause
WinsockNetLayer::JoinGame(the path taken by a UI-initiated connection) did not write the resolved IP and port back into the globals thatJoinSplitScreenreads. The globals remained zeroed-out from startup.tryJoincondition inMinecraft::run_middlehad no window-focus guard, so gamepad input was evaluated even when the application was not in the foreground.UISplitScreenHelpersalready provides. It also passed the raw viewport dimensions toIggyPlayerSetDisplaySize, ignoring the fitted sub-rectangle.ui.NavigateToScenewas called without an explicit UI group, defaulting to the current split-screen group instead of the fullscreen group.New Behavior
Fix Implementation
WinsockNetLayer::JoinGame(WinsockNetLayer.cpp): After a successful connection and small-ID assignment, copy the resolvedipandportintog_Win64MultiplayerIP/g_Win64MultiplayerPort. Added a guard inPushFreeSmallIdto never recycle small IDs in the range[0, XUSER_MAX_COUNT), which are permanently reserved for the host's local pads — this prevents a recycled ID collision when a remote client reconnects.Minecraft::run_middle(Minecraft.cpp): Appended&& g_KBMInput.IsWindowFocused()to thetryJoinboolean so the join path is only evaluated when the window has focus.UIComponent_TutorialPopup::render(UIComponent_TutorialPopup.cpp): Replaced the manual per-viewport coordinate switch withGetViewportRect+Fit16x9fromUISplitScreenHelpers.h(the same helpers used byUIScene::render). The fitted width/height are now also passed toIggyPlayerSetDisplaySizeinstead of the raw viewport dimensions. Addeddefault: breakto both safezone switches to silence compiler warnings.Minecraft::tick(Minecraft.cpp): PasseUIGroup_Fullscreenas the fifth argument toui.NavigateToScenewhen opening the debug overlay, so it is placed in the fullscreen group and covers the whole window.AI Use Disclosure
Was used to write this pr description
Related Issues