Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPC manager rework #52

Draft
wants to merge 14 commits into
base: development
Choose a base branch
from
Draft

RPC manager rework #52

wants to merge 14 commits into from

Conversation

Arkensor
Copy link
Owner

  • Added CF.RPC / CF_RPC as easier to use wrapper around rpcs
  • Moved RPCManager to legacy folder

The main advantage is that you do not need to explicitly register your mod and function for everything you might want to send. If a mod only needs one handler you only set it up once and then every new function added does not require any interaction with CF.
Additionally, the now underlying ScriptRPC allows for retransmission of the same data to multiple receivers.

What is not part of the reworked interface is the whole concept of single-player execution behavior. There are other more compact ways to switch logic for single-player testing. Either they use different functions, or within the function, you check if you are the host or not and handle the data accordingly. Besides - I have not seen much clever use of it. If someone absolutely needs it (which I doubt) they can refer to the legacy RPCManager that continues to exist until we figure out a way to update the new interface to offer that as well.

See the minimal demo of how RPC's would work with this now:

class RpcMinimalDemo
{
    void RpcMinimalDemo()
    {
        CF.RPC.RegisterHandler(this); //This can be skipped if the handler inheris from CF_RPC_HandlerBase
    }
    
    /*
        This is is the interface of functions that handle rpcs. It always has to be these 3 parameters.
        Sender will return null in singleplayer.
    */
    void Hello(ParamsReadContext ctx, PlayerIdentity sender, Object target)
    {
        PrintFormat("Hello -> ParamsReadContext %1; PlayerIdentity %2; Object %3", ctx, sender, target);

        string data;
        if(!ctx.Read(data)) return;

        PrintFormat("Data received: %1", data);
    }
    
    void SendRpcToServer()
    {
        auto rpc = CF.RPC.Prepare("RpcMinimalDemo", "Hello", true);
        rpc.Write("Hello from the other side!");
        
        //Option 1 - NULL target
        rpc.SendTo();
        
        //Option 2 - explicit but still NULL target
        rpc.SendTo(NULL);
        
        //Option 3 for more readability
        rpc.SendTo(CF.RPC.SERVER);
    }
    
    void SendRpcFrommServerToPlayer(PlayerIdentity player)
    {
        auto rpc = CF.RPC.Prepare("RpcMinimalDemo", "Hello", true);
        rpc.Write("Hello Player!");
        rpc.SendTo(player);
    }
}

@Arkensor Arkensor self-assigned this Apr 21, 2021
@InclementDab
Copy link
Contributor

What happens when you register multiple RPC handlers of the same typename? It goes specifically by instance right? Will it send an RPC to all instances of RpcMinimalDemo?

@Arkensor
Copy link
Owner Author

Arkensor commented Apr 21, 2021

What happens when you register multiple RPC handlers of the same typename? It goes specifically by instance right? Will it send an RPC to all instances of RpcMinimalDemo?

Registering an instance of the same type twice will result in a warning: https://github.com/Jacob-Mango/DayZ-Community-Framework/blob/f9deb5e56a43e898fb1cb63ad1a43599b4bcd765/JM/CF/Scripts/3_Game/CommunityFramework/RPC/RPC.c#L34

There can only be one handler because of the read context in the RPC. Once the first handler read the data from it, it would be empty for the next one. I do not see a way around it currently.

@Jacob-Mango
Copy link
Collaborator

Jacob-Mango commented Apr 21, 2021

Needs to be able to explicitly mark the functions which can be handled. Don't want to allow any function within the handler to be called.

@Arkensor
Copy link
Owner Author

Needs to be able to explicitly mark the functions which can be handled. Don't want to allow any function within the handler to be called.

Maybe an optional whitelist when registering? Default would be everything is whitelisted (for the main use-case of extra handlers that ONLY contain functions to be called). And if you were to add MissionGameplay as handler instance you could restrict it to the 2-3 functions you want to be callable.
I think a whitelist approach is better for the workflow than blacklisting functions you are not allowed to call?

Arkensor added 11 commits April 22, 2021 13:16
Updated invoke function interface order
Remove invoke function target object parameter
Removed target parameter from context prepare
Updated inline documentation
Added CF.RPC parameter validation
Updated CF.RPC inline documentation
Updated RPC documentation with tweaks
Updated RPC context functions to be chainable
Updated RPC.SendTo to force recipient parameter
Updated RPC documentation formatting
@Arkensor Arkensor marked this pull request as draft June 30, 2021 16:39
Updated function name for setting rpc handler function parameters
Fixed typos

if(m_FunctionParameterTypes.Contains(hashKey))
{
PrintFormat("[WARNING][CommunityFramework][RPC] Failed to register function parameters for %1. Already registered.\n Existing parameters: %2. See trace below:", hashKey, m_FunctionParameterTypes.Get(hashKey));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove print message here, there are 2 reasons the constructor can be run again.

  1. The class is initiated during mission runtime many times during the games lifetime
  2. The class contains a network ID (Object)

Remove the message from other offending areas as well.

@Arkensor Arkensor added the enhancement New feature or request label Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants