[Fix Incoming] Seriously damaging change to Online Subsystems in 4.20

This was logged as Unreal Engine Issues and Bug Tracker (UE-63326) and believed to be fixed with the 4.20.3 hotfix, however new information is showing that may not be the case.

I’ve pulled some conversation from private channels that may help.

For those using source code via GitHub:

For binary users, this might be a viable workaround

Does anyone know if the solution above for binary users works (particularly with the UWorks plugin)?

I’ve been following this thread.

The binary “workaround” is not a solution for V13 (and below) of UWorks. I didn’t know we would reach this point so I didn’t use FUniqueNetIdString. I’m looking into modifying this part for the next version.

That being said, I find both of these solution ugly. It’s already a pain to even create a new, blank subsystem (the module loads subsystems by creating factories which create subsystems which create interfaces… Now we have to take care of hashes and so on… What? And that’s just the tip).

I don’t see this being fixed at all, despite the issue tracking having it marked as “FIXED”: Unreal Engine Issues and Bug Tracker (UE-63326)

Here’s the relevant code from “OnlineSubsystemUtilsModule.cpp”:



private:

    void Init()
    {
        FOnlineSubsystemDelegates::OnOnlineSubsystemCreated.AddRaw(this, &FOnlineSubsystemUtils::OnOnlineSubsystemCreated);
        CreateNameHashes();
    }

    void CreateNameHashes()
    {
        int32 HashId = 1;
#define CREATE_HASH(MacroName) \
{ \
 uint8 Hash = static_cast<uint8>(HashId); \
 SubsystemNameToHash.Add(MacroName, Hash); \
 HashToSubsystemName.Add(Hash, MacroName); \
 HashId++; \
}
        CREATE_HASH(NULL_SUBSYSTEM); 
        CREATE_HASH(MCP_SUBSYSTEM);
        CREATE_HASH(STEAM_SUBSYSTEM);
        CREATE_HASH(PS4_SUBSYSTEM);
        CREATE_HASH(LIVE_SUBSYSTEM);
        CREATE_HASH(GOOGLE_SUBSYSTEM);
        CREATE_HASH(GOOGLEPLAY_SUBSYSTEM);
        CREATE_HASH(FACEBOOK_SUBSYSTEM);
        CREATE_HASH(IOS_SUBSYSTEM);
        CREATE_HASH(TENCENT_SUBSYSTEM);
        CREATE_HASH(SWITCH_SUBSYSTEM);
        CREATE_HASH(AMAZON_SUBSYSTEM);
        CREATE_HASH(GAMECIRCLE_SUBSYSTEM);
        CREATE_HASH(THUNDERHEAD_SUBSYSTEM); 
        CREATE_HASH(WECHAT_SUBSYSTEM); 
        CREATE_HASH(TWITCH_SUBSYSTEM); 
        CREATE_HASH(OCULUS_SUBSYSTEM); 
        CREATE_HASH(QUAIL_SUBSYSTEM);
        // Shouldn't need these as they are mocking interfaces for existing platforms
        CREATE_HASH(PS4SERVER_SUBSYSTEM);
        CREATE_HASH(LIVESERVER_SUBSYSTEM);

#undef CREATE_HASH

        ensure(SubsystemNameToHash.Num() == (HashId - 1));
        ensure(HashToSubsystemName.Num() == (HashId - 1));

        // FUniqueNetIdRepl uses 5 bits to transmit the HashId and 31 is used for OnlineSubsystems not included in this list
        check(HashId < 31);
    }

    /** If false it will not try to do online PIE at all */
    bool bShouldTryOnlinePIE;

    /** Delegate set by the engine for notification of external UI operations */
    FOnExternalUIChangeDelegate OnExternalUIChangeDelegate;
    TMap<FName, FDelegateHandle> ExternalUIDelegateHandles;

    /** Delegate binding when new online subsystems are created */
    FDelegateHandle OnOnlineSubsystemCreatedDelegateHandle;
    /** Mapping of OSS Names to uint8 hashes */
    TMap<FName, uint8> SubsystemNameToHash;
    TMap<uint8, FName> HashToSubsystemName;

    friend FOnlineSubsystemUtilsModule;
};

As you can notice, the function “CreateNameHashes()” and the variables “SubsystemNameToHash” and “HashToSubsystemName” are still private. If you want to allow plugins to host custom subsystems, as you should, this is where the change needs to happen.

Just add a public function “AddNameHash” instead of that hideous macro.

Has this been fixed with the release of 4.21?
Waiting for clean fix unitil I purchase, so can’t check myself

It appears to be fixed in 4.21. The changes described in post #21 above have been applied in 4.21, in the fix changelist linked from Unreal Engine Issues and Bug Tracker (UE-63326).

I haven’t tested it myself yet though.

I have. Couldn’t get it work.

[USER=“9”]Stephen Ellis[/USER] Would you please take a look at post #24? I have tested this with a custom OnlineSubsystem hosted in a plugin, several times. You can take a look at my product from my subscription and use the exact files from the Marketplace to reproduce. Thanks in advance!

