DFHack / dfhack

Memory hacking library for Dwarf Fortress and a set of tools that use it
Other
1.86k stars 465 forks source link

Document how function ids are assigned for RemoteFortressReader/RPC clients #1574

Closed alexchandel closed 3 years ago

alexchandel commented 4 years ago

In a RemoteFortressReader connection, server functions are queried based on their int16_t id, which is specified in the RPCMessageHeader.id and looked up in ServerConnection::threadFn().

It appears to be populated by repeatedly calling RPCService::finalize to fill functions with various plugins' methods, but there is no indication what order they are populated in. This seems like an incredibly brittle way of specifying an RPC API. devel/dump-rpc provides some information, but not enough to discern what RPCMessageHeader.id should be.

Are devel/dump-rpc's outputs all and exactly the functions available to RemoteFortressReader clients, in order? And how are remote clients meant to discover this?

ragundo commented 4 years ago

RemoteFortressReader uses google protobuff as the RPC channel. The plugin register the rpc methods

RemoteFortressReader.cpp:238 `

DFhackCExport RPCService *plugin_rpcconnect(color_ostream &)

RPCService *svc = new RPCService();

svc->addFunction("GetMaterialList", GetMaterialList, SF_ALLOW_REMOTE);

svc->addFunction("GetGrowthList", GetGrowthList, SF_ALLOW_REMOTE);

etc`

And the you, as a client, send each protobuf message type with the in parameters filled and receive the out parameters.

All the protobuff messages are defined in RemoteFortressReader.proto. For example

message MaterialList
{
    repeated MaterialDefinition material_list = 1;
}

corresponds to the the first function added to the RPC service

lethosor commented 4 years ago

devel/dump-rpc is intended for CI to verify that all RPC functions are registered properly. It's not really intended to be user-facing or complete.

alexchandel commented 4 years ago

Right, I saw the protobuf types. But they don't help you determine the actual functions. For example MaterialList isn't the first function; it's the return type of the RPC function GetMaterialList : EmptyMessage -> MaterialList.

GetMaterialList isn't the "first" function either, in terms of id, even though it appears so. When an RPCService is finalized, it writes pushes its functions to someone else's vector. That's the confusing part. There's no indication or way of discerning what order the plugins are so loaded. By experimentation, the "first" function available to RPC clients is BindMethod (with id = 0).

How should RFR clients discover the ids of methods?

Is there a way to specify methods by string?

lethosor commented 4 years ago

https://github.com/RosaryMala/RemoteClientDF-Net/blob/master/DFHack/RemoteFunction.cs and https://github.com/RosaryMala/RemoteClientDF-Net/blob/master/DFHack/RemoteClient.cs seem to be looking up RPC functions by name. @RosaryMala might be more able to answer specific questions about it than me.

RosaryMala commented 4 years ago

There's really no real need to know the ID. The IDs are essentially random and shared amongst all plugins, which may or may not add more functions. Part of the server connection handshake involves getting a list of functions and corresponding IDs.

alexchandel commented 4 years ago

@RosaryMala

Part of the server connection handshake involves getting a list of functions and corresponding IDs.

The key bit of information. Is this documented anywhere?

Also, unrelatedly, do you know why GetBlocksList returns an illegal first block with no map_x, map_y, and map_z? You can see here in GetBlockList() that if firstBlock is true, but nothing else, then map_x, map_y, and map_z are never written. This is illegal, as the protobuf requires those fields. This frequently happens to me, resulting in protobuf parser failures. You need to write those fields for every block, i.e. right after net_block = out->add_map_blocks();.

RosaryMala commented 4 years ago

Yeah, this is indeed an oversight on my part. Feel free to make a PR to fix that, or I can get around it at some point.

That said, RemoteFortressReader is terrible and I had no idea what I was doing when I wrote it, and it'd probably be a better idea to just kind of make your own from scratch that matches your use-case. Either that, or we can work together to make RFR less terrible.

RosaryMala commented 4 years ago

Okay, fixed in 3db490ee9e31c7a0f950f242b8c338a161a63fcc

alexchandel commented 4 years ago

Happy to help where I can. I'm working on browser bindings for native DFHack access from JS, so I'm running across all the problems of a naive implementor of this API. I think I have the network bindings down for RFR (it's not the best design… 😆), and am working on a proof-of-concept in-browser viewer now.

Tbh I would put all those subsequent tests (e.g. if (tileChanged) {...} if ...) within the braces of if (tileChanged || desChanged || spatterChanged || firstBlock || itemsChanged || flows) {...} as well. They're never meant to run when the union isn't true, and hygienic coding practices like that reduce possible bugs from future changes.

alexchandel commented 4 years ago

@RosaryMala Once it's in a less hideous state, I'll post the JS library on github / the forums. Any interest in writing a browser-based AV Lite with voxeljs / threejs?

lethosor commented 4 years ago

That sounds pretty neat! (Not that I'm a graphics person, but I'll keep an eye out for it)

Also, documentation of the RPC protocol is practically nonexistent, so we'd gladly also welcome improvements there if you're able to contribute. (I can help with the Sphinx side of things but I'm not really familiar with the RPC side myself)

RosaryMala commented 4 years ago

Yeah, I went through a similar thing when I ported remoteclient library from C++ to C+

BenLubar commented 4 years ago

There is now a Wireshark dissector for the RPC

https://github.com/DFHack/df_misc/blob/master/wireshark_dfhack_rpc.lua

BenLubar commented 4 years ago

Here's some temporary documentation until it's part of the official DFHack docs: https://gist.github.com/BenLubar/1f51cf570b4c8dca275db9687fa6b9e3

Basically, for anything apart from IDs -4 through 1, you ask method 0 and it gives you back an ID.

ID -4 and all non-negative IDs are sent by the client. -3, -2, and -1 are sent by the server.

alexchandel commented 4 years ago

@RosaryMala @BenLubar The JavaScript bindings to DFHack/RFR are now up: https://github.com/alexchandel/dfhack-remote. It's a very thin wrapper over RFR, but it has every function including Lua commands.

If anyone has experience with DF + web graphics, we now have the capability for a complete web GUI (no more SDLv1!!!).

lethosor commented 4 years ago

https://docs.dfhack.org/en/latest/docs/Remote.html has been added with more detail (when it's finished building, anyway), largely taken from posts in this thread. It's still preliminary, so let me know if there are any changes you'd like to see.