Skip to content

Comments

iCloud Sync v1#2414

Merged
JoeMatt merged 11 commits intoProvenance-Emu:feature/EmulationScenefrom
pabloarista:feature/iCloud-sync-v1
Apr 24, 2025
Merged

iCloud Sync v1#2414
JoeMatt merged 11 commits intoProvenance-Emu:feature/EmulationScenefrom
pabloarista:feature/iCloud-sync-v1

Conversation

@pabloarista
Copy link
Contributor

@pabloarista pabloarista commented Apr 18, 2025

User description

What does this PR do

Adds iCloud syncing to provenance. Removes constant values and replaces them with build variables for entitlements, plist and project files.

Where should the reviewer start

It would be easiest to just start with the build variables added

How should this be manually tested

I tested a multitude of combinations. So it was a small library and a large library. Within that it could be a combination of a small number of files for each kind: ROMs, Save States, Battery States, BIOS, Screenshots and RetroArch. For the ROMs different tests of just single file ROMs and then mixing with multi file ROMs. The importer sometimes imports those multi file ROMs fine and sometimes not. I believe that requires another PR if the upcoming import changes doesn't fix it. Really the ROMs and save states is what matters for many files because everything else doesn't get imported into the db. a small library would be less than 500 files and a large one would be larger than that, I would say at least 1k-2k files overall. The large libraries benefits from the pre-downloading. On older chips, it can be really sluggish for large libraries, but that's expected since the hardware is dated.

Any background context you want to provide

None.

What are the relevant tickets

Only one I can find is this one: Game Library Syncing

Screenshots (important for UI changes)

No UI changes

Questions

  1. I was going to add a unit test for the PVFile, but seeing as it's realm I'm not sure if that made sense. Other than that, I'm not sure if there's specific classes you want me to unit test which I have no problem doing that.
  2. I'll be available for issues discovered by users with TF too

PR Type

Enhancement, Bug fix


Description

  • Introduces a comprehensive iCloud sync system overhaul, leveraging async/actor-based concurrency for robust file and database management, including new types, protocols, and thread-safe data structures.

  • Adds a Mednafen network play server implementation, including C/C++ bridges, configuration, error handling utilities, and protocol logic.

  • Implements a Realm-based SwiftUI game library driver for efficient game data access and preview support.

  • Adds advanced DeltaSkin input handling, supporting comprehensive button mapping, analog controls, and special emulator commands.

  • Introduces a retrowave-styled in-game menu and default controller skin for the emulator, with dynamic layouts, neon effects, and responsive UI.

  • Integrates audio visualizer support into the emulator view controller, with multiple visualization modes and seamless SwiftUI/UIKit integration.

  • Updates controller mappings (e.g., Lynx, PS2) for improved accuracy and bug fixes, including null checks and button tag corrections.

  • Adds or updates various utility headers and implementations (MD5, time, types, Reachability) to support new features and system integration.

  • Refactors and expands iCloud sync logic to support large libraries, error handling, and notification-based triggers.

  • Removes obsolete SwiftUI views and components, streamlining the UI codebase.


Changes walkthrough 📝

Relevant files
Enhancement
18 files
MednafenControllerMappings.h
Update Lynx controller mapping array to add Pause button 