Hi Vlad.

Our engineers have reviewed the issue, and in our tests the functionality is working as expected, so you may have something unique to your case that isn’t covered by our tests. In order for us to investigate this fully, please submit a bug report via Unreal Engine Community. Please be sure to include as much information as possible, including the exact failure, and ideally a test project that reproduces the issue.

Thank you.

In 4.21, even with the changes, when we try to call UniqueID.IsValid() inside of AGameModeBase::PreLogin(), when adding a breakpoint, the uniqueID is always nil. It’s just empty. It then crashes because it says “incompatible_net_id” or something.

[USER=“9”]Stephen Ellis[/USER] OnlineReplStructs.cpp, #237



// Non empty and hex encoded
uint8 TypeHash = GetTypeHashFromEncoding(EncodingFlags);
if (TypeHash == 0)
{
    // If no type was encoded, assume default
    TypeHash = UOnlineEngineInterface::Get()->GetReplicationHashForSubsystem(UOnlineEngineInterface::Get()->GetDefaultOnlineSubsystemName());
}
FName Type;
bool bValidTypeHash = TypeHash != 0;


Why? My one single question? Just why do you “assume default”? If I have an “OnlineSubsystemStephen” and you can’t find the hash for “Stephen”, why do you just assume “Null” instead? By what logic is that supposed to work for custom subsystems?

Here’s how the “UOnlineEngineInterface::Get()” part works. I don’t see any trace of loading my config’s subsystem anywhere.



UOnlineEngineInterface* UOnlineEngineInterface::Get()
{
    if (!Singleton)
    {
        // Proper interface class hard coded here to emphasize the fact that this is not expected to change much, any need to do so should go through the OGS team first
        UClass* OnlineEngineInterfaceClass = StaticLoadClass(UOnlineEngineInterface::StaticClass(), NULL, TEXT("/Script/OnlineSubsystemUtils.OnlineEngineInterfaceImpl"), NULL, LOAD_Quiet, NULL);
        if (!OnlineEngineInterfaceClass)
        {
            // Default to the no op class if necessary
            OnlineEngineInterfaceClass = UOnlineEngineInterface::StaticClass();
        }

        Singleton = NewObject<UOnlineEngineInterface>(GetTransientPackage(), OnlineEngineInterfaceClass);
        Singleton->AddToRoot();
    }

    return Singleton;
}


I’m done with investigating this myself. Everything I find points to the fact that the engine can’t read a simple config variable and create its own hash based on that. Hard stuff. You’ll have a new bug report with an empty subsystem in an empty plugin in an empty project with 100% repro success and another mention here.

[USER=“9”]Stephen Ellis[/USER] You’re in luck.

Found the fix for the hash issue. Instead of adding a hash, given the “fix” implemented by Epic in 4.20.3, we now need to override the following function in FUniqueNetIdUWorks, inherited from FUniqueNetId:


virtual FName GetType() const override
{
    return TEXT("UWorks");
}

It wasn’t even a pure virtual function. There wasn’t any indication whatsoever to make me check this. I found this mostly by mistake. Really ■■■■■■■ smooth change by the Epic guys.

@vlad.serbanescu11

Sorry it caused you pain. We always have “reasons”. I’ll share this one.

It was a mistake made by an integration engineer moving code from one branch to another and it was the path of least resistance “temporarily” to keep multiple projects working that didn’t have all the code for FUniqueNetId and so were failing to compile “can’t instantiate abstract class”. So it was set non virtual and forgotten about.

Apologies. It was never an intentional design decision. Lots going on around here :slight_smile:

Yeah, the silent failures really bit me, I’d much rather be forced to change my code so that it works.

Unfortunately as noted this doesn’t work at all as it is. The tests are not testing the real usage of it.

The problem is that FUniqueNetIdRepl::MakeReplicationData is using FMemoryWriter, and FUniqueNetIdRepl::NetSerialize is using FBitReader.

When writing the Type with FMemoryWriter it goes down this path:


virtual FArchive& FMemoryArchive::operator<<( class FName& N ) override

Which basically does:



FString StringName = N.ToString();
*this << StringName;


When reading with FBitReader it goes down:


virtual FArchive& FNetBitReader::operator<<( class FName& N ) override

Which basically does:


uint8 bHardcoded = InName.GetComparisonIndex() <= MAX_NETWORKED_HARDCODED_NAME;
Ar.SerializeBits(&bHardcoded, 1);
if (bHardcoded)
{
      // send by hardcoded index
      checkSlow(InName.GetNumber() <= 0); // hardcoded names should never have a Number
      uint32 NameIndex = uint32(InName.GetComparisonIndex());
      Ar.SerializeInt(NameIndex, MAX_NETWORKED_HARDCODED_NAME + 1);
}
else
{
     // send by string
     FString OutString = InName.GetPlainNameString();
     int32 OutNumber = InName.GetNumber();
     Ar << OutString << OutNumber;
}

Which causes all kinds of problems…

That path has been fixed in 4.21 ^

I still have the error. Throws it from the server. Help please. I use playfab Legacy Multiplayer