DCS-gRPC / rust-server

DCS gRPC server written in Rust. Get data out of DCS and send commands into DCS.
GNU Affero General Public License v3.0
87 stars 23 forks source link

Shot events not registering #258

Open MartinHell opened 3 months ago

MartinHell commented 3 months ago

Heya,

The shot events seems to not be registering at all. I'm no rust or lua expert but I've tried to debug this as best as I can.

From the logs I can see that the event is being handled by the eventHandler however it does give the following error:

 ERROR   dcs_grpc: failed to deserialize event: deserialize error: missing field `id`

If I print the weapon.id and weapon.type fields in the debug log by modifying the event function as such:

  elseif event.id == world.event.S_EVENT_SHOT then
    GRPC.logDebug("Shot event")
    weapon_test = exporter(event.weapon)
    GRPC.logDebug("Weapon ID: " .. tostring(weapon_test.id))
    GRPC.logDebug("Weapon Type: " .. tostring(weapon_test.type))
    return {
      time = event.time,
      event = {
        type = "shot",
        initiator = {initiator = typed_exporter(event.initiator)},
        weapon = exporter(event.weapon)
      },
    }

The logs will confirm that the id is in fact nil or missing:

2024-05-22 15:32:47.714 DEBUG   dcs_grpc: Weapon ID: nil
2024-05-22 15:32:47.714 DEBUG   dcs_grpc: Weapon Type: SA2V755

the exporter(event.weapon) should to my understanding reach the following code: https://github.com/DCS-gRPC/rust-server/blob/6621b6d93d02d94f5cc604bcf85bedd39a6ae001/lua/DCS-gRPC/exporters/object.lua#L56

What I don't understand is why tonumber is used for the id at: https://github.com/DCS-gRPC/rust-server/blob/6621b6d93d02d94f5cc604bcf85bedd39a6ae001/lua/DCS-gRPC/exporters/object.lua#L58

The documentation for weapon:getName() returns a string and not an int according to https://wiki.hoggitworld.com/view/DCS_func_getName. So to me it seems like this will always return nil or an empty id unless there's something else going on that I can't figure out.

For testing I've tried to just set the id to 1 which seems to work (I'm mostly interested in the type anyway).

Anyway if someone could either explain to me a bit more about the reasoning for using tonumber and/or explain what I'm missunderstanding here it would be greatly appreciated:) I'm sure there might be other instances where the tonumber for id is causing issues since I see it used for IDs in other places then the weapon.

Thanks

rurounijones commented 3 months ago

Unfortunately the DCS API is a constantly shitfing base where bugs and changes get introduced without any documentation.

Weapon IDs were always numbers in strings when the code was written. Now? I have no idea as I don't play DCS anymore really or look into the code. There are a few possibilities:

If you are able, put in a log entry to figure out what the return value is and then we can go from there.

MartinHell commented 3 months ago

Understood. I've noticed as well that it's not the most stable api:P So I decided to take a look at all the instances where tonumber is used and log them:

For weapons you're assumption is correct. The return from weapon:getName() is indeed nil or an empty string:

2024-05-26 13:34:01.916 INFO    dcs_grpc: GREP Weapon: Value : LUA_TYPE: string
2024-05-26 13:34:01.919 INFO    dcs_grpc: GREP Weapon: Value : LUA_TYPE: string
2024-05-26 13:34:02.024 INFO    dcs_grpc: GREP Weapon: Value : LUA_TYPE: string
2024-05-26 13:34:02.036 INFO    dcs_grpc: GREP Weapon: Value : LUA_TYPE: string
2024-05-26 13:34:03.746 INFO    dcs_grpc: GREP Weapon: Value : LUA_TYPE: string
2024-05-26 13:34:04.884 INFO    dcs_grpc: GREP Weapon: Value : LUA_TYPE: string

For the other occurences where getID() is used instead of getName() it looks like the returned value is fine. I suspect that any instance where getName() is used and then being converted using tonumber could be an issue such as scenery:getName() however I didn't see that getting triggered during my (limited) testing.

I used the following snippet to log the above:

  -- Debug Logging
  dlog = weapon:getName()
  dtype = type(dlog)
  format_string = string.format("GREP Weapon: Value %s: LUA_TYPE: %s", dlog, dtype)
  GRPC.logInfo(format_string)

So the question then is what we should do when the value is an empty string, nil or even a value that can't be converted using tonumber()? Since I'm not 100% sure how the id for the weapon is used within the rust code further down the line I don't have a good suggestion. But seeing how the getName() function always return a string and will most likely not be convertable to a number using tonumber() then maybe it should just be completely ignored for weapons, i.e a weapon doesn't have an id (at least according to the game). Even if weapon:getName() returned an actual value it would still be turned to nil by the tonumber() function.

For my specific use-case the ID is never used (I'm only looking for the return of getTypeName()) but I can't say if other people are dependent on an ID, although right now it's not working anyway:P

Regards, Martin

rurounijones commented 3 months ago

Could you try replacing getName with getID and see if that stops the error logging and produces a usable unique ID for the weapon?

https://wiki.hoggitworld.com/view/DCS_func_getID

If that works then we could use that instead with maybe no other consequences.

MartinHell commented 3 months ago

I would but the problem is that getID() is not a part of the weapon object (https://wiki.hoggitworld.com/view/DCS_Class_Weapon). So we can't use that unfortunately. I did try using it in case the documentation is wrong or outdated but the getID() function isn't part of the Weapon object.

2024-06-03 16:12:50.821 ERROR dcs_grpc: Error in event handler: [string "C:\Users\dcs\Saved Games\DCS.openbeta_server\Scripts\DCS-gRPC\exporters\object.lua"]:68: attempt to call method 'getID' (a nil value)

rurounijones commented 3 months ago

In that case the only thing to do is raise a bug report with ED and hope they fix the weapon:getName method on shot events at some point.

Would be interesting to know if it is broken everywhere or just in the event. I bet on the latter.

A PR that sets the ID to -1 if the real value is blank or nil in the exporter would work.

MartinHell commented 3 months ago

I'm not sure I agree that our main issue is a bug in ED code (there might be a bug causing it not to be set for us). But even if it worked and the game set a value to weapon:getName it would still be a string according to https://wiki.hoggitworld.com/view/DCS_func_getName so we would still crash/fail on the tonumber() step.

I guess if the string is "1","2","3" etc it might work but I wouldn't count on that being the case.

The biggest issue here is that the weapon type doesn't have an ID which seems to be an oversight from ED's side, that would probably be what a bug report with ED should be about.

I'll look to see if I can get a PR done for the lua code with your suggestion.

Anyway thanks for taking the time and looking at this with me, I appreciate it!:)

rurounijones commented 3 months ago

Sorry, I wasn't clear so let me rephase. This used to work. weapon:getName() returned a unique numeric ID string which was the equivalent of getID for weapons and just another quirk of ED's APIs. Hence our code using tonumber.

It was working when I submitted this bug report: https://forum.dcs.world/topic/308247-invalid-ballistics-objects-being-created-and-not-cleaned-up-resulting-in-fps-impact/ in September 2022 (as I was checking shot events and the shooting_start and shooting_end events back then to see if they were causing the issue) and some time after that it got broken.

Therefore this is an ED introduced breaking change which reduces functionality; which is something I consider a bug.