Cores/Mednafen/Sources/MednafenGameCoreC/include/MednafenGameCoreC/MednafenControllerMappings.h

  • Updated the LynxMap mapping array to include an additional element,
    changing its mapping order and adding a new value (8) at the end.
  • +2/-2     
    MednafenServerBridge.c
    Add C bridge for Mednafen server main entry point               

    Cores/Mednafen/Sources/MednafenServerBridge/MednafenServerBridge.c

  • Added a C bridge file that provides a function (mednafen_server_main)
    to invoke the Mednafen server's main function from other code, passing
    a configuration path.
  • Handles argument construction and memory management for calling the
    server's main function.
  • +27/-0   
    MednafenServerBridge.h
    Add header for Mednafen server C bridge entry point           

    Cores/Mednafen/Sources/MednafenServerBridge/include/MednafenServerBridge.h

  • Added a C header file declaring the mednafen_server_main function for
    external use.
  • Provides documentation for the function's purpose and parameters.
  • +11/-0   
    main.c
    Add main function stub for Mednafen server bridge integration

    Cores/Mednafen/Sources/mednafen-server/src/main.c

  • Added a stub C file that renames the main function to main_function
    for use with the C bridge.
  • Placeholder for the original main function code.
  • +4/-0     
    md5.cpp
    Add RFC 1321 compliant MD5 implementation                               

    Cores/Mednafen/Sources/mednafen-server/src/md5.cpp

  • Added an RFC 1321 compliant MD5 implementation in C++.
  • Provides functions for MD5 hashing, including initialization, update,
    finish, and ASCII string conversion.
  • +249/-0 
    md5.h
    Add header for MD5 hashing functions                                         

    Cores/Mednafen/Sources/mednafen-server/src/md5.h

  • Added header file for the MD5 implementation, declaring the context
    structure and function prototypes.
  • +18/-0   
    mednafen-server.cpp
    Add Mednafen network play server implementation                   

    Cores/Mednafen/Sources/mednafen-server/src/mednafen-server.cpp

  • Added a large C++ source file implementing the Mednafen network play
    server.
  • Includes server configuration, networking, protocol handling,
    client/game management, and main server loop.
  • Implements custom protocol commands, error handling, and communication
    logic.
  • Provides detailed comments and TODOs for future improvements.
  • +1998/-0
    time64.cpp
    Add time utility functions for monotonic time and sleep   

    Cores/Mednafen/Sources/mednafen-server/src/time64.cpp

  • Added utility functions for retrieving monotonic time in microseconds
    and sleeping for a specified duration.
  • Supports multiple system APIs for time and sleep.
  • +93/-0   
    time64.h
    Add header for time utility functions                                       

    Cores/Mednafen/Sources/mednafen-server/src/time64.h

  • Added header file declaring time utility functions for 64-bit time and
    sleep.
  • +7/-0     
    types.h
    Add types header for fixed-width integers and macros         

    Cores/Mednafen/Sources/mednafen-server/src/types.h

  • Added header file defining fixed-width integer types and
    compiler-specific macros for inlining and attributes.
  • Provides typedefs for int8, int16, int32, int64, and their unsigned
    counterparts.
  • +44/-0   
    Reachability.h
    Add public header for Reachability framework                         

    External/Reachability.swift/Sources/Reachability.h

  • Added a public Objective-C header for the Reachability framework.
  • Declares version number and version string symbols.
  • +17/-0   
    RealmGameLibraryDriver.swift
    Add Realm-based SwiftUI game library driver and data source

    PVUI/Sources/PVUIBase/SwiftUI/GameMoreInfoUI/Drivers/RealmGameLibraryDriver.swift

  • Added a new Swift file implementing a Realm-based game library driver
    for SwiftUI.
  • Provides methods for accessing, updating, and observing game data
    using Realm.
  • Implements artwork loading, notification handling, and context menu
    delegate methods.
  • Includes a preview helper for generating mock data in previews.
  • [link]   
    iCloudSync.swift
    Major iCloud sync system overhaul with async/actor concurrency and
    robust file/database management

    PVLibrary/Sources/PVLibrary/Importer/iCloud/iCloudSync.swift

  • Introduces comprehensive iCloud sync logic for Provenance, including
    new async/actor-based concurrency models.
  • Adds new types and protocols for iCloud container syncing, error
    handling, and file management.
  • Implements iCloudContainerSyncer, iCloudSaveStateSyncer, and
    iCloudRomsSyncer classes for handling different file types.
  • Adds actor-based thread-safe data structures (ConcurrentSet,
    ConcurrentQueue, ConcurrentDictionary, ConcurrentSingle).
  • Refactors and expands iCloud sync logic to support large libraries,
    error handling, and database purging.
  • Adds logic for initial iCloud setup, file migration, and
    notification-based sync triggers.
  • Provides detailed logging and debugging for iCloud operations.
  • +1621/-330
    DeltaSkinInputHandler.swift
    Add DeltaSkinInputHandler for advanced skin-based input mapping and
    forwarding

    PVUI/Sources/PVUIBase/SwiftUI/DeltaSkins/Models/DeltaSkinInputHandler.swift

  • Adds a new DeltaSkinInputHandler class to handle input from Delta
    Skins and forward it to the emulator core or controller.
  • Implements comprehensive button mapping, analog stick handling, and
    special command support (quicksave, quickload, fast forward, slow
    motion).
  • Supports notification-based reconnection and skin change handling for
    robust input state management.
  • Provides detailed logging, error handling, and fallback mechanisms for
    input forwarding.
  • Defines new protocols for controller and analog responders.
  • +1116/-0
    RetroMenuView.swift
    Introduce retrowave-styled SwiftUI in-game menu with skin/filter
    options

    PVUI/Sources/PVUIBase/PVEmulatorVC/RetroMenuView.swift

  • Added a new SwiftUI view RetroMenuView implementing a retrowave-styled
    in-game menu for the emulator.
  • Provides category-based navigation (Main, States, Options, Skins) with
    dynamic content and responsive layout.
  • Integrates skin and filter selection, button effect/sound pickers, and
    applies user selections with session/game/system scope.
  • Includes advanced UI features: neon effects, animated transitions,
    orientation handling, and async skin loading.
  • +1493/-0
    EmulatorWithSkinView+DefaultSkin.swift
    Add retrowave-styled default controller skin for emulator view

    PVUI/Sources/PVUIBase/SwiftUI/DeltaSkins/Views/Display/EmulatorWithSkinView+DefaultSkin.swift

  • Added a comprehensive SwiftUI implementation for a default controller
    skin with retrowave styling.
  • Supports dynamic layouts based on system control data, including
    D-pad, joystick, action, shoulder, and utility buttons.
  • Implements custom UI components: neon text, retrowave backgrounds,
    grids, sun effects, and touch indicators.
  • Handles both portrait and landscape orientations, and adapts to
    system-specific control layouts.
  • +1395/-0
    PVEmulatorViewController+AudioVisualizer.swift
    Add audio visualizer integration and management to emulator view
    controller

    PVUI/Sources/PVUIBase/PVEmulatorVC/PVEmulatorViewController+AudioVisualizer.swift

  • Added an extension to PVEmulatorViewController for audio visualizer
    support.
  • Implements setup, removal, toggling, and mode selection for various
    audio visualizer styles (standard, circular, Metal-based).
  • Handles orientation changes, view hierarchy management, and user
    preferences for visualizer mode.
  • Integrates SwiftUI-based visualizer views into UIKit with proper
    positioning and z-order.
  • +363/-0 
    PVAzaharCore.swift
    Specify skin support capability for Azahar core                   

    Cores/Citra/PVAzaharCore/Core/PVAzaharCore.swift

  • Explicitly set supportsSkins property to false for the Azahar core.
  • +2/-0     
    Configuration changes
    1 files
    config.h
    Add generated config header for Mednafen server build       

    Cores/Mednafen/Sources/mednafen-server/include/config.h

  • Added a generated configuration header file with macros indicating
    available system features, library versions, and package information.
  • Defines macros for system capabilities (e.g., presence of certain
    functions and headers).
  • +109/-0 
    Error handling
    2 files
    errno_holder.cpp
    Add error handling utility for errno string management     

    Cores/Mednafen/Sources/mednafen-server/src/errno_holder.cpp

  • Added implementation of the ErrnoHolder class for error handling.
  • Provides logic for storing and retrieving error codes and messages,
    with support for strerror_r if available.
  • +47/-0   
    errno_holder.h
    Add header for error handling utility class                           

    Cores/Mednafen/Sources/mednafen-server/src/errno_holder.h

  • Added header file declaring the ErrnoHolder C++ class for managing
    error codes and messages.
  • Provides interface for setting and retrieving error information.
  • +42/-0   
    Bug fix
    2 files
    runloop.c
    Add null check for current_core in core_run function         

    CoresRetro/RetroArch/PVRetroArchCore/Core/runloop.c

  • Added a null check for current_core in the core_run function to
    prevent potential crashes.
  • Returns early if current_core is NULL.
  • +5/-1     
    PVPS2ControllerViewController.swift
    Fix PS2 controller cross button tag assignment for lowercase variant

    PVUI/Sources/PVUIBase/Controller/Systems/PVPS2ControllerViewController.swift

  • Fixes button tag assignment to handle lowercase "✕" for the cross
    button.
  • Minor formatting adjustment for code consistency.
  • +2/-2     
    Additional files
    101 files
    Package.swift +62/-4   
    MednafenGameCoreBridge+Controls.mm +13/-4   
    MednafenGameCoreBridge.mm +30/-10 
    InputMaps.swift +1/-1     
    NetworkConnection.swift +52/-0   
    AUTHORS [link]   
    COPYING +340/-0 
    ChangeLog +157/-0 
    INSTALL +237/-0 
    Makefile +745/-0 
    Makefile.am +1/-0     
    Makefile.in +745/-0 
    NEWS [link]   
    README +48/-0   
    TODO [link]   
    acinclude.m4 [link]   
    aclocal.m4 +1077/-0
    autogen.sh +10/-0   
    compile +142/-0 
    config.guess +1516/-0
    config.status +1219/-0
    config.sub +1626/-0
    configure +7602/-0
    configure.ac +45/-0   
    depcomp +589/-0 
    config.h.in +108/-0 
    config.h.in~ +111/-0 
    stamp-h1 +1/-0     
    install-sh +519/-0 
    ax_cflags_gcc_option.m4 +225/-0 
    missing +367/-0 
    run.sh +3/-0     
    serverlog [link]   
    serverlog2 +1/-0     
    errno_holder.Po +91/-0   
    md5.Po +116/-0 
    mednafen-server.Po +1588/-0
    time64.Po +272/-0 
    Makefile +521/-0 
    Makefile.am +7/-0     
    Makefile.in +521/-0 
    rand.inc +44/-0   
    run.sh +1/-0     
    standard.conf +1/-0     
    trim.inc +50/-0   
    standard.conf +30/-0   
    PVPPSSPPCore.swift +2/-0     
    VirtualJaguar +1/-1     
    PVEmuThreeCore.swift +2/-0     
    PVRetroArchCore+RetroArchUI.m +1/-0     
    PVRetroArchCore+Controls.swift +4/-1     
    PVRetroArchCoreCore.swift +3/-1     
    project.pbxproj +4/-4     
    IndexRequestHandler.swift +268/-59
    Info.plist +9/-0     
    Spotlight.entitlements +1/-1     
    SpotlightMockDriver.swift +160/-0 
    ImportExtension.swift +279/-23
    Info.plist +29/-2   
    Info.plist +3/-1     
    PVGame+TopShelf.swift +15/-7   
    ServiceProvider.swift +124/-61
    TopShelf.entitlements +2/-2     
    ContentProvider.swift +346/-0 
    DebugLogger.swift +79/-0   
    Info.plist +62/-0   
    PVGame+TopShelf.swift +66/-0   
    TopShelf.entitlements +11/-0   
    TopShelfDataDriver.swift +418/-0 
    CHANGELOG.md +69/-0   
    CONTRIBUTING.md +34/-0   
    LICENSE +19/-0   
    Package.swift +25/-0   
    README.md +171/-0 
    project.pbxproj +989/-0 
    contents.xcworkspacedata +7/-0     
    IDEWorkspaceChecks.plist +8/-0     
    WorkspaceSettings.xcsettings +8/-0     
    Reachability.xcscheme +95/-0   
    AppDelegate.swift +19/-0   
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +17/-0   
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +17/-0   
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +32/-0   
    Contents.json +16/-0   
    Contents.json +16/-0   
    Contents.json +6/-0     
    Contents.json +22/-0   
    Main.storyboard +58/-0   
    Additional files not shown

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • … added new notification names relating to iCloud syncing; updated PVFile initializers to use iCloud as relativeRoot where necessary; added posting a notification when ROMs db finished initializing; updated pathDecoded to use a function that does NOT return nil; updated game importer to post a notification when finished importing; added a case for determineSystems to check the current parent directory and use that as the system identifier if the import path isn't in the import directory; updated Realm converters to allow non MainActor; added actualPartialPath on PVFile that fixes the partialPath; added check for file existence in PVImageFile calculateSizeData(); added date/timestamp on logs; added actors for concurrent collections and database access; refactored iCloud code from app delegate to iCloudSync; fixed/implemented missing iCloud syncing for all directories; added deletion of iCloud files if the app was closed by checking in the db and purge those that no longer exist; added code to pre-download files initially because the iCloud documents API doesn't play nice when there's 500+ files; added aggregating multi file ROMs into a dictionary and sends those to import once everything has been downloaded, per iteration (initial or normal sync); added attempt to convert a PV 2.x to a 3.x save state; added an errors queue that can be used to display to the user any iCloud sync issues
    @qodo-code-review
    Copy link

    qodo-code-review bot commented Apr 18, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 363aeea)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    1 - PR Code Verified

    Compliant requirements:

    • Implement game library syncing functionality
    • Sync ROMs, saves, etc.

    Requires further human verification:

    • Verify that the iCloud sync implementation works correctly with different library sizes
    • Confirm that the sync works for all file types (ROMs, Save States, Battery States, BIOS, Screenshots, RetroArch)
    • Test multi-file ROM handling during sync

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    Sensitive information exposure:
    The Mednafen server implementation includes password handling (lines 332-336 in mednafen-server.cpp) where passwords are stored in memory after MD5 hashing. While hashing is used, there's no salt applied, making it vulnerable to rainbow table attacks. Additionally, there are multiple places where buffer operations occur without proper bounds checking, which could lead to buffer overflows and potential remote code execution vulnerabilities.

    ⚡ Recommended focus areas for review

    Security Concerns

    The Mednafen server implementation contains several potential security issues including buffer handling, memory allocation without proper checks, and string operations that could lead to buffer overflows.

    /* FIXME IMPORTANT:
    	Add setting to restrict /take and /dupe.
    
    	Add setting to restrict arbitrary save state loading.
    
    	Add setting to restrict command < 0x40(IE reset, power, disc change, etc.).
    
    */
    
    /* Mednafen Network Play Server
     *
     * This program is free software; you can redistribute it and/or modify
     * it under the terms of the GNU General Public License as published by
     * the Free Software Foundation; either version 2 of the License, or
     * (at your option) any later version.
     *
     * This program is distributed in the hope that it will be useful,
     * but WITHOUT ANY WARRANTY; without even the implied warranty of
     * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
     * GNU General Public License for more details.
     *
     * You should have received a copy of the GNU General Public License
     * along with this program; if not, write to the Free Software
     * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
     */
    
    /*
     TODO to protocol 4:
    	Revamped login process:
    		Give protocol-version-agnostic error message on unsupported protocol versions.
    
    		<other stuff>
    
     Next version TODO:
    	More verbose quit messages(sent to remaining players).
    
    	Better exception handling - different exceptions for client-specific error and game-end "error".
    */
    
    #ifdef HAVE_CONFIG_H
    #include <config.h>
    #endif
    
    #include <unistd.h>
    #include <time.h>
    #include <stdlib.h>
    #include <string.h>
    #include <sys/types.h>
    #include <sys/socket.h>
    #include <sys/ioctl.h>
    #include <sys/time.h>
    #include <netinet/in.h>
    #include <netinet/tcp.h>
    #include <arpa/inet.h>
    #include <stdio.h>
    #include <netdb.h>
    #include <errno.h>
    #include <fcntl.h>
    #include <stdarg.h>
    #include <signal.h>
    
    #ifdef HAVE_MLOCKALL
    #include <sys/mman.h>
    #endif
    
    #include <assert.h>
    
    #include <exception>
    #include <algorithm>
    #include <vector>
    
    #include "types.h"
    #include "md5.h"
    #include "time64.h"
    #include "trim.inc"
    #include "rand.inc"
    #include "errno_holder.h"
    
    #ifndef SOL_TCP
    #define 	SOL_TCP   IPPROTO_TCP
    #endif
    
    static const int MaxNickLen = 150; 		// In bytes, not glyphs(worst-case max glyphs using UTF8 is probably around MaxNickLen / 5).
    static const int MaxClientsPerGame = 32;	// Maximum number of clients per game.
    static const int MaxControllersPerGame = 16;	// Maximum number of controllers per game.  Values higher than 16 are currently illegal.
    
    static const int MaxTotalControllersDataSize = 	512; // Maximum data size for all the controller data per game.
    						// Assume usable-worst-case 12 bytes per controller(in case of mouse, x delta, y delta, buttons), a maximum of 8
    						// controllers, would give us 96 bytes.
    						// We'll be extra-generous and set it to 512 bytes(in case anyone is goofy enough to try to use
    						// the Family Keyboard over netplay or something).
    
    // TODO: Ignoring incoming save state receives for a game if a previous save state transfer is in progress.  Or perhaps, if any sendq is >= MaxCommandPayload
    // for the game, ignore all large incoming data transfers.
    
    // TODO: Save state transfer throttling.
    
    // TODO: Text throttling.
    
    // TODO: Connection throttling.
    
    // TODO: Nick change throttling.
    
    
    // 0x40 ... 0x7F are used for payload-less netplay-specific commands(or when the payload can fit in the 4-byte length field).
    #define MDFNNPCMD_SETFPS        0x40 /* Sent from client to server ONLY(it should be ignored server-side if it's not from the first
                                            active player for the game). */
    #define MDFNNPCMD_NOP		0x41
    
    //
    #define MDFNNPCMD_CTRL_CHANGE     0x43  // Server->client, new local MPS mask as len.
    #define MDFNNPCMD_CTRL_CHANGE_ACK 0x44  // Client->server.  Acknowledge controller change.  Sent using old local data length, everything after
                                            // this should be new data size.
    
    //
    #define MDFNNPCMD_CTRLR_SWAP_NOTIF	0x68	// Server->Client
    
    //
    
    #define MDFNNPCMD_CTRLR_TAKE		0x70	// Client->server.  Take the specified controllers(from other clients)
    #define MDFNNPCMD_CTRLR_DROP		0x71	// Client->server.  Drop(relinquish) the specified controllers.
    #define MDFNNPCMD_CTRLR_DUPE		0x72	// Client->server.  Take the specified controllers(but let other clients still keep their control).
    #define MDFNNPCMD_CTRLR_SWAP		0x78	// Client->server.
    //
    
    #define MDFNNPCMD_REQUEST_LIST  0x7F    // client->server	(return list of players, and what controllers they control)
    
    #define MDFNNPCMD_LOADSTATE     0x80    // Client->server, and server->client
    #define	MDFNNPCMD_REQUEST_STATE 0x81	// Server->client
    //
    //
    //
    
    
    #define MDFNNPCMD_TEXT          0x90
    #define MDFNNPCMD_SERVERTEXT    0x93 // Server text message(informational), server->client (TODO;;; Restrict to protocol 3 and higher).
    #define MDFNNPCMD_ECHO          0x94 // Echos the string(no larger than 256 bytes) back to the client(used for pinging).
    #define MDFNNPCMD_INTEGRITY     0x95 // Send from a client to a server, then from the server to all clients.
    #define MDFNNPCMD_INTEGRITY_RES 0x96 // Integrity result, sent from the clients to the server.  The result should be no larger
                                         // than 256 bytes.
    #define MDFNNPCMD_SETNICK       0x98 /* Sent from client to server only. */
    #define MDFNNPCMD_PLAYERJOINED  0xA0    // Data:  <byte: bitmask, which inputs this player controls>
                                            //        <bytestream: nickname>
    #define MDFNNPCMD_PLAYERLEFT    0xA1    // Data: (see above)
    #define MDFNNPCMD_YOUJOINED     0xB0
    #define MDFNNPCMD_YOULEFT       0xB1
    
    #define MDFNNPCMD_NICKCHANGED   0xB8    // Sent from server to client
    
    #define MDFNNPCMD_LIST          0xC0 // Server->client
    
    #define MDFNNPCMD_SET_MEDIA             0xD0    // Client->server, and server->client
    
    #define MDFNNPCMD_CTRLR_TAKE_NOTIF	0xF0	// Server->client
    #define MDFNNPCMD_CTRLR_DROP_NOTIF	0xF1	// Server->client
    #define MDFNNPCMD_CTRLR_DUPE_NOTIF	0xF2	// Server->client
    
    #define MDFNNPCMD_QUIT          	0xFF // Client->server
    
    struct GameEntry;
    
    struct ClientEntry
    {
    	bool InUse;
    
    	uint32 id;	/* mainly for faster referencing when pointed to from the Games
    		           entries.
    			 */
    	char nickname[MaxNickLen + 1];
    
    	int TCPSocket;
    	GameEntry *game;	/* Pointer to the game this player is in. */
    	unsigned int game_csid;	// index into game->Clients[]
    
    	unsigned int local_players;	/* The number of local players the client requests.  Can be anywhere from 0 to UINT_MAX(granted we won't be able to GIVE a client anywhere near the number of controllers :b) */
    
    	int64 timeconnect_us;	// Time the client made the connection(used in connection/login idle timeout calculation)
    
    
    	uint8 pending_command;	/* Only for protocol >= 1 */
    	int total_controllers;  /* Total controllers for the emulated system. */
    	int controller_type[MaxControllersPerGame];	/* Controller type. */
    	int controller_data_size[MaxControllersPerGame]; /* Controller data size. */
    	int total_controllers_data_size; /* Data size of all controllers */
    	int local_controllers_data_size; /* Data size of local controllers */
    	int controller_data_offset[MaxControllersPerGame];
    
    	int protocol_version;	/* 0 = FCE Ultra + Mednafen < 0.5.0 - NOT SUPPORTED*/
    				/* 1 = Mednafen >= 0.5.0 
    				    Changes:  
    					1 extra byte in incoming data packet to denote command, instead
    					of using 0xFF in the first joystick byte as an escape.
    				   2 = Mednafen >= 0.7.0
    				    Changes:
    					Support for a wider variety of input devices.
    				*/
    	uint64 change_pending;
    
    	/* Variables to handle non-blocking TCP reads. */
    	uint8 *nbtcp;
    	uint32 nbtcphas, nbtcplen;
    	uint32 nbtcptype;
    	int64 last_receive_time;	// In microseconds
    
    	uint32 sendqcork;
    	uint8 *sendq;
    	uint32 sendqlen;
    	uint32 sendqalloced;
    
    	// Local controller buffer
    	uint8 local_controller_buffer[MaxTotalControllersDataSize];
    	char DisconnectReason[1024];
    };
    
    struct GameEntry
    {
    	/* No players, slated for deletion. */
    	bool Zombie;
    
    	/* Unique 128-bit identifier for this game session. */
            uint8 id[16];
    
    	/* The version of the protocol the clients in this game are communicating with. */
    	int ProtocolVersion;
    
    	/* Text string(not necessarily null-terminated) representing the emulator being used, and the version. */
    	uint8 EmulatorID[64];
    
    	/* Total controllers for this game. */
    	int TotalControllers;
    
    	/* Controller type for each controller. */
    	int ControllerType[MaxControllersPerGame];
    
    	/* Data size for each controller. */
    	int ControllerDataSize[MaxControllersPerGame];
    
    	/* Total data size for all controllers. */
            int TotalControllersDataSize;
    
    	/* Offset into joybuf for each controller. */
    	int ControllerDataOffset[MaxControllersPerGame];
    
    	/* Pointers to connected clients. */
            ClientEntry *Clients[MaxClientsPerGame];
    
    	uint32 ClientToController[MaxClientsPerGame];	// array of bitfields
    	uint32 ControllersInUse;			// bitfield.
    
    	int64 last_time;
    	uint32 fps;
    
            uint8 joybuf[MaxTotalControllersDataSize + 1]; /* X player data + 1 command byte8 */
    };
    
    struct CONFIG
    {
    	int MaxClients;		/* The maximum number of clients to allow. */
    				/* You should expect each client to use
    				   65-70Kbps(not including save state loads) while
    				   connected(and logged in).
    				*/
    
    	int ConnectTimeout;	/* How long to wait(in seconds) for the client to provide
    				   login data before disconnecting the client.
    				*/
    
    	int IdleTimeout;	/* If the data is not received from the client in this amount of time(in seconds), disconnect the client.
    				*/
    
    	int Port;		/* The port to listen on. */
    	uint8 *Password;	/* The server password. */
    
    	uint32 MinSendQSize;	//
    	uint32 MaxSendQSize;	// Maximum size the internal soft send queue is allowed to grow to.  If it grows
                                    // larger than this, the connection will be dropped.  Per client limit.
    	uint32 MaxCommandPayload;
    };
    
    CONFIG ServerConfig;
    
    int LoadConfig(char *fn)
    {
     FILE *fp;
    
     ServerConfig.Port = -1;
     ServerConfig.MaxClients = -1;
     ServerConfig.ConnectTimeout = -1;
    
     // Settings with defaults(can be missing from config file):
     ServerConfig.IdleTimeout = 30;
     ServerConfig.MinSendQSize = 262144;
     ServerConfig.MaxSendQSize = 8388608;
     ServerConfig.MaxCommandPayload = 5 * 1024 * 1024;
    
     if((fp=fopen(fn,"rb")))
     {
      char buf[512];
      char sname[sizeof(buf)];
      char args[sizeof(buf)];
    
      while(fgets(buf, sizeof(buf), fp) != NULL)
      {
       char *sc_pos;
    
       // Handle comments
       if((sc_pos = strchr(buf, ';')))
        *sc_pos = 0;
    
       trim(buf);
    
       if(buf[0] == 0)
        continue;
    
       sscanf(buf, "%s %[^\n]", sname, args);
       trim(args);
    
       if(!strncasecmp(sname, "maxclients",strlen("maxclients")))
        sscanf(args, "%d",&ServerConfig.MaxClients);
       else if(!strncasecmp(sname, "connecttimeout ",strlen("connecttimeout")))
        sscanf(args, "%d",&ServerConfig.ConnectTimeout);
       else if(!strncasecmp(sname, "port",strlen("port")))
       {
        sscanf(args, "%d",&ServerConfig.Port);
       }
       else if(!strncasecmp(sname, "password",strlen("password")))
       {
        if(args[0] != 0)
        {
         struct md5_context md5;
         ServerConfig.Password = (uint8 *)malloc(16);
         md5_starts(&md5);
         md5_update(&md5,(uint8*)args,strlen(args));
         md5_finish(&md5,ServerConfig.Password);
         puts("Password required to log in.");
        }
       }
       else if(!strncasecmp(sname, "idletimeout", strlen("idletimeout")))
        sscanf(args, "%d", &ServerConfig.IdleTimeout);
       else if(!strncasecmp(sname, "minsendqsize", strlen("minsendqsize")))
        sscanf(args, "%u", &ServerConfig.MinSendQSize);
       else if(!strncasecmp(sname, "maxsendqsize", strlen("maxsendqsize")))
        sscanf(args, "%u", &ServerConfig.MaxSendQSize);
       else if(!strncasecmp(sname, "maxcmdpayload", strlen("maxcmdpayload")))
        sscanf(args, "%u", &ServerConfig.MaxCommandPayload);
       else
       {
        printf("Unknown directive in configuration file: %s\n", sname);
        return(0);
       }
      }
     }
     else return(0);
    
     if(ServerConfig.Port == -1 || ServerConfig.MaxClients == -1 || ServerConfig.ConnectTimeout == -1)
     {
      puts("Incomplete configuration file");
      return(0);
     }
    
    
     printf("Server configuration:\n");
     printf("\tMaximum clients: %u\n", ServerConfig.MaxClients);
     printf("\tConnect timeout: %u seconds\n", ServerConfig.ConnectTimeout);
     printf("\tListen port: %u\n", ServerConfig.Port);
     printf("\tPassword: %s\n", ServerConfig.Password ? "(used)" : "(unused)");
     printf("\tIdle timeout: %u seconds\n", ServerConfig.IdleTimeout);
     printf("\tMinimum internal send queue size: %u bytes\n", ServerConfig.MinSendQSize);
     printf("\tMaximum internal send queue size: %u bytes\n", ServerConfig.MaxSendQSize);
     printf("\tMaximum command payload size: %u bytes\n", ServerConfig.MaxCommandPayload);
     printf("\t---------------------------------------\n");
     printf("\tRough worst-case internal queue memory usage: %.2f MiB\n", ((double)ServerConfig.MaxSendQSize + ServerConfig.MaxCommandPayload) * ServerConfig.MaxClients / 1024 / 1024);
     printf("\n");
    
     return(1);
    }
    
    static ClientEntry *AllClients;
    static GameEntry *Games;
    
    static void en32(uint8 *buf, uint32 morp)
    {
     buf[0]=morp;
     buf[1]=morp>>8;
     buf[2]=morp>>16;
     buf[3]=morp>>24;
    }
    
    static uint32 de32(const uint8 *morp)
    {
     return(morp[0]|(morp[1]<<8)|(morp[2]<<16)|(morp[3]<<24));
    }
    
    static void MakeSendTCP(ClientEntry *client, const uint8 *data, uint32 len);
    static void CleanText(char *text, unsigned len);
    static void CleanNick(char *nick);
    static bool NickUnique(ClientEntry *client, const char *);
    static void ParseAndSetNickname(ClientEntry *client, const uint8 *data, uint32 data_len, bool is_change = false);
    
    static bool AddClientToGame(ClientEntry *client, const uint8 id[16], const uint8 *emu_id);
    static void SendCommand(ClientEntry *client, uint8 cmd, const uint8 *data, uint32 len);
    static void SendDataToAllInGame(GameEntry *game, const uint8 *data, uint32 len);
    static void SendCommandToAllInGame(GameEntry *game, int cmd, const uint8 *data, uint32 len);
    static void TextToClient(ClientEntry *client, const char *fmt, ...) MDFN_FORMATSTR(printf, 2, 3);
    static void AnnouncePlayer(ClientEntry *client, ClientEntry *newclient);
    static void AnnouncePlayerLeft(ClientEntry *client, ClientEntry *newclient, uint32 mps);
    static void SendPlayerList(ClientEntry *client);
    static void DisconnectClient(ClientEntry *client, const char *format, ...) MDFN_FORMATSTR(printf, 2, 3);
    static void KillClient(ClientEntry *client);
    
    
    static uint32 EncodePlayerNumData(ClientEntry *client, uint8 *out_buffer, uint32 out_buffer_size, uint32 mps_override = 0, bool override_m = false);
    
    
    #define NBTCP_LOGINLEN		0x100
    #define NBTCP_LOGIN		0x200
    #define NBTCP_COMMANDLEN	0x300
    #define NBTCP_COMMAND		0x400
    
    #define NBTCP_UPDATEDATA	0x800
    
    static void StartNBTCPReceive(ClientEntry *client, uint32 type, uint32 len)
    {
     if(client->TCPSocket == -1) // Client is disconnected?
      return;
    
     if(!(client->nbtcp = (uint8 *)malloc(len)))
     {
      DisconnectClient(client, "malloc() failed: %s", ErrnoHolder(errno).StrError());
      return;
     }
    
     client->nbtcplen = len;
     client->nbtcphas = 0;
     client->nbtcptype = type;
    }
    
    static void RedoNBTCPReceive(ClientEntry *client)
    {
     if(client->TCPSocket == -1) // Client is disconnected?
      return;
    
     client->nbtcphas = 0;
    }
    
    static void EndNBTCPReceive(ClientEntry *client)
    {
     if(client->TCPSocket == -1) // Client is disconnected?
      return;
    
     free(client->nbtcp);
     client->nbtcplen = 0;
     client->nbtcphas = 0;
     client->nbtcptype = 0;
     client->nbtcp = 0;
    }
    
    static uint32 MakeMPS(ClientEntry *client)
    {
     return client->game->ClientToController[client->game_csid];
    }
    
    static void MakePRandNick(char *nick)
    {
     int i;
    
     for(i = 0; i < 8 && i < MaxNickLen; i++)
     {
      int tmp = rand_range(0, 47);
    
      if(tmp >= 24)
       nick[i] = 'a' + tmp - 24;
      else
       nick[i] = 'A' + tmp;
     }
    
     nick[i] = 0;
    }
    
    
    struct login_data_t
    {
     uint8 gameid[16];
     uint8 password[16];
    
     union
     {
      struct
      {
       uint8 protocol_version;
       uint8 total_controllers;
       uint8 padding0[2];
    
       uint8 pd_name_len[4];		// (protocol 2.  port device name len.  Deprecated, never implemented server-side) LSB first, 32-bit
    					// Protocol 3, length of emulator name and version string(up to 64 bytes) - (note that any non-0
    					// bytes < 0x20 in this string will be replaced by 0x20).
    
       uint8 padding1[8];
    
       uint8 controller_data_size[16];	// Only valid for protocol >= 2.
    
       uint8 padding3[16];
    
       uint8 controller_type[16];		// Protocol >= 3+
      };
    
      uint8 extra[64];
     };
    
     uint8 local_players;
     uint8 nickname[0];
    };
    
    
    #if 0
    // Ideas:
    //  use MSB of login len to denote Protocol 4 and higher
    //
    struct p4_login_data_t
    {
     uint8 magic[16]; 		// string 	- "MEDNAFEN_NETPLAY"
     uint8 protocol_version[4];	// uint32 	- Protocol version
     uint8 password[16];		//
     uint8 nickname[256];
    
     // Game configuration data.
     uint8 game_id[32];		//
     uint8 emu_id[64];		//		- Unique emulator identifier incorporating name and version.
     uint8 emu_config[2048];	//		- Free-form, emulator-specific emulator configuration data.
     uint8 total_controllers;
     uint8 controller_data_size[32];
    
     // Player-specific game configuration data.
     uint8 local_players;
    };
    #endif
    
    static inline void RecalcCInUse(GameEntry *game)
    {
     uint32 cinuse_mask = 0;
    
     for(int x = 0; x < MaxClientsPerGame; x++)
     {
      cinuse_mask |= game->ClientToController[x];
     }
    
     game->ControllersInUse = cinuse_mask;
    }
    
    
    /* Returns 1 if we are back to normal game mode, 0 if more data is yet to arrive. */
    static int CheckNBTCPReceive(ClientEntry *client)
    {
     if(client->TCPSocket == -1)	// Client is disconnected?
      return(0);
    
     if(!client->nbtcplen)
      return(0);			/* Should not happen. */
    
     while(1)
     {
      if(client->TCPSocket == -1)
       return(0);
    
      int l = recv(client->TCPSocket, client->nbtcp + client->nbtcphas, client->nbtcplen  - client->nbtcphas, 0);
    
      if(l == -1)
      {
       if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
        break;
    
       DisconnectClient(client, "recv() failed: %s", ErrnoHolder(errno).StrError());
       return(0);
      }
      else if(l == 0)
      {
       DisconnectClient(client, "Client gracefully closed connection.");
       return(0);
      }
    
      client->last_receive_time = MBL_Time64();
      client->nbtcphas += l;
    
      //printf("Read: %d, %04x, %d, %d\n",l,client->nbtcptype,client->nbtcphas, client->nbtcplen);
    
      /* We're all full.  Yippie. */
      if(client->nbtcphas == client->nbtcplen)
      {
       switch(client->nbtcptype & 0xF00)
       {
        case NBTCP_UPDATEDATA:
    			{
    			 if(client->nbtcp[0])
    			 {
    			  client->pending_command = client->nbtcp[0];
    			  EndNBTCPReceive(client);
    			  StartNBTCPReceive(client, NBTCP_COMMANDLEN, 4);
    			  return(1);
    			 }
    
    			 if(!client->change_pending)
    			 {
    			  memcpy(client->local_controller_buffer, &client->nbtcp[1], client->local_controllers_data_size);
    			 }
    			 RedoNBTCPReceive(client);
    			}
    			return(1);
    
        case NBTCP_COMMANDLEN:
    			{
    			 const uint8 cmd = client->pending_command;
    			 uint32 len = de32(client->nbtcp);
    
    			 if((cmd & 0x80) && len > ServerConfig.MaxCommandPayload)	/* Sanity check. */
    			 {
    			  DisconnectClient(client, "Tried to exceed maximum command payload by %u bytes!", len - ServerConfig.MaxCommandPayload);
    			  return(0);
    			 }
    
    			 if(!(cmd & 0x80))	// No payload, or payload is length data.
    		         {
    			  if(cmd == MDFNNPCMD_SETFPS)
                              {
    			   const uint32 fps = len;
    			   GameEntry *game = client->game;
    
    			   if(fps < (1U * 65536 * 256) || fps > (130U * 65536 * 256))
    			   {
                                TextToClient(client, "FPS out of range(range is from 1 to 130).");
                                EndNBTCPReceive(client);
                                StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			    return(1);
    			   }
    
                               game->fps = fps;
    
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
                              }
    			  else if(cmd == MDFNNPCMD_NOP)
    			  {
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			  else if(cmd == MDFNNPCMD_CTRLR_SWAP)
    			  {
    			   GameEntry *game = client->game;
    			   const uint8 sconts[2] = { (uint8)(len & 0xFF), (uint8)((len >> 8) & 0xFF) };
    			   bool had_any = false;
    
    			   for(int i = 0; i < 2; i++)
    			   {
    			    if(sconts[i] >= game->TotalControllers)
    			    {
                                 TextToClient(client, "Nonexistent controller(s) specified.");
                                 EndNBTCPReceive(client);
                                 StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			     return(1);
    			    }
    			   }
    
    			   for(int n = 0; n < MaxClientsPerGame; n++)
    			   {
    			    if(!game->Clients[n])
    			     continue;
    
    			    if((bool)(game->ClientToController[n] & (1U << sconts[0])) ^ (bool)(game->ClientToController[n] & (1U << sconts[1])))
    			    {
    			     had_any = true;
    
    			     if(game->ClientToController[n] & (1U << sconts[0]))
    			     {
    			      game->ClientToController[n] &= ~(1U << sconts[0]);
    			      game->ClientToController[n] |= (1U << sconts[1]);
    			     }
    			     else
    			     {
    			      game->ClientToController[n] &= ~(1U << sconts[1]);
    			      game->ClientToController[n] |= (1U << sconts[0]);
    			     }
    			     //printf("%d %08x\n", n, game->ClientToController[n]);
    			     game->Clients[n]->change_pending++;
    			     SendCommand(game->Clients[n], MDFNNPCMD_CTRL_CHANGE, NULL, game->ClientToController[n]);
    			    }
    			   }
    
    			   if(!had_any)
    			   {
                                TextToClient(client, "No client had any of the controller(s) you specified.");
                                EndNBTCPReceive(client);
                                StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			    return(1);
    		           }
    
    			   RecalcCInUse(game);
    
    			   SendCommandToAllInGame(game, MDFNNPCMD_CTRLR_SWAP_NOTIF, NULL, len & 0xFFFF);
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			  else if(cmd == MDFNNPCMD_CTRLR_TAKE || cmd == MDFNNPCMD_CTRLR_DROP || cmd == MDFNNPCMD_CTRLR_DUPE)
    			  {
    			   GameEntry *game = client->game;
    
    			   //printf("%02x %08x\n", cmd, len);
    			   if(len > ((1ULL << game->TotalControllers) - 1))
    			   {
                                TextToClient(client, "Nonexistent controller(s) specified.");
                                EndNBTCPReceive(client);
                                StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			    return(1);
    			   }
    
    			   if(cmd == MDFNNPCMD_CTRLR_DROP && ((len & game->ClientToController[client->game_csid]) != len))
    			   {
                                TextToClient(client, "Controller(s) you don't have control of specified.");
                                EndNBTCPReceive(client);
                                StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			    return(1);
    			   }
    
    			   client->change_pending++;
    
    			   if(cmd == MDFNNPCMD_CTRLR_DROP)
    			    game->ClientToController[client->game_csid] &= ~len;
    			   else
    			    game->ClientToController[client->game_csid] |= len;
    
                               SendCommand(client, MDFNNPCMD_CTRL_CHANGE, NULL, game->ClientToController[client->game_csid]);
    
    			   if(cmd == MDFNNPCMD_CTRLR_TAKE)
    			   {
    			    for(int n = 0; n < MaxClientsPerGame; n++)
    			    {
    			     if(game->Clients[n] && game->Clients[n] != client && (game->ClientToController[n] & len))
    			     {
    			      game->Clients[n]->change_pending++;
    			      game->ClientToController[n] &= ~len;
    			      SendCommand(game->Clients[n], MDFNNPCMD_CTRL_CHANGE, NULL, game->ClientToController[n]);
    			     }
    			    }
    			   }
    
    			   RecalcCInUse(game);
    
    			   {
    			    uint8 ntf_buf[4 + 4 + 4 + MaxNickLen];
    			    uint8 ntf_cmd;
    			    uint32 ntf_len = 0;
    
    			    if(cmd == MDFNNPCMD_CTRLR_TAKE)
    			     ntf_cmd = MDFNNPCMD_CTRLR_TAKE_NOTIF;
    			    else if(cmd == MDFNNPCMD_CTRLR_DROP)
    			     ntf_cmd = MDFNNPCMD_CTRLR_DROP_NOTIF;
    			    else
    			     ntf_cmd = MDFNNPCMD_CTRLR_DUPE_NOTIF;
    
    			    en32(&ntf_buf[ntf_len], len);
    			    ntf_len += 4;
    
    			    ntf_len += EncodePlayerNumData(client, ntf_buf + 4, sizeof(ntf_buf) - 4);
    			    SendCommandToAllInGame(client->game, ntf_cmd, ntf_buf, ntf_len);
    			   }
    
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			  else if(cmd == MDFNNPCMD_CTRL_CHANGE_ACK)
    			  {
                               // Calculate the size of this client's local controllers data size.
                               uint32 localmask = MakeMPS(client);
                               client->local_controllers_data_size = 0;
    
    			   if(client->change_pending > 0)
    			    client->change_pending--;
    			   else
    			    puts("BUG? :(");
    
    			   if(client->change_pending)	// Icky way to handle change_pending > 1.  (Caution: might have security implications if we change too much stuff around)
    			    localmask = len;
    
                               for(int x = 0; x < client->total_controllers; x++)
                                if(localmask & (1U << x))
                                 client->local_controllers_data_size += client->controller_data_size[x];
    
    			   if(!client->change_pending)	// Clear out any leftover (potential garbage) that would be used now that client->change_pending == 0
    			    memset(client->local_controller_buffer, 0, client->local_controllers_data_size);
    
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			  else if(cmd == MDFNNPCMD_REQUEST_LIST)
    			  {
    			   SendPlayerList(client);
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    		          else if(cmd < 0x40)	// Emulator commands, like power, reset, etc.
    			  {
    			   SendCommandToAllInGame(client->game, cmd, 0, 0);
    			   EndNBTCPReceive(client);
    			   StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			  else
    			  {
                               TextToClient(client, "Unknown command: 0x%02x", cmd);
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			 }
    			 else // Everything else.
    			 {
    			  if(cmd == MDFNNPCMD_INTEGRITY)
                              {
                               SendCommandToAllInGame(client->game, MDFNNPCMD_INTEGRITY, NULL, 0);
                               EndNBTCPReceive(client);
                               StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
                              }
    			  else
    			  {
                               EndNBTCPReceive(client);
    
    			   if(len)
    			    StartNBTCPReceive(client, NBTCP_COMMAND | cmd,len);
    			   else	/* Woops.  Client probably tried to send a text message of 0 length. Or maybe a 0-length cheat file?  Better be safe! */
    			    StartNBTCPReceive(client, NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			  }
    			 }
    			}
    			return(1);
    
        case NBTCP_COMMAND: 
    			{
    			 uint32 len = client->nbtcplen;
    			 uint32 tocmd = client->nbtcptype & 0xFF;
    
                             if(tocmd == MDFNNPCMD_INTEGRITY_RES)
                             {
                              printf("%s\n", md5_asciistr(client->nbtcp));
                             }
                             else if(tocmd == MDFNNPCMD_ECHO)
                             {
                              uint32 newlen = len;
    
                              if(newlen > 256)
                               newlen = 256;
    
                              SendCommand(client, tocmd, client->nbtcp, newlen);
                             }
    			 else if(tocmd == MDFNNPCMD_SETNICK)
    			 {
    			  ParseAndSetNickname(client, client->nbtcp, len, true);
    			 }
    			 else if(tocmd == MDFNNPCMD_TEXT)
    			 {
                              const uint32 nicklen = strlen(client->nickname);
                              uint8 mewmewbuf[4 + MaxNickLen];
    
                              en32(mewmewbuf, nicklen);
                              memcpy(mewmewbuf + 4, client->nickname, nicklen);
    
                              SendCommandToAllInGame(client->game, tocmd, NULL, 4 + nicklen + len);
    			  SendDataToAllInGame(client->game, mewmewbuf, 4 + nicklen);
    			  SendDataToAllInGame(client->game, client->nbtcp, len);
    			 }
    			 else if(tocmd == MDFNNPCMD_SET_MEDIA)
    			 {
    			  if(len != 16)
    			  {
    			   DisconnectClient(client, "SET_MEDIA command length is wrong.");
    			   return 0;
    			  }
    
    			  SendCommandToAllInGame(client->game, tocmd, client->nbtcp, len);
    			 }
    			 else if(tocmd == MDFNNPCMD_QUIT)
    		         {
    			  if(len)
    			  {
    			   char quit_message[1024 + 1];
    			   uint32 quit_len = std::min<uint32>(1024, len);
    
    			   memcpy(quit_message, client->nbtcp, quit_len);
    			   quit_message[quit_len] = 0;
    
    			   CleanText(quit_message, quit_len);
                               DisconnectClient(client, "Quit: %s", quit_message);
    			  }
    			  else
    			   DisconnectClient(client, "Quit");
    
    			  return(0);
    			 }
    			 else if(tocmd == MDFNNPCMD_LOADSTATE)
    			  SendCommandToAllInGame(client->game, tocmd, client->nbtcp, len);
    
                             EndNBTCPReceive(client);
    			 StartNBTCPReceive(client,NBTCP_UPDATEDATA, client->local_controllers_data_size + 1);
    			}
    			return(1);
    
        case NBTCP_LOGINLEN:
    			{
    			 uint32 len = de32(client->nbtcp);
    
    			 // + 8192 because the client wouldn't know our maximum nickname length setting on the server beforehand, and to handle
    			 // versions of Mednafen that send garbage(not technically garbage, but never really implemented on the server side, and a poor
    			 // solution to the problem of input device selection anyway, so GARBAGE).
    			 if(len > (sizeof(login_data_t) + MaxNickLen + 8192) || len < sizeof(login_data_t))
    			 {
    			  DisconnectClient(client, "Login len(%u) out of range.", len);
    			  return(0);
    			 }
    
    			 EndNBTCPReceive(client);
    			 StartNBTCPReceive(client, NBTCP_LOGIN, len);
    			}
    			return(1);
    
        case NBTCP_LOGIN:
    			{
    			 const login_data_t *ld = (const login_data_t *)client->nbtcp;
    			 const uint32 len = client->nbtcplen;
    			 const uint32 pd_name_len = de32(ld->pd_name_len);
    			 uint32 nickname_len = 0;
    			 uint8 emu_id[64];
    
    			 if(pd_name_len < (len - sizeof(login_data_t)))
    			  nickname_len = len - (sizeof(login_data_t) + pd_name_len);
    
    			 client->local_players = ld->local_players;
    			 client->protocol_version = ld->protocol_version;
    			 client->total_controllers = ld->total_controllers;
    
    			 // Don't even send error messages for these, because the
    			 // message sending code relies on the parameters that are invalid x_x
                             if(client->protocol_version < 3 && client->total_controllers > 8)
                             {
    			  DisconnectClient(client,"That number of controllers(%u) isn't supported with that protocol version.", client->total_controllers);
    			  return(0);
                             }
    
                             if(client->total_controllers > MaxControllersPerGame)
                             {
    			  DisconnectClient(client, "That number of controllers(%u) isn't supported.", client->total_controllers);
    			  return(0);
                             }
    
    			 if(!client->protocol_version)
    			 {
    			  DisconnectClient(client, "Protocol 0 not supported.");
    			  return(0);
    		         }
    
    			 client->sendqcork = 0;
    			 client->sendqalloced = ServerConfig.MinSendQSize;
    			 client->sendqlen = 0;
    			 client->sendq = (uint8 *)calloc(client->sendqalloced, 1);
    
    			 if(!client->controller_data_size || !client->controller_data_offset || !client->sendq)
    			 {
    			  DisconnectClient(client, "Memory allocation failure!");
    			  return(0);
    			 }
    
    			 if(client->protocol_version < 2)
    			 {
    			  for(int x = 0; x < client->total_controllers; x++)
    			  {
    			   client->controller_type[x] = 1;
    			   client->controller_data_size[x] = 1;
    			   client->controller_data_offset[x] = x;
    			  }
    			  client->total_controllers_data_size = client->total_controllers;
    			 }
    		 	 else
    			 {
    			  int zeindex = 0;
    
    			  for(int x = 0; x < client->total_controllers; x++)
    			  {
    			   client->controller_type[x] = ld->controller_type[x];
    			   client->controller_data_size[x] = ld->controller_data_size[x];
    			   client->controller_data_offset[x] = zeindex;
    			   zeindex += client->controller_data_size[x];
    			  }
    			  client->total_controllers_data_size = zeindex;
    		 	 }
    
    			 // Hack so we can always be able to encode our command length...
    			 if(client->total_controllers_data_size < 4) 
    			  client->total_controllers_data_size = 4;
    
    			 if(client->total_controllers_data_size > MaxTotalControllersDataSize)
    			 {
    			  DisconnectClient(client, "Exceeded MaxTotalControllersDataSize");
    			  return(0);
    			 }
    
    			 client->local_controllers_data_size = 0;
    
                             if(ServerConfig.Password)
    			 {
                              if(memcmp(ServerConfig.Password, ld->password, 16))
                              {
                               TextToClient(client, "Invalid server password.");
    			   DisconnectClient(client, "Invalid server password.");
    			   return(0);
                              }
    			 }
    
    			 memset(emu_id, 0, 64);
    
    			 if(client->protocol_version == 3 && pd_name_len && pd_name_len <= 64 && ((int64_t)sizeof(login_data_t) + nickname_len + pd_name_len) <= len)
    			 {
    			  strncpy((char *)emu_id, (const char *)ld->nickname + nickname_len, pd_name_len);
    
    			  for(int i = 0; i < 64; i++)
    			  {
    			   if(emu_id[i] > 0 && emu_id[i] < 0x20)
    			    emu_id[i] = 0x20;
    			  }
    			 }
    
    			 // Needs to be before nickname set, but we'll fix this eventually.
                             if(!AddClientToGame(client, ld->gameid, emu_id))
    			  return(0);
    
                             ParseAndSetNickname(client, ld->nickname, nickname_len);
    
    			 printf("Client(protocol %u) %u assigned to game %u as player mask 0x%08x <%s> - EmuID=%.64s\n", client->protocol_version, client->id, (int)(client->game - Games), MakeMPS(client), client->nickname, emu_id);
    
    			 // First, announce any existing clients to the new client.
    			 SendPlayerList(client);
    
    			 // Now, announce this new client to other players.
    			 // (Do this in a separate loop so we need less convoluted error handling and to avoid out-of-order messages)
    			 GameEntry *tg = client->game;
                             for(int x = 0; x < MaxClientsPerGame; x++)
                             {
                              if(tg->Clients[x])
                              {
                               if(tg->Clients[x] != client)
                               {
                                AnnouncePlayer(tg->Clients[x], client);
                               }
                              }
                             }
    
    			 // Calculate the size of this client's local controllers data size.
    			 {
    			  uint32 localmask = MakeMPS(client);
    			  client->local_controllers_data_size = 0;
    
    			  for(int x = 0; x < client->total_controllers; x++)
    			   if(localmask & (1U << x))
    			    client->local_controllers_data_size += client->controller_data_size[x];
    			 }
    			} 
    			EndNBTCPReceive(client);
    			StartNBTCPReceive(client,NBTCP_UPDATEDATA,client->local_controllers_data_size + 1);
    			return(1);
       }
      }
     }
     return(0);
    }
    
    struct ListenSocketDef
    {
     int fd;
     int family;
    };
    
    static std::vector<ListenSocketDef> ListenSockets;
    
    static void CleanText(char *text, unsigned len)
    {
     for(unsigned i = 0; i < len; i++)
     {
      if((unsigned char)text[i] < 0x20)
       text[i] = 0x20;
     }
    }
    
    // Remove any control characters or reserved characters from the nickname.
    static void CleanNick(char *nick)
    {
     for(unsigned int x = 0; x < strlen(nick); x++)
      if(nick[x] == '<' || nick[x] == '>' || nick[x] == '*' || ((unsigned char)nick[x] < 0x20))
       nick[x] = 0;
    }
    
    static bool NickUnique(ClientEntry *client, const char *newnick)
    {
     GameEntry *game = client->game;
    
     for(int x = 0; x < MaxClientsPerGame; x++)
      if(game->Clients[x] && client != game->Clients[x])
       if(!strcasecmp(newnick, game->Clients[x]->nickname))
        return(0);
     return(1);
    }
    
    static void ParseAndSetNickname(ClientEntry *client, const uint8 *data, uint32 data_len, bool is_change)        // is_change is false on initial login, true if trying to change nickname after login.
    {
     char newnick[MaxNickLen + 1];
    
     if(data_len > (uint32)MaxNickLen)
      data_len = MaxNickLen;
    
     newnick[data_len] = 0;
     memcpy(newnick, data, data_len);
    
     CleanNick(newnick);
    
     if(!strlen(newnick) || !NickUnique(client, newnick))
     {
      do
      {
       MakePRandNick(newnick);
      } while(!NickUnique(client, newnick));
     }
    
     if(!is_change) // Login nickname set
     {
      strcpy(client->nickname, newnick);
     }
     else
     {
      uint8 ncdata[MaxNickLen + 1 + MaxNickLen + 1];
      uint32 ncdata_len;
    
      ncdata_len = strlen(client->nickname) + 1 + strlen(newnick) + 1;
    
      memcpy(ncdata, client->nickname, strlen(client->nickname));
      ncdata[strlen(client->nickname)] = '\n';
      strcpy((char*)ncdata + strlen(client->nickname) + 1, newnick);
    
      strcpy(client->nickname, newnick);
    
      SendCommandToAllInGame(client->game, MDFNNPCMD_NICKCHANGED, ncdata, ncdata_len);
     }
    }
    
    static void MakeSendTCP(ClientEntry *client, const uint8 *data, uint32 len)
    {
     if(client->TCPSocket == -1) // Client is disconnected?
      return;
    
     //if(client->sendqcork > 0)
     // puts("CORKED");
    
     // If there's no data in our software sendq, try sending this data without using the soft sendq.
     if(!client->sendqlen)
     {
      int32 sent;
    
      if(client->sendqcork > 0)
       sent = 0;
      else 
       sent = send(client->TCPSocket, data, len, 0);
    
      if(sent < 0)
      {
       if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
        sent = 0;
       else
       {
        DisconnectClient(client, "send() failed: %s", ErrnoHolder(errno).StrError());
        return;
       }
      }
    
      assert((uint32)sent <= len);
    
      // ...and adjust the data pointer and length appropriately.
      data += sent;
      len -= sent;
    
      // If we sent all the data, no need to go through the software sendq code at all~
      if(!len)
       return;
     }
    
     assert(client->sendq);
     if((client->sendqlen + len) > client->sendqalloced)
     {
      uint32 new_alloc_size;
    
      new_alloc_size = client->sendqlen + len;
    
      if(new_alloc_size > ServerConfig.MaxSendQSize)
      {
       DisconnectClient(client, "Exceeded MaxSendQSize by %u bytes", ServerConfig.MaxSendQSize - new_alloc_size);
       return;
      }
    
      new_alloc_size += new_alloc_size >> 1;
      if(new_alloc_size > ServerConfig.MaxSendQSize)
       new_alloc_size = ServerConfig.MaxSendQSize;
    
      uint8 *new_sendq = (uint8 *)realloc(client->sendq, new_alloc_size);
      if(!new_sendq && new_alloc_size)
      {
       DisconnectClient(client, "realloc() failed: %s", ErrnoHolder(errno).StrError());
       return;
      }
      client->sendq = new_sendq;
      client->sendqalloced = new_alloc_size;
     }
    
     //puts("SendQ path");
    
     // Append new data to the end of the sendq
     memcpy(client->sendq + client->sendqlen, data, len);
     client->sendqlen += len;
    
     // Now try to send all the data in the sendq
     int32 sent;
    
     if(client->sendqcork > 0)
      sent = 0;
     else
      sent = send(client->TCPSocket, client->sendq, client->sendqlen, 0);
    
     if(sent < 0) 
     {
      if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
       sent = 0;
      else
      {
       DisconnectClient(client, "send() failed: %s", ErrnoHolder(errno).StrError());
       return;
      }
     }
    
     if(sent)
     {
      uint32 new_alloc_size;
    
      client->sendqlen -= sent;
      memmove(client->sendq, client->sendq + sent, client->sendqlen);
    
      new_alloc_size = client->sendqlen;
      if(new_alloc_size < ServerConfig.MinSendQSize)
       new_alloc_size = ServerConfig.MinSendQSize;
    
      uint8 *new_sendq = (uint8 *)realloc(client->sendq, new_alloc_size);
      if(!new_sendq && new_alloc_size)
      {
       DisconnectClient(client, "realloc() failed: %s", ErrnoHolder(errno).StrError());
       return;
      }
      client->sendq = new_sendq;
      client->sendqalloced = new_alloc_size;
     }
    }
    
    static void SendCommand(ClientEntry *client, uint8 cmd, const uint8 *data, uint32 len)
    {
     uint8 poo[MaxTotalControllersDataSize + 1];
    
     memset(poo, 0, client->total_controllers_data_size);
    
     poo[client->total_controllers_data_size] = cmd;
    
     en32(poo, len);
     MakeSendTCP(client, poo, client->total_controllers_data_size + 1);
    
     if((cmd & 0x80) && data != NULL)
      MakeSendTCP(client, data, len);
    }
    
    static void SendDataToAllInGame(GameEntry *game, const uint8 *data, uint32 len)
    {
     for(int x = 0; x < MaxClientsPerGame; x++)
     {
      if(!game->Clients[x])
       continue;
    
      MakeSendTCP(game->Clients[x], data, len);
     }
    }
    
    
    static void SendCommandToAllInGame(GameEntry *game, int cmd, const uint8 *data, uint32 len)
    {
     for(int x = 0; x < MaxClientsPerGame; x++)
     {
      if(!game->Clients[x])
       continue;
    
      SendCommand(game->Clients[x], cmd, data, len);
     }
    }
    
    static void TextToClient(ClientEntry *client, const char *fmt, ...)
    {
     uint8 moo[4 + 1024]; // nick len(=0) and message
     va_list ap;
    
     va_start(ap,fmt);
     vsnprintf((char*)moo + 4, 1024, fmt, ap);
     va_end(ap);
    
     en32(moo, 0); // nick len of 0 to signify this is a server message
     SendCommand(client, MDFNNPCMD_TEXT, moo, 4 + strlen((char*)moo + 4));
    }
    
    // out_buffer should have at least 4 + 4 + MaxNickLen bytes size
    static uint32 EncodePlayerNumData(ClientEntry *client, uint8 *out_buffer, uint32 out_buffer_size, uint32 mps_override, bool override_m)
    {
     uint32 mps;
     uint32 datalen;
    
     if(override_m)
     {
      mps = mps_override;
     }
     else
      mps = MakeMPS(client);
    
     if(client->game->ProtocolVersion >= 3)
     {
      datalen = 4 + 4 + strlen(client->nickname);
    
      assert(datalen <= out_buffer_size);
    
      en32(&out_buffer[0], mps);
      en32(&out_buffer[4], 0);
    
      memcpy(out_buffer + 8, client->nickname, strlen(client->nickname));
     }
     else
     {
      datalen = 1 + 1 + strlen(client->nickname);
    
      assert(datalen <= out_buffer_size);
    
      out_buffer[0] = mps;
      out_buffer[1] = 0;
    
      memcpy(out_buffer + 2, client->nickname, strlen(client->nickname));
     }
    
     return(datalen);
    }
    
    static void AnnouncePlayer(ClientEntry *client, ClientEntry *newclient)
    {
     uint8 data[4 + 4 + MaxNickLen];
     uint32 datalen;
    
     datalen = EncodePlayerNumData(newclient, data, sizeof(data));
    
     if(client == newclient)
      SendCommand(client, MDFNNPCMD_YOUJOINED, data, datalen);
     else
      SendCommand(client, MDFNNPCMD_PLAYERJOINED, data, datalen);
    }
    
    static void AnnouncePlayerLeft(ClientEntry *client, ClientEntry *newclient, uint32 mps)
    {
     uint8 data[4 + 4 + MaxNickLen];
     uint32 datalen;
    
     datalen = EncodePlayerNumData(newclient, data, sizeof(data), mps, true);
    
     if(client == newclient)
      SendCommand(client, MDFNNPCMD_YOULEFT, data, datalen);
     else
      SendCommand(client, MDFNNPCMD_PLAYERLEFT, data, datalen);
    }
    
    static void SendPlayerList(ClientEntry *client)
    {
     GameEntry *tg = client->game;
    
     //if(client->protocol >= 4)
     //{
     //
     //}
     //else
     {
      for(int x = 0; x < MaxClientsPerGame; x++)
      {
       if(tg->Clients[x])
       {
        AnnouncePlayer(client, tg->Clients[x]);
       }
      }
     }
    }
    
    // Disconnect client, and destroy some of the connection structures, but leave everything else intact.
    static void DisconnectClient(ClientEntry *client, const char *format, ...)
    {
     if(client->TCPSocket != -1)
     {
      time_t curtime = time(NULL);
    
      if(format == NULL)
      {
       client->DisconnectReason[0] = 0;
      }
      else
      {
       va_list ap;
       va_start(ap, format);
       vsnprintf(client->DisconnectReason, sizeof(client->DisconnectReason), format, ap);
       va_end(ap);
      }
    
      if(client->game)
       printf("Client %u, <%s> disconnected from game %u for reason <%s> on %s", client->id, client->nickname, (unsigned)(client->game - Games), client->DisconnectReason, ctime(&curtime));
      else
       printf("Unassigned client %u disconnected for reason <%s> on %s", client->id, client->DisconnectReason, ctime(&curtime));
     }
    
     if(client->sendq)
     {
      free(client->sendq);
      client->sendq = NULL;
     }
    
     if(client->nbtcp)
     {
      free(client->nbtcp);
      client->nbtcp = NULL;
     }
    
     if(client->TCPSocket != -1)
     {
      close(client->TCPSocket);
      client->TCPSocket = -1;
     }
    
     client->nbtcphas = 0;
     client->nbtcplen = 0;
     client->nbtcptype = 0;
    
     client->sendqcork = 0;
     client->sendqlen = 0;
     client->sendqalloced = 0;
    }
    
    static void KillClient(ClientEntry *client)
    {
     GameEntry *game;
    
     if(!client)
     {
      puts("Erhm, attempt to kill NULL client o_O");
      return;
     }
    
     game = client->game;
     if(game)
     {
      //
      // Calculate what the player controlled before we destroy that information.
      //
      uint32 mps;
    
      mps = MakeMPS(client);
    
      // Remove the client from the game's player list BEFORE we do an announcement that the client has left.  Which should be obvious, but we used to do it backwards
      // and wonkiness randomly ensued. :/
      //
      // And uncomment out this code immediately below if we ever have a "nice" quit handshake rather than a hard disconnect.
      // AnnouncePlayerLeft(client, client, mps, mergedto);
    
      for(int x = 0; x < MaxClientsPerGame; x++)
      {
       if(game->Clients[x] && (game->Clients[x] == client))
       {
        game->Clients[x] = NULL;
        game->ClientToController[x] = 0;
        break;
       }
      }
    
      RecalcCInUse(game); // Recalc ControllersInUse bitfield.
    
      // Now announce to the remaining players that the client has left.
      for(int x = 0; x < MaxClientsPerGame; x++)
      {
       if(!game->Clients[x])
        continue;
    
       AnnouncePlayerLeft(game->Clients[x], client, mps);
      }
    
      if(!game->Zombie)	// Make sure the game is not already a zombie, since we could have zombified it in a recursive call to KillClient() above.
      {
       bool AnyClientsLeft = false;		// Any clients left connected to this game?
    
       for(int x = 0; x < MaxClientsPerGame && !AnyClientsLeft; x++)
        if(game->Clients[x])
         AnyClientsLeft = true;
    
       if(!AnyClientsLeft)
       {
        game->Zombie = true;
       }
      }
     }
    
     DisconnectClient(client, NULL);
    
     memset(client, 0, sizeof(ClientEntry));
     client->TCPSocket = -1;
    }
    
    static bool AddClientToGame(ClientEntry *client, const uint8 id[16], const uint8 *emu_id)
    {
     GameEntry *game,*fegame;
     game = NULL;
    
     fegame = NULL;
    
     /* First, find an available game. */
     for(int wg = 0; wg < ServerConfig.MaxClients; wg++)
     {
      if(!Games[wg].TotalControllers && !fegame)
      {
       if(!fegame)
        fegame=&Games[wg];
      }
      else if(Games[wg].TotalControllers && !memcmp(Games[wg].id,id,16)) /* A match was found! */
      {
       game = &Games[wg];
       break;
      }
     }
    
     if(game)
     {
      if(game->ProtocolVersion != client->protocol_version)
      {
       TextToClient(client, "Protocol version mismatch.  Game: %u, You: %u", game->ProtocolVersion, client->protocol_version);
       DisconnectClient(client, "Protocol version mismatch.  Game: %u, You: %u", game->ProtocolVersion, client->protocol_version);
       return(0);
      }
    
      if(memcmp(game->EmulatorID, emu_id, 64))
      {
       TextToClient(client, "Emulator (version) mismatch.  Game: %.64s, You: %.64s", game->EmulatorID, emu_id);
       DisconnectClient(client, "Emulator (version) mismatch.  Game: %.64s, You: %.64s", game->EmulatorID, emu_id);
       return(0);
      }
    
      if(game->TotalControllers != client->total_controllers)
      {
       TextToClient(client, "Number of controllers mismatch.  Game: %u, You: %u", game->TotalControllers, client->total_controllers);
       DisconnectClient(client, "Number of controllers mismatch.  Game: %u, You: %u", game->TotalControllers, client->total_controllers);
       return(0);
      }
    
      for(int x = 0; x < client->total_controllers; x++)
      {
       if(game->ControllerType[x] != client->controller_type[x])
       {
        TextToClient(client, "Controller type mismatch for controller %u.  Game: %u, You: %u", x + 1, game->ControllerType[x], client->controller_type[x]);
        DisconnectClient(client, "Controller type mismatch for controller %u.  Game: %u, You: %u", x + 1, game->ControllerType[x], client->controller_type[x]);
        return(0);
       }
    
       if(game->ControllerDataSize[x] != client->controller_data_size[x])
       {
        TextToClient(client, "Controller data size mismatch for controller %u.  Game: %u, You: %u", x + 1, game->ControllerDataSize[x], client->controller_data_size[x]);
        DisconnectClient(client, "Controller data size mismatch for controller %u.  Game: %u, You: %u", x + 1, game->ControllerDataSize[x], client->controller_data_size[x]);
        return(0);
       }
      }
    
      if(game->TotalControllersDataSize != client->total_controllers_data_size)
      {
       // This shouldn't happen...
       TextToClient(client, "Controllers total data size mismatch.  Game: %u, You: %u", game->TotalControllersDataSize, client->total_controllers_data_size);
       DisconnectClient(client, "Controllers total data size mismatch.  Game: %u, You: %u", game->TotalControllersDataSize, client->total_controllers_data_size);
       return(0);
      }
     }
    
     if(!game) /* Hmm, no game found.  Guess we'll have to create one. */
     {
      time_t curtime = time(NULL);
    
      game=fegame;
      printf("Game %u added on %s", (int)(game-Games), ctime(&curtime));
      memset(game, 0, sizeof(GameEntry));
    
      game->Zombie = false;
      game->TotalControllers = client->total_controllers; // total_controllers is validated in the user login code
      game->TotalControllersDataSize = client->total_controllers_data_size;
    
      for(int x = 0; x < game->TotalControllers; x++)
      {
       game->ControllerType[x] = client->controller_type[x];
       game->ControllerDataSize[x] = client->controller_data_size[x];
       game->ControllerDataOffset[x] = client->controller_data_offset[x];
      }
    
      game->fps = 1008307711; // NES NTSC FPS magical number from beyond the moon, the default.
      game->last_time = MBL_Time64();
      game->ProtocolVersion = client->protocol_version;
      memcpy(game->EmulatorID, emu_id, 64);
      memcpy(game->id, id, 16);
     }
    
     for(int n = 0; n < MaxClientsPerGame; n++)
     {
      if(game->Clients[n])
      {
       SendCommand(game->Clients[n], MDFNNPCMD_REQUEST_STATE, NULL, 0);
       break;
      }
     }
    
     // First, find an empty client slot.
     int client_slot = -1;
     for(int n = 0; n < MaxClientsPerGame; n++)
     {
      if(!game->Clients[n])
      {
       game->Clients[n] = client;
       client_slot = n;
       break;
      }
     }
    
     // Game is full.
     if(client_slot < 0)
     {
      TextToClient(client, "Sorry, game is full.");
      DisconnectClient(client, "Game is full.");
      return(0);
     }
    
     client->game = game;
     client->game_csid = client_slot;
    
     {
      game->ClientToController[client_slot] = 0;
      unsigned int lc_count = client->local_players;
      for(int n = 0; n < game->TotalControllers && lc_count; n++)
      {
       if(!(game->ControllersInUse & (1U << n)))
       {
        game->ClientToController[client_slot] |= (1U << n);
        game->ControllersInUse |= (1U << n); 
        lc_count--;
       }
      }
     }
    
     // Zap the game with the de-zombification ray since it has a guaranteed juicy player now.
     game->Zombie = false;
     return(true);
    }
    
    int main(int argc, char *argv[])
    {
     union
     {
      struct sockaddr_in6 sockin6;
      struct sockaddr_in sockin;
      struct sockaddr saddr;
     };
    
     SeedRand(time(NULL));
    
     if(argc < 2)
     {
      if(strchr(argv[0],' '))
       printf("Usage: \"%s\" <configfile>\n",argv[0]);
      else
       printf("Usage: %s <configfile>\n",argv[0]);
      exit(-1);
     }
    
     printf("Starting Mednafen-Server %s\n\n", VERSION);
    
     if(!LoadConfig(argv[1]))
     {
      puts("Error loading configuration file");
      exit(-1);
     }
    
    #ifdef SIGPIPE
     signal(SIGPIPE, SIG_IGN);
    #endif
    
     Games = (GameEntry *)calloc(ServerConfig.MaxClients, sizeof(GameEntry));
     AllClients = (ClientEntry *)calloc(ServerConfig.MaxClients, sizeof(ClientEntry));
    
     for(int x = 0; x < ServerConfig.MaxClients; x++)
     {
      AllClients[x].TCPSocket = -1;
     }
    
     const int family_try[2] = { AF_INET, AF_INET6 };
     const char *family_try_name[2] = { "IPv4", "IPv6" };
    
     for(unsigned fti = 0; fti < 2; fti++)
     {
      ListenSocketDef lsd;
    
      lsd.family = family_try[fti];
    
      /* Create the socket. */
      if(-1 == (lsd.fd = socket(family_try[fti], SOCK_STREAM, 0)))
      {
       ErrnoHolder ene(errno);
    
       printf("Error creating %s socket: %s\n", family_try_name[fti], ene.StrError());
       continue;
      }
    
      /* Disallow IPv4 connections on an IPv6 socket. */
      if(lsd.family == AF_INET6)
      {
    #ifdef IPV6_V6ONLY
       int v6only_opt = 1;
       if(setsockopt(lsd.fd, IPPROTO_IPV6, IPV6_V6ONLY, &v6only_opt, sizeof(int)))
       {
        ErrnoHolder ene(errno);
    
        printf("Warning: Failed to set IPv6 socket to disallow IPv4 connections: %s\n", ene.StrError());
       }
    #endif
      }
    
      /* Bind */
      socklen_t saddr_len;
      if(lsd.family == AF_INET6)
      {
       saddr_len = sizeof(sockin6);
    
       memset(&sockin6, 0, sizeof(sockin6));
       memcpy(&sockin6.sin6_addr, &in6addr_any, sizeof(in6_addr));	// = IN6ADDR_ANY_INIT;
       sockin6.sin6_family = AF_INET6;
       sockin6.sin6_port = htons(ServerConfig.Port);
      }
      else
      {
       saddr_len = sizeof(sockin);
    
       memset(&sockin, 0, sizeof(sockin));
       sockin.sin_addr.s_addr = INADDR_ANY;
       sockin.sin_family = AF_INET;
       sockin.sin_port = htons(ServerConfig.Port);
      }
    
      if(bind(lsd.fd, &saddr, saddr_len))
      {
       ErrnoHolder ene(errno);
       printf("Bind error: %s\n", ene.StrError());
       exit(-1);
      }
    
      /* Set send buffer size to 262,144 bytes. */
      int sndbufsize = 262144;
      if(setsockopt(lsd.fd, SOL_SOCKET, SO_SNDBUF, &sndbufsize, sizeof(int)))
      {
       // Not a fatal error.
       ErrnoHolder ene(errno);
    
       printf("Warning: Send buffer size set failed: %s", ene.StrError());
      }
    
      int tcpopt = 1;
      if(setsockopt(lsd.fd, SOL_TCP, TCP_NODELAY, &tcpopt, sizeof(int)))
      {
       ErrnoHolder ene(errno);
    
       printf("Enabling option TCP_NODELAY failed: %s\n", ene.StrError());
       exit(-1);
      }
    
      if(listen(lsd.fd, 16))
      {
       ErrnoHolder ene(errno);
    
       printf("Listen error: %s\n", ene.StrError());
       exit(-1);
      }
    
      { 
       char tbuf[256];
    
       if(lsd.family == AF_INET6)
        inet_ntop(lsd.family, &sockin6.sin6_addr, tbuf, sizeof(tbuf));
       else
        inet_ntop(lsd.family, &sockin.sin_addr, tbuf, sizeof(tbuf));
    
       printf("Listening on %s %u\n", tbuf, ServerConfig.Port);
      }
    
      /* We don't want to block on accept() */
      fcntl(lsd.fd, F_SETFL, fcntl(lsd.fd, F_GETFL) | O_NONBLOCK);
    
      ListenSockets.push_back(lsd);
     }
    
     if(ListenSockets.size() == 0)
      exit(-1);
    
     #if defined(HAVE_MLOCKALL) && defined(MCL_CURRENT)
     if(mlockall(MCL_CURRENT) != 0)
     {
      ErrnoHolder ene(errno);
    
      printf("Note: mlockall(MCL_CURRENT) failed: %s (not a major problem; see the documentation if curious)\n", ene.StrError());
     }
     #endif
    
     /* Now for the BIG LOOP. */
     while(1)
     {
      const time_t cur_epoch_time = time(NULL);
      bool try_accept = true;
    
      // Kill(and send quit messages) any zombie-type clients, and accept() new connections.
      for(int n = 0; n < ServerConfig.MaxClients; n++)
      {
       if(AllClients[n].TCPSocket != -1)
        continue;
    
       if(AllClients[n].TCPSocket == -1 && AllClients[n].InUse)
        KillClient(&AllClients[n]);
    
       if(try_accept)
       {  
        for(unsigned lsi = 0; lsi < ListenSockets.size() && (AllClients[n].TCPSocket == -1); lsi++)
        {
         socklen_t saddr_accept_len;
    
         if(ListenSockets[lsi].family == AF_INET6)
          saddr_accept_len = sizeof(sockin6);
         else
          saddr_accept_len = sizeof(sockin);
    
         if((AllClients[n].TCPSocket = accept(ListenSockets[lsi].fd, &saddr, &saddr_accept_len)) != -1)
         {
          char tbuf[256];
    
          /* We have a new client.  Yippie. */
    
          fcntl(AllClients[n].TCPSocket, F_SETFL, fcntl(AllClients[n].TCPSocket, F_GETFL) | O_NONBLOCK);
    
          AllClients[n].InUse = true;
          AllClients[n].timeconnect_us = MBL_Time64();
          AllClients[n].id = n;
    
          if(ListenSockets[lsi].family == AF_INET6)
           inet_ntop(ListenSockets[lsi].family, &sockin6.sin6_addr, tbuf, sizeof(tbuf));
          else
           inet_ntop(ListenSockets[lsi].family, &sockin.sin_addr, tbuf, sizeof(tbuf));
    
          printf("Client %u connecting from %s on %s", n, tbuf, ctime(&cur_epoch_time));
    
          StartNBTCPReceive(&AllClients[n], NBTCP_LOGINLEN, 4);
         }
    
         // If we didn't accept() any new client, don't call accept() any more on iterating through AllClients; we'll catch any new clients the next time around the BIG LOOP.
         if(AllClients[n].TCPSocket == -1)
          try_accept = false;
        } // end if(try_accept)
       }  
      }
    
      /* Check for users still in the login process(not yet assigned a game). BOING */
      {
       const int64 CurTimeUS = MBL_Time64();
    
       for(int n = 0; n < ServerConfig.MaxClients; n++)
       {
        if(AllClients[n].TCPSocket != -1 && !AllClients[n].game)
        {
         if((AllClients[n].timeconnect_us + (int64)ServerConfig.ConnectTimeout * 1000 * 1000) < CurTimeUS)
         {
          KillClient(&AllClients[n]);
         }
         else
          while(CheckNBTCPReceive(&AllClients[n])) {};
        }
       }
      }
    
      for(int whichgame = 0; whichgame < ServerConfig.MaxClients; whichgame++)
      {
       if(!Games[whichgame].fps)
        continue;
    
       // Destroy zombies. noooooo, poor zombies :(
       if(Games[whichgame].Zombie)
       {
        time_t curtime = time(NULL);
        printf("Game %u destroyed on %s", whichgame, ctime(&curtime));
        memset(&Games[whichgame], 0, sizeof(GameEntry));
        continue;
       }
    
       int64 last_time_inc = (int64)65536 * 256 * 1000000 / Games[whichgame].fps;
       const int64 CurGameTimeUS = MBL_Time64();
    
       if((Games[whichgame].last_time + last_time_inc) > CurGameTimeUS)
        continue;
    
       Games[whichgame].last_time += last_time_inc;
       //printf("%lld\n", last_time_inc);
    
       /* Now, the loop to get data from each client.  Meep. */
       for(int n = 0; n < MaxClientsPerGame; n++)
       {
        ClientEntry *client = Games[whichgame].Clients[n];
    
        if(!client)
         continue;
    
        while(CheckNBTCPReceive(client)) {};
    
        if((client->last_receive_time + (int64)ServerConfig.IdleTimeout * 1000 * 1000) < CurGameTimeUS)
        {
         DisconnectClient(client, "Idle timeout met.");
         continue;
        }
       } // A games clients
    
       //
       //
       //
       memset(Games[whichgame].joybuf, 0, Games[whichgame].TotalControllersDataSize + 1);
       for(int n = 0; n < MaxClientsPerGame; n++)
       {
        if(!Games[whichgame].Clients[n])
         continue;
    
        if(Games[whichgame].Clients[n]->change_pending)
         continue;
    
        int wx = 0;
        for(int c = 0; c < Games[whichgame].TotalControllers; c++)
        {
         if(Games[whichgame].ClientToController[n] & (1U << c))
         {
          for(int cnt = 0; cnt < Games[whichgame].ControllerDataSize[c]; cnt++)
          {
           Games[whichgame].joybuf[Games[whichgame].ControllerDataOffset[c] + cnt] |= Games[whichgame].Clients[n]->local_controller_buffer[wx];
           wx++;
          }
         }
        }
       }
       //
       //
       //
    
       /* Now we send the data to all the clients. */
       for(int n = 0; n < MaxClientsPerGame; n++)
       {
        if(!Games[whichgame].Clients[n])
         continue;
    
        Games[whichgame].Clients[n]->sendqcork = 0;
        MakeSendTCP(Games[whichgame].Clients[n], Games[whichgame].joybuf, Games[whichgame].TotalControllersDataSize + 1);
        Games[whichgame].Clients[n]->sendqcork = 1;
       } // A game's clients
      } // Games
    
      // Now, determine the amount of time we should sleep, and sleep it.
      // TODO: Consider using select() with a timeout in the future, so that throughput will be higher
      //       when running on very fast links with small local OS TCP receive and send buffers.
      int64 sleep_amount = 25000;
      int64 curgametime = MBL_Time64(); // Don't use cached game time from above, otherwise we could oversleep.
      for(int whichgame = 0; whichgame < ServerConfig.MaxClients; whichgame ++)
      {
       if(!Games[whichgame].fps)
        continue;
       int64 last_time_inc = (int64)65536 * 256 * 1000000 / Games[whichgame].fps;
       int64 sandwich = (Games[whichgame].last_time + last_time_inc) - curgametime;
       if(sandwich <= 0)
       {
        sleep_amount = 0;
        break;
       }
       else if(sandwich < sleep_amount)
        sleep_amount = sandwich;
      }
    
      if(sleep_amount > 0)
       MBL_Sleep64(sleep_amount);
      //printf("%lld, %lld\n", sleep_amount, MBL_Time64() - curgametime);
     } // while(1)
    }
    

    Controller Mapping Change
    The Lynx controller mapping has been modified, changing the order of buttons and adding a new button. This could affect gameplay for existing users.

    @qodo-code-review
    Copy link

    qodo-code-review bot commented Apr 18, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 363aeea
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Reuse filtered texture

    The code creates a new texture for filtering in every frame, which can lead to
    memory leaks and performance issues. The filtered texture is never released or
    reused. Implement texture reuse by storing the filtered texture as a property
    and only recreating it when dimensions change.

    PVUI/Sources/PVUIBase/PVGLViewController/PVMetalViewController.swift [1529-1553]

     // Apply custom filter if available
     if let filter = self.filter, let ciContext = self.ciContext {
         // Create CIImage from the input texture
         let ciImage = CIImage(mtlTexture: inputTexture, options: nil)
     
         if let ciImage = ciImage, let filteredImage = filter.apply(to: ciImage, in: CGRect(x: 0, y: 0, width: inputTexture.width, height: inputTexture.height)) {
    -        // Create a temporary texture for the filtered image
    -        let textureDescriptor = MTLTextureDescriptor.texture2DDescriptor(
    -            pixelFormat: inputTexture.pixelFormat,
    -            width: inputTexture.width,
    -            height: inputTexture.height,
    -            mipmapped: false
    -        )
    -        textureDescriptor.usage = [.shaderRead, .renderTarget]
    -
    -        if let filteredTexture = device?.makeTexture(descriptor: textureDescriptor) {
    +        // Check if we need to recreate the filtered texture
    +        if filteredTexture == nil || 
    +           filteredTexture?.width != inputTexture.width || 
    +           filteredTexture?.height != inputTexture.height || 
    +           filteredTexture?.pixelFormat != inputTexture.pixelFormat {
    +            
    +            // Create a new texture descriptor
    +            let textureDescriptor = MTLTextureDescriptor.texture2DDescriptor(
    +                pixelFormat: inputTexture.pixelFormat,
    +                width: inputTexture.width,
    +                height: inputTexture.height,
    +                mipmapped: false
    +            )
    +            textureDescriptor.usage = [.shaderRead, .renderTarget]
    +            
    +            // Create the new texture
    +            filteredTexture = device?.makeTexture(descriptor: textureDescriptor)
    +        }
    +        
    +        if let filteredTexture = filteredTexture {
                 // Render the filtered image to the texture
                 ciContext.render(filteredImage, to: filteredTexture, commandBuffer: nil, bounds: CGRect(x: 0, y: 0, width: inputTexture.width, height: inputTexture.height), colorSpace: CGColorSpaceCreateDeviceRGB())
    -
    -            // Use the filtered texture instead
    +            
    +            // Use the filtered texture
                 renderEncoder.setFragmentTexture(filteredTexture, index: 0)
             } else {
                 // Fall back to original texture if filtering fails
                 renderEncoder.setFragmentTexture(inputTexture, index: 0)
             }
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion implements texture reuse which significantly improves performance and prevents memory leaks by avoiding the creation of a new texture every frame. This is a critical optimization for graphics-intensive applications.

    High
    Ensure non-blocking socket operation

    The code doesn't check if the socket is non-blocking before attempting to send
    data. This could lead to blocking behavior if the socket is in blocking mode,
    potentially causing the server to hang.

    Cores/Mednafen/Sources/mednafen-server/src/mednafen-server.cpp [1160-1163]

     if(client->sendqcork > 0)
      sent = 0;
    -else 
    +else {
    + int flags = fcntl(client->TCPSocket, F_GETFL, 0);
    + if(!(flags & O_NONBLOCK))
    +   fcntl(client->TCPSocket, F_SETFL, flags | O_NONBLOCK);
      sent = send(client->TCPSocket, data, len, 0);
    +}

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 8

    __

    Why: This suggestion addresses a potential server hang issue by ensuring the socket is in non-blocking mode before sending data. This is a critical improvement for server stability and performance, preventing the server from freezing when network operations would otherwise block.

    Medium
    Add string null termination

    The function is missing null-termination of the string. Add a null terminator at
    the end of the string to ensure it's properly terminated.

    Cores/Mednafen/Sources/mednafen-server/src/md5.cpp [236-247]

     char *md5_asciistr(uint8 digest[16])
     {
      static char str[33];
      static char trans[16]={'0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f'};
      int x;
     
      for(x=0;x<16;x++)
      {
       str[x*2+1]=trans[digest[x]&0x0F];
       str[x*2]=trans[digest[x]>>4];
      }
    + str[32] = '\0';
      return(str);
     }

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 8

    __

    Why: The function creates a string representation of an MD5 hash but fails to null-terminate it, which could lead to buffer overruns or incorrect string handling. This is a significant bug that could cause unpredictable behavior when the string is used.

    Medium
    Prevent potential deadlock

    The code is missing a check for potential deadlock. If the emulation is paused
    after the first condition check but before acquiring the lock, the code could
    wait indefinitely. Add a timeout mechanism to the wait operation and recheck
    conditions after waiting.

    PVUI/Sources/PVUIBase/PVGLViewController/PVMetalViewController.swift [1388-1408]

     // For OpenGL cores, we need to handle the front buffer synchronization
     if !emulatorCore.isSpeedModified
         && (!emulatorCore.isEmulationPaused || emulatorCore.isFrontBufferReady)
         && !emulatorCore.skipLayout { // Skip layout is mostly for Retroarch
         emulatorCore.frontBufferCondition.lock()
    -    while !emulatorCore.isFrontBufferReady && !emulatorCore.isEmulationPaused {
    -        emulatorCore.frontBufferCondition.wait()
    -    }
    -    let isFrontBufferReady = emulatorCore.isFrontBufferReady
    +    // Add timeout to prevent potential deadlock
    +    let waitResult = emulatorCore.frontBufferCondition.wait(until: Date().addingTimeInterval(0.1))
    +    // Recheck conditions after waiting
    +    let isFrontBufferReady = emulatorCore.isFrontBufferReady && !emulatorCore.isEmulationPaused
         emulatorCore.frontBufferCondition.unlock()
     
         if isFrontBufferReady {
             // For OpenGL cores, the texture is updated in didRenderFrameOnAlternateThread
             // We just need to render it here
             directRender(in: view)
     
             emulatorCore.frontBufferCondition.lock()
             emulatorCore.isFrontBufferReady = false
             emulatorCore.frontBufferCondition.signal()
             emulatorCore.frontBufferCondition.unlock()
         }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion addresses a critical concurrency issue by adding a timeout to the condition wait, which prevents potential deadlocks. This is a significant improvement for application stability and responsiveness.

    Medium
    Add memory allocation checks

    The function doesn't check if the strdup calls succeeded. If memory allocation
    fails, strdup returns NULL, which would lead to a crash when accessing or
    freeing these pointers.

    Cores/Mednafen/Sources/MednafenServerBridge/MednafenServerBridge.c [9-27]

     int32_t mednafen_server_main(const char* configPath) {
         // Create the arguments array for the Mednafen server
         char* argv[2];
     
         // Program name (first argument)
         argv[0] = strdup("mednafen-server");
    +    if (argv[0] == NULL) {
    +        return -1; // Memory allocation failure
    +    }
     
         // Config file path (second argument)
         argv[1] = strdup(configPath);
    +    if (argv[1] == NULL) {
    +        free(argv[0]);
    +        return -1; // Memory allocation failure
    +    }
     
         // Call the main function of the Mednafen server
         int result = main_function(2, argv);
     
         // Free allocated memory
         free(argv[0]);
         free(argv[1]);
     
         return (int32_t)result;
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The function doesn't verify if memory allocation succeeded, which could lead to null pointer dereferences if strdup fails. This is a significant issue that could cause crashes in low-memory situations.

    Medium
    Add nil check

    The code doesn't check if outputTexture is nil before using it as the texture
    for the color attachment. This could cause a crash if outputTexture is nil. Add
    a nil check for outputTexture before creating the render pass descriptor.

    PVUI/Sources/PVUIBase/PVGLViewController/PVMetalViewController.swift [1453-1464]

    +// Check if outputTexture exists
    +guard let outputTexture = outputTexture else {
    +    DLOG("Error: Output texture is nil")
    +    return
    +}
    +
     // Create a render pass descriptor
     let renderPassDescriptor = MTLRenderPassDescriptor()
     renderPassDescriptor.colorAttachments[0].texture = outputTexture
     renderPassDescriptor.colorAttachments[0].loadAction = .clear
     renderPassDescriptor.colorAttachments[0].storeAction = .store
     renderPassDescriptor.colorAttachments[0].clearColor = MTLClearColor(red: 0.0, green: 0.0, blue: 0.0, alpha: 1.0)
     
     // Create a render command encoder
     guard let renderEncoder = commandBuffer.makeRenderCommandEncoder(descriptor: renderPassDescriptor) else {
         DLOG("Error: Could not create render encoder")
         return
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion adds an important nil check for outputTexture before using it, which prevents potential crashes. This is a valuable defensive programming practice that improves application stability.

    Medium
    Fix function overload ambiguity

    The code has two overloaded functions with the same name but different return
    type handling. This can cause ambiguity in some C++ implementations. Consider
    renaming one of the functions or using a template to handle both cases.

    Cores/Mednafen/Sources/mednafen-server/src/errno_holder.cpp [7-21]

    -static const char *srr_wrap(int ret, const char *local_strerror)
    +static const char *srr_wrap_int(int ret, const char *local_strerror)
     {
      if(ret == -1)
       return("ERROR IN strerror_r()!!!");
     
      return(local_strerror);
     }
     
    -static const char *srr_wrap(const char *ret, const char *local_strerror)
    +static const char *srr_wrap_str(const char *ret, const char *local_strerror)
     {
      if(ret == NULL)
       return("ERROR IN strerror_r()!!!");
     
      return(ret);
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    __

    Why: Having two overloaded functions with the same name but different return type handling can cause compilation issues or ambiguity in some C++ implementations. Renaming the functions makes the code more maintainable and prevents potential compiler errors.

    Low
    General
    Fix inconsistent indentation

    The indentation in this code block is inconsistent with the rest of the file,
    which uses spaces for indentation. This inconsistency could lead to readability
    issues and potential bugs in future modifications.

    Cores/Mednafen/Sources/mednafen-server/src/mednafen-server.cpp [1222-1225]

     if(client->sendqcork > 0)
    -sent = 0;
    + sent = 0;
     else
    -sent = send(client->TCPSocket, client->sendq, client->sendqlen, 0);
    + sent = send(client->TCPSocket, client->sendq, client->sendqlen, 0);

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 2

    __

    Why: The suggestion correctly identifies inconsistent indentation, but the improved code actually matches what's already in the PR. The impact is minimal as it's purely a cosmetic issue that doesn't affect functionality.

    Low
    • More

    Previous suggestions

    ✅ Suggestions up to commit 43cc433
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix syntax error
    Suggestion Impact:The commit fixed the syntax error by removing the extra closing parenthesis in the PRODUCT_BUNDLE_IDENTIFIER exactly as suggested

    code diff:

    -				PRODUCT_BUNDLE_IDENTIFIER = "$(PRODUCT_BUNDLE_IDENTIFIER_LITE))";
    +				PRODUCT_BUNDLE_IDENTIFIER = "$(PRODUCT_BUNDLE_IDENTIFIER_LITE)";

    There's a syntax error in the bundle identifier - an extra closing parenthesis
    that will cause build failures. Remove the extra closing parenthesis.

    Provenance.xcodeproj/project.pbxproj [12193]

    -PRODUCT_BUNDLE_IDENTIFIER = "$(PRODUCT_BUNDLE_IDENTIFIER_LITE))";
    +PRODUCT_BUNDLE_IDENTIFIER = "$(PRODUCT_BUNDLE_IDENTIFIER_LITE)";

    [Suggestion has been applied]

    Suggestion importance[1-10]: 10

    __

    Why: The extra closing parenthesis in the variable reference will cause build failures when the project is compiled. This is a critical syntax error that would prevent the application from building properly.

    High
    Fix incorrect system lookup
    Suggestion Impact:The commit fixed the incorrect system lookup by replacing the hardcoded string 'identifier' with the game's system identifier, but also added a nil-coalescing operator to handle potential nil values

    code diff:

    -            object.system = self.object(ofType: PVSystem.self, forPrimaryKey: "identifier")
    +            object.system = self.object(ofType: PVSystem.self, forPrimaryKey: game.system?.identifier ?? "identifier")

    The code is using a hardcoded string "identifier" as the primary key for
    PVSystem lookup, which is incorrect. It should use the game's system identifier
    instead.

    PVLibrary/Sources/PVRealm/RealmPlatform/Entities/PVGame.swift [286]

    -object.system = self.object(ofType: PVSystem.self, forPrimaryKey: "identifier")
    +object.system = self.object(ofType: PVSystem.self, forPrimaryKey: game.system.identifier)

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    __

    Why: The code is using a hardcoded string "identifier" as the primary key for PVSystem lookup, which is a critical bug. This would always retrieve the same system regardless of the game's actual system, leading to incorrect associations in the database.

    High
    Release security-scoped resource access
    Suggestion Impact:The commit implemented exactly what was suggested - adding code to start accessing the security-scoped resource and properly releasing it using defer and stopAccessingSecurityScopedResource() to prevent resource leaks

    code diff:

    +                let secureAccess = fileToDownload.startAccessingSecurityScopedResource()
    +                defer {
    +                    if secureAccess {
    +                        fileToDownload.stopAccessingSecurityScopedResource()
    +                    }
    +                }

    The function doesn't release the security-scoped resource access after
    downloading the file. When accessing security-scoped resources, you should
    always call stopAccessingSecurityScopedResource() after you're done with the
    resource to avoid resource leaks.

    PVLibrary/Sources/PVLibrary/Importer/iCloud/iCloudSync.swift [311-323]

     func handleFileToDownload(_ file: URL, isDownloading: Bool, percentDownload: Double) async {
         do {//only start download if we haven't already started
             if let fileToDownload = await insertDownloadingFile(file),
                !isDownloading || percentDownload < 100,
                //during the initial run we do NOT do any downloads otherwise we run into issues where the query gets stuck for large libraries.
                await status.value != .initialUpload {
    +            let secureAccess = fileToDownload.startAccessingSecurityScopedResource()
    +            defer {
    +                if secureAccess {
    +                    fileToDownload.stopAccessingSecurityScopedResource()
    +                }
    +            }
                 try fileManager.startDownloadingUbiquitousItem(at: fileToDownload)
                 ILOG("Download started for: \(file.pathDecoded)")
             }
         } catch {
             await errorHandler.handleError(error, file: file)
             ELOG("Failed to start download on file \(file.pathDecoded): \(error)")
         }
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    __

    Why: This suggestion addresses a potential resource leak by properly managing security-scoped resource access. Not releasing security-scoped resources can lead to permission issues and resource leaks over time, especially in an app that handles many files.

    Medium
    Handle cascading deletions properly
    Suggestion Impact:The commit implemented the suggestion's core functionality by adding proper validation checks before deleting related objects. The implementation checks if objects are invalidated before attempting to delete them, preventing crashes from trying to delete already deleted objects.

    code diff:

         /// - Parameter game: game entity to delete related entities
         @RealmActor
         private func deleteGame(_ game: PVGame) async throws {
    +        // Make a frozen copy of the game before deletion to use for cache removal
    +        let frozenGame = game.freeze()
    +        
             await try realm.asyncWrite {
    -            //TODO: validate what needs to be deleted because currently it's crashing if it's already deleted so only deleting game works
    -            /*game.saveStates.forEach { try? $0.delete() }
    -            game.cheats.forEach { try? $0.delete() }
    -            game.recentPlays.forEach { try? $0.delete() }
    -            game.screenShots.forEach { try? $0.delete() }*/
    +            guard !game.isInvalidated else { return }
    +            // Delete related objects if they exist
    +            if !game.saveStates.isInvalidated {
    +                game.saveStates.forEach { save in
    +                    if !save.isInvalidated {
    +                        realm.delete(save)
    +                    }
    +                }
    +            }
    +            if !game.cheats.isInvalidated {
    +                game.cheats.forEach { cheat in
    +                    if !cheat.isInvalidated {
    +                        realm.delete(cheat)
    +                    }
    +                }
    +            }
    +            if !game.recentPlays.isInvalidated {
    +                game.recentPlays.forEach { play in
    +                    if !play.isInvalidated {
    +                        realm.delete(play)
    +                    }
    +                }
    +            }
    +            if !game.screenShots.isInvalidated {
    +                game.screenShots.forEach { screenshot in
    +                    if !screenshot.isInvalidated {
    +                        realm.delete(screenshot)
    +                    }
    +                }
    +            }
                 realm.delete(game)
    +            
    +            // Use the more efficient cache removal instead of reloading the entire cache
    +            RomDatabase.removeGameFromCache(frozenGame)
             }

    The function is ignoring potential cascading deletions which could lead to
    orphaned records in the database. Instead of commenting out the code, implement
    proper error handling to check if related objects exist before attempting to
    delete them.

    PVLibrary/Sources/PVLibrary/Importer/iCloud/iCloudSync.swift [890-900]

     @RealmActor
     func deleteGame(_ game: PVGame) async throws {
         await try realm.asyncWrite {
    -        //TODO: validate what needs to be deleted because currently it's crashing if it's already deleted so only deleting game works
    -        /*game.saveStates.forEach { try? $0.delete() }
    -        game.cheats.forEach { try? $0.delete() }
    -        game.recentPlays.forEach { try? $0.delete() }
    -        game.screenShots.forEach { try? $0.delete() }*/
    +        // Delete related objects if they exist
    +        if !game.saveStates.isInvalidated {
    +            game.saveStates.forEach { save in
    +                if !save.isInvalidated {
    +                    realm.delete(save)
    +                }
    +            }
    +        }
    +        if !game.cheats.isInvalidated {
    +            game.cheats.forEach { cheat in
    +                if !cheat.isInvalidated {
    +                    realm.delete(cheat)
    +                }
    +            }
    +        }
    +        if !game.recentPlays.isInvalidated {
    +            game.recentPlays.forEach { play in
    +                if !play.isInvalidated {
    +                    realm.delete(play)
    +                }
    +            }
    +        }
    +        if !game.screenShots.isInvalidated {
    +            game.screenShots.forEach { screenshot in
    +                if !screenshot.isInvalidated {
    +                    realm.delete(screenshot)
    +                }
    +            }
    +        }
             realm.delete(game)
         }
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion properly implements cascading deletions with validation checks instead of commenting out the code. This prevents orphaned records in the database and ensures proper cleanup of related objects when deleting a game.

    Medium
    Fix incorrect error flagging

    Setting failedToFixPartialPath = true unconditionally when the path contains
    "iCloud" or "private" is incorrect. This flag should only be set if the path
    resolution actually fails, not as a default assumption.

    PVLibrary/Sources/PVRealm/RealmPlatform/Entities/Files/PVFile.swift [175-176]

     if fixedPartialPath.contains("iCloud") || fixedPartialPath.contains("private") {
    -    failedToFixPartialPath = true
         var pathComponents = (fixedPartialPath as NSString).pathComponents
         pathComponents.removeFirst()
         let path = pathComponents.joined(separator: "/")
         let isDocumentsDir = path.contains("Documents")
    Suggestion importance[1-10]: 7

    __

    Why: Setting failedToFixPartialPath to true unconditionally when paths contain "iCloud" or "private" is incorrect and would log errors for valid paths. This flag should only be set if path resolution actually fails.

    Medium
    General
    Standardize variable naming
    Suggestion Impact:The commit implemented the suggestion by changing ORG_IDENTIFIER to ORG_PREFIX in multiple locations in the project.pbxproj file, exactly as suggested

    code diff:

    -				PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_IDENTIFIER).ThumbnailExtensionMacOS";
    +				PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_PREFIX).ThumbnailExtensionMacOS";
     				PRODUCT_NAME = "$(TARGET_NAME)";
     				PROVISIONING_PROFILE_SPECIFIER = "";
     				SDKROOT = macosx;
    @@ -14362,7 +14362,7 @@
     				MACOSX_DEPLOYMENT_TARGET = 11.0;
     				MARKETING_VERSION = 1.0;
     				MTL_FAST_MATH = YES;
    -				PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_IDENTIFIER).ThumbnailExtensionMacOS";
    +				PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_PREFIX).ThumbnailExtensionMacOS";
     				PRODUCT_NAME = "$(TARGET_NAME)";
     				PROVISIONING_PROFILE_SPECIFIER = "";
     				SDKROOT = macosx;
    @@ -14408,7 +14408,7 @@
     				MACOSX_DEPLOYMENT_TARGET = 11.0;
     				MARKETING_VERSION = 1.0;
     				MTL_FAST_MATH = YES;
    -				PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_IDENTIFIER).ThumbnailExtensionMacOS";
    +				PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_PREFIX).ThumbnailExtensionMacOS";

    The variable ORG_IDENTIFIER is used here but ORG_PREFIX is used elsewhere in the
    project. For consistency, you should use the same variable name throughout the
    project.

    Provenance.xcodeproj/project.pbxproj [14318]

    -PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_IDENTIFIER).ThumbnailExtensionMacOS";
    +PRODUCT_BUNDLE_IDENTIFIER = "$(ORG_PREFIX).ThumbnailExtensionMacOS";

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    __

    Why: Using inconsistent variable names (ORG_IDENTIFIER vs ORG_PREFIX) across the project can lead to confusion and potential build issues. Standardizing on one variable name improves maintainability and reduces the chance of configuration errors.

    Medium
    Fix notification timing

    The notification is posted regardless of whether there are items to process.
    This could trigger false "finished importing" notifications even when no actual
    import occurred. Move the notification post inside the guard condition to only
    fire when actual processing completes.

    PVLibrary/Sources/PVLibrary/Importer/Services/GameImporter/GameImporter.swift [681-688]

     defer {
         DispatchQueue.main.async {
             // Only change to idle if we're not paused
             if self.processingState != .paused {
                 self.processingState = .idle
    +            NotificationCenter.default.post(name: .RomsFinishedImporting, object: nil)
             }
    -        NotificationCenter.default.post(name: .RomsFinishedImporting, object: nil)
         }
     }
    Suggestion importance[1-10]: 6

    __

    Why: The notification is being posted regardless of whether any ROMs were actually processed, which could trigger false "finished importing" notifications. Moving it inside the conditional would ensure it only fires when actual processing completes.

    Low

    …fore starting initial start of downloads, so users can play games that have previously imported into the db/app while icloud syncs.
    @pabloarista pabloarista reopened this Apr 24, 2025
    @pabloarista pabloarista changed the base branch from develop to feature/EmulationScene April 24, 2025 01:26
    @JoeMatt JoeMatt merged commit be490d0 into Provenance-Emu:feature/EmulationScene Apr 24, 2025
    4 of 13 checks passed
    @pabloarista pabloarista deleted the feature/iCloud-sync-v1 branch April 25, 2025 02:31
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants