millicast / millicast-player-unreal-engine-plugin

Millicast Player plugin for Unreal Engine
Other
19 stars 15 forks source link

Fix crash if no HTTPResponse is set #55

Closed improbable-andreaskrugersen closed 1 year ago

improbable-andreaskrugersen commented 1 year ago

In Unreal it's not guaranteed that the FHttpResponsePtr is valid when handling an HTTP request. In the bConnectedSuccessfully case it's fine, but otherwise you can crash

improbable-andreaskrugersen commented 1 year ago

Also fixed some more includes in non-Unity builds

improbable-andreaskrugersen commented 1 year ago

I'd strongly recommend to run non-Unity builds as part of your CI as well to catch all of these missing includes

rweber89 commented 1 year ago

Hi @improbable-andreaskrugersen - the 4.27 branch is severely outdated. I will recommend not merging this in. I'm working on making our various version branches redundant.

I will incorporate your changes into that.

rweber89 commented 1 year ago

@improbable-andreaskrugersen - Would you mind letting me know what you do to run non-Unity builds? I have not done that before, adding bUseUnityBuild = false; on the Target.cs for the project seems to do nothing, and changing the UBT build configuration fails compiling on engine code in 4.27.

improbable-andreaskrugersen commented 1 year ago

@rweber89 Setting bUseUnityBuild = false is correct but you need to do that in the Build.cs file of your module

In regards to the outdated branch: We're still currently upgrading to UE5 but needed an upgrade of the Millicast plugin before that, so the UE4.27 branch was all we could use

rweber89 commented 1 year ago

Hi @improbable-andreaskrugersen any chance that you could confirm that the issues are fixed on the dev branch?

In which case I would close this PR.

improbable-andreaskrugersen commented 1 year ago

This is obsolete now. I created a separate PR for include fixes on dev