Announcement

Collapse
No announcement yet.

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

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

  • replied
    That path has been fixed in 4.21 ^

    Leave a comment:


  • replied
    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:

    Code:
    virtual FArchive& FMemoryArchive::operator<<( class FName& N ) override
    Which basically does:
    Code:
    FString StringName = N.ToString();
    *this << StringName;
    When reading with FBitReader it goes down:

    Code:
    virtual FArchive& FNetBitReader::operator<<( class FName& N ) override
    Which basically does:
    Code:
    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...

    Leave a comment:


  • replied
    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

    Leave a comment:


  • replied
    Stephen Ellis 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:
    Code:
    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 ****ing smooth change by the Epic guys.

    Leave a comment:


  • replied
    Stephen Ellis OnlineReplStructs.cpp, #237

    Code:
    // 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.

    Code:
    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.

    Leave a comment:


  • replied
    Originally posted by Stephen Ellis View Post

    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 https://epicsupport.force.com/unrealengine/s/. 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.

    Leave a comment:


  • replied
    Originally posted by vlad.serbanescu11 View Post

    I have. Couldn't get it work.

    Stephen Ellis 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 https://epicsupport.force.com/unrealengine/s/. 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.

    Leave a comment:


  • replied
    Originally posted by Paul &quot;TBBle&quot; Hampson View Post
    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 https://issues.unrealengine.com/issue/UE-63326.

    I haven't tested it myself yet though.
    I have. Couldn't get it work.

    Stephen Ellis 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!

    Leave a comment:


  • replied
    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 https://issues.unrealengine.com/issue/UE-63326.

    I haven't tested it myself yet though.

    Leave a comment:


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

    Leave a comment:


  • replied
    I don't see this being fixed at all, despite the issue tracking having it marked as "FIXED": https://issues.unrealengine.com/issue/UE-63326

    Here's the relevant code from "OnlineSubsystemUtilsModule.cpp":

    Code:
    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.

    Leave a comment:


  • replied
    Originally posted by Vormulac View Post
    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).

    Leave a comment:


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

    Leave a comment:


  • replied
    This was logged as https://issues.unrealengine.com/issue/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:

    If you change the serialization of the type in both places to serialize it as a string it should work.

    In FUniqueNetIdRepl::MakeReplicationData, change the two places Type is serialized from
    1. if (TypeHash == TypeHash_Other)
    2. {
    3. Writer << Type;
    4. }

    to:
    1. if (TypeHash == TypeHash_Other)
    2. {
    3. FString TypeString = Type.ToString();
    4. Writer << TypeString;
    5. }

    In FUniqueNetIdRepl::NetSerialize, change the two places Type is serialized from
    1. if (TypeHash == TypeHash_Other)
    2. {
    3. Ar << Type;
    4. if (Ar.IsError() || Type == NAME_None)
    5. {
    6. bValidTypeHash = false;
    7. }
    8. }

    to:
    1. if (TypeHash == TypeHash_Other)
    2. {
    3. FString TypeString;
    4. Ar << TypeString;
    5. Type = *TypeString;
    6. if (Ar.IsError() || Type == NAME_None)
    7. {
    8. bValidTypeHash = false;
    9. }
    10. }
    For binary users, this might be a viable workaround
    If you happen to use FUniqueNetIdString in your custom online subsystem then it might be possible. You could provide one of the compiled in FNames of an online subsystem to the Type paramter of the FUniqueNetIdString ctor in your online subsystem. I tested this with "LIVE" in our case. If this built in subsystem is not loaded the serialization code will fall back to FUniqueNetIdString. ("LIVE" is a good match in this case as it also uses FUniqueNetIdString so it doesn't matter if it's loaded or not.)

    Leave a comment:


  • replied
    Any updates on this? I believe we are running into this and its critically holding up development.

    Cheers!

    Leave a comment:

Working...
X