Skip to content

Improve duel client connection handling and bot room startup#3133

Open
mercury233 wants to merge 12 commits into
Fluorohydride:masterfrom
mercury233:patch-connect
Open

Improve duel client connection handling and bot room startup#3133
mercury233 wants to merge 12 commits into
Fluorohydride:masterfrom
mercury233:patch-connect

Conversation

@mercury233

@mercury233 mercury233 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR improves duel client connection lifecycle handling, LAN room discovery, and bot room startup behavior.

Changes

  • Move host name resolution out of menu_handler and into DuelClient.

    • Keeps menu handling easier to read.
    • Reduces coupling so menu_handler can be made cheaper to compile in the future.
  • Use inet_pton for parsing numeric IPv4 addresses.

    • inet_pton is available on Windows since Vista, which is older than the supported Windows baseline.
    • DNS names are still resolved through evutil_getaddrinfo.
  • Add named constants for duel client connection state.

    • Replaces magic bit values with explicit CONNECT_STATE_* names.
    • Makes the later connection-state fixes easier to follow.
  • Replace the client is_closing flag with close_reason.

    • Makes normal stop and application-exit behavior explicit.
    • CLIENT_CLOSE_REASON_EXIT skips a little more UI/update handling than a normal stop.
  • Fix disconnect handling while the client is connected but has not joined a room yet.

    • Previously, disconnecting in the CONNECTED state could leave the client stuck forever.
    • One reproducible case was connecting to an unreleated endpoint such as www.baidu.com:443.
  • Track and cancel the client connect timeout event.

    • Prevents the timeout event allocated by event_new from leaking.
    • Cancels it after successfully joining a room or when the client thread exits.
  • Disable the client bufferevent in StopClient.

    • Avoids extra processing after the client has already started closing.
  • Fix LAN room refresh cleanup paths.

    • Failed setup paths now restore the refresh state and re-enable the refresh button.
    • Refresh completion now goes through a shared cleanup helper.
  • Stop using gethostbyname for LAN room refresh.

    • Local IPv4 addresses are now collected with evutil_getaddrinfo.
  • Make bot rooms private by default.

    • Bot rooms now bind to localhost, do not broadcast to LAN, and use a random available port by default.
    • This prevents other LAN users from unexpectedly seeing or watching bot games.
    • Random ports also allow multiple YGOPro instances on the same machine to host bot games without competing for the configured fixed port.
    • A new bot_room_public config option restores the previous public fixed-port behavior.
  • Delay spawning the bot process until after the host has joined the room.

    • Avoids a theoretical startup race where the bot could connect before the host finishes entering the room.
    • If bot process startup fails, the host client disconnects and lets the server shut down through its normal host-leave path instead of calling NetServer::StopServer() directly.
    • Bot startup failure now shows a dedicated error message.
    • Closes fix spawn fail in BUTTON_BOT_START #3132 .
  • Allow StopClient to stop the client from any active connection state, not only after successfully joining a room.

    • The close reason is updated even when no client connection is active, allowing an in-progress LAN refresh to suppress UI updates during application shutdown.
  • Extend NetServer::StartServer.

    • Supports selecting the bind address and broadcast policy.
    • Reports the actual bound port when the operating system assigns one.
  • Improve LAN discovery across multiple interfaces.

    • Duplicate local IPv4 addresses are ignored.
    • Failure on one local address no longer prevents trying the remaining addresses.

Future Work

  • Deduplicate repeated UI reset calls such as btnCreateHost->setEnabled(true).
  • Move DNS lookup to an asynchronous path.
  • Move bot process startup to a separate thread.

Previously, this failure path only called `StopServer`. When the server shut down, the client would encounter an error or timeout, allowing it to restore the UI. Although the recently added `StopClient` is semantically clearer, it may does not trigger the client's failure path, so the UI is not restored.
fix ignoring BEV_EVENT_EOF between connected and joined

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors and hardens the duel client/server connection lifecycle, improves LAN host discovery reliability, and changes bot-room startup to be private-by-default with randomized ports and delayed bot spawning.

Changes:

  • Centralize host name resolution in DuelClient (using inet_pton for numeric IPv4 and evutil_getaddrinfo for DNS).
  • Improve connection lifecycle correctness (explicit connect states/close reasons, connect-timeout tracking/cancellation, safer disconnect handling).
  • Make bot rooms private by default (bind localhost, random port, no LAN broadcast unless configured), and defer bot process spawn until after the host joins.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
strings.conf Adds a new system string for bot-start failure messaging.
gframe/netserver.h Extends StartServer API to support bind IP, actual port output, and optional broadcast.
gframe/netserver.cpp Implements bind-IP and ephemeral-port support and gates broadcast by configuration.
gframe/menu_handler.cpp Moves hostname resolution into DuelClient and updates bot-room startup to use private-by-default randomized local binding.
gframe/game.h Adds bot_room_public config and pending bot spawn state.
gframe/game.cpp Loads/saves new bot_room_public option and adjusts client stop reason on exit.
gframe/duelclient.h Adds explicit connect-state/close-reason constants and exposes ResolveHostName.
gframe/duelclient.cpp Implements hostname resolution, connect-timeout tracking/cancellation, improved disconnect paths, safer LAN refresh cleanup, and delayed bot spawning.
Comments suppressed due to low confidence (1)

gframe/duelclient.cpp:4187

  • SOCKADDR_IN local and SOCKADDR_IN sockTo are not zero-initialized before being passed to bind/sendto. Uninitialized padding/fields can trigger warnings (and, on some platforms, can be observable via kernel copies).
	SOCKADDR_IN local;
	local.sin_family = AF_INET;
	local.sin_port = htons(7922);
	SOCKADDR_IN sockTo;
	sockTo.sin_addr.s_addr = htonl(INADDR_BROADCAST);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gframe/duelclient.cpp
Comment thread gframe/duelclient.cpp Outdated
Comment thread gframe/menu_handler.cpp
mercury233 and others added 4 commits July 5, 2026 22:04
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This reverts commit 9ad8908.

The threading issue has not been resolved. Releasing it here would instead cause a conflict.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants