Human:getVehicleSeat() could returns nil instead of -1

[Deewarz]

Description

When the player is not in a vehicle, could we return nil instead of -1?

MafiaMP/code/server/src/core/builtins/human.cpp

Lines 45 to 52 in b4dfb16

	int Human::GetVehicleSeat() const {
	const auto updateData = _ent.get<MafiaMP::Shared::Modules::HumanSync::UpdateData>();
	const auto carEnt = flecs::entity(_ent.world(), updateData->carPassenger.carId);
	if (carEnt.is_valid() && carEnt.is_alive()) {
	return updateData->carPassenger.seatId;
	}
	return -1;
	}

What do you think about it?

Reproduction

When not in vehicle, connected to a server, type /seat in chat

listenEvent("onChatCommand", function (player, message, command, args)
  if command == "seat" then
    local seat = player:getVehicleSeat() -- returns -1 currently, could returns nil
  end
end)

[Kamzik123]

Doesn't it return -1 in the original game's lua too? I know it does in Mafia 2.

[Deewarz]

This is a very interesting remark that I often ask myself, should we be similar to the behavior and naming of the original Mafia code or make it as clear as possible for the end user in the sdk layer? cc @zpl-zak @Segfaultd

[zpl-zak]

I'd split the function to two, one providing Seat index (or -1 if empty), other a valid Human object (or nil in case it's empty).

[Segfaultd]

Agree with @zpl-zak. If the function expected outcome is an index, then -1 is the answer. If it's supposed to return an instance then nil, a string then "" etc...

[Deewarz]

So not applicable then. Can I close the issue?

[zpl-zak]

So not applicable then. Can I close the issue?

I'd keep it open since I feel it would be beneficial to split this method. User might want to access the Human object directly, which this method does not provide. I envision having this method renamed to something like getVehicleSeatIndex probably, while the original name would get repurposed for grabbing Human ref.

[Deewarz]

Ok for the renaming to GetVehicleSeatIndex, I'll do it.

Howerver, I don't understand the role of the second function?
Do you mean a function that would be more on the vehicle, like Vehicle::GetHumanAtSeatIndex(seatIndex)?

[zpl-zak]

Ok for the renaming to GetVehicleSeatIndex, I'll do it.

Howerver, I don't understand the role of the second function? Do you mean a function that would be more on the vehicle, like Vehicle::GetHumanAtSeatIndex(seatIndex)?

oh yea sorry, something like that would work, yes.