Proposal: Namespace methods according to respective Proto file

[justin-lovell]

In browsing the source code, I may have generated a conflict without realizing it at the time. Here are two functions which both define getPlayers:

rust-server/lua/DCS-gRPC/methods/coalitions.lua

Lines 153 to 160 in b1d3906

	GRPC.methods.getPlayers = function(params)
	local units = coalition.getPlayers(params.coalition)
	local result = {}
	for i, unit in ipairs(units) do
	result[i] = GRPC.exporters.unit(unit)
	end
	return GRPC.success({units = result})
	end

rust-server/lua/DCS-gRPC/methods/net.lua

Lines 24 to 43 in c437fb4

	GRPC.methods.getPlayers = function()
	local players = {};
	for _,v in pairs(net.get_player_list()) do
	local playerInfo = net.get_player_info(v);
	table.insert(players, {
	id = playerInfo.id,
	name = playerInfo.name,
	coalition = playerInfo.side + 1, -- common.Coalition enum offset
	slot = playerInfo.slot,
	ping = playerInfo.ping,
	remoteAddress = playerInfo.ipaddr,
	ucid = playerInfo.ucid,
	locale = playerInfo.lang
	})
	end
	return GRPC.success({players = players})
	end

I would like to propose to avoid naming conflicting, let's prefix the method name with some annotation of the proto file as a namespace.

E.g.

- GRPC.methods.getPlayers = function(params)
+ GRPC.methods.net_getPlayers = function(params)

and

- GRPC.methods.getPlayers = function(params)
+ GRPC.methods.coalition_getPlayers = function(params)

An alternative is setup a localized Lua table with the following pseudo:

local coalition = {}
GRPC.methods.coalition = coalition

coalition.getPlayers = function() {
-- ...
}

and the Rust code would perform a function call similar to:

- self.request("getPlayers", request)
+self.request("coalition", "getPlayers", request)

[rurounijones]

Generally I would go with the simplest thing and just prefix like net_getPlayers.

However in this particular case I wonder if we shouldn't instead change the API names to be a bit more specifically accurate. coalition.getPlayers was named as such because that was the ED name.

However, especially in light of the conflict above, I wonder if we should rename this to coalition.getPlayerUnits since it is actually a more accurate representation of the data being returned which is in contrast to the data being returned by net.getPlayers which is more about the player than the unit.

[justin-lovell]

I wonder if we should rename this to coalition.getPlayerUnits since it is actually a more accurate representation of the data being returned which is in contrast to the data being returned by net.getPlayers which is more about the player than the unit.

Good observation. I will integrate that in a PR to come shortly.

This morning, I thought of a lightweight solution to detect conflicts in registration. What if we had functional registrations such as

GRPC.registerMethod('theMethodName', function(params)

end)

If there is already another method, deregister both methods. That way the person who is writing a new function would be forced to acknowledge the duplication.

The only other alternative is to perform a full regression test suite, which is very heavy given for full confidence, we need to launch DCS on a CI build (ouch!)