code-iai / ROSIntegration

Unreal Engine Plugin to enable ROS Support
MIT License
411 stars 133 forks source link

Subsystem Game Instance #182

Open phuicy opened 2 years ago

phuicy commented 2 years ago

Hi All,

I have a branch where I have reimplemented the GameInstance as a GameInstanceSubsystem, this allows to not need to overide other game instances; but also allows for late loading the connection.

Thoughts? Is this useful?

thanks, Guy

phuicy commented 2 years ago

For your consideration: https://github.com/phuicy/ROSIntegration/tree/subsystem

I would be tempted to implement in game IP selection.

Sanic commented 2 years ago

Hi @phuicy Thank you for the proposition. I haven't used subsystems so far, but they look pretty useful to me and might solve a couple of problems i sometimes had with GameInstances. Do you think there is any caveat using them instead of normal GameInstances that we should highlight in the docs or ask other users before?

PetterVMC commented 1 year ago

I agree that it should be a GameInstanceSubsystem instead of GameInstance. For example, the plugin cannot currently be used together with another plugin if the other plugin is also implemented as a GameInstance derived class, unless modifying the code so that one plugin's class derives from the other.

Looking at the way GameInstanceSubsystems' Initialize and Deinitialize methods are being called from GameInstance.cpp, changing it doesn't appear to lead to any critical change in code flow from how it is today.

@phuicy In addition to your implementation, I think it could be useful to add a static helper method to get the subsystem more easily in C++. Perhaps something like:

class ROSINTEGRATION_API UROSIntegrationGameInstance : public UGameInstanceSubsystem
{
    // ...
    static UROSIntegrationGameInstance* GetFrom(const UObject* Obj)
    {
        Obj->GetWorld()->GetGameInstance()->GetSubsystem<UROSIntegrationGameInstance>();
    }
    // ...
};

Also, a critical thing that needs to be solved is to make the properties of UROSIntegrationGameInstance accessible in the editor. From what I know a subsystem's properties will not magically show anywhere automatically. Though, it is possible to create a ROS Integration section for them in the editor's Project Settings window, which I think would be the most logical thing to do, but it requires creating an editor module and some additional code. Did you try something like that yet?

We should probably also have some code for migrating the settings from old projects to the new system seamlessly. I think core redirects can do it